little cleanup: useEventListener hook (#2749)

frontend/common/useDialog.js

@@ -1,7 +1,7 @@
 //@ts-ignore
 import dialogPolyfill from "https://cdn.jsdelivr.net/npm/[email protected]/dist/dialog-polyfill.esm.min.js"
 
-import { useEffect, useLayoutEffect, useRef } from "../imports/Preact.js"
+import { useLayoutEffect, useMemo, useRef } from "../imports/Preact.js"
 
 /**
  * @returns {[import("../imports/Preact.js").Ref<HTMLDialogElement?>, () => void, () => void, () => void]}
@@ -13,9 +13,11 @@ export const useDialog = () => {
         if (dialog_ref.current != null) dialogPolyfill.registerDialog(dialog_ref.current)
     }, [dialog_ref.current])
 
-    const open = () => dialog_ref.current?.showModal()
-    const close = () => dialog_ref.current?.close()
-    const toggle = () => (dialog_ref.current?.open === true ? dialog_ref.current?.close() : dialog_ref.current?.showModal())
+    return useMemo(() => {
+        const open = () => dialog_ref.current?.showModal()
+        const close = () => dialog_ref.current?.close()
+        const toggle = () => (dialog_ref.current?.open === true ? dialog_ref.current?.close() : dialog_ref.current?.showModal())
 
-    return [dialog_ref, open, close, toggle]
+        return [dialog_ref, open, close, toggle]
+    }, [dialog_ref])
 }

frontend/common/useEventListener.js

@@ -0,0 +1,10 @@
+import { useCallback, useEffect } from "../imports/Preact.js"
+
+export const useEventListener = (element, event_name, handler, deps) => {
+    let handler_cached = useCallback(handler, deps)
+    useEffect(() => {
+        if (element == null) return
+        element.addEventListener(event_name, handler_cached)
+        return () => element.removeEventListener(event_name, handler_cached)
+    }, [element, event_name, handler_cached])
+}

frontend/components/BottomRightPanel.js

@@ -5,6 +5,7 @@ import { LiveDocsTab } from "./LiveDocsTab.js"
 import { is_finished, ProcessTab, total_done, total_tasks, useStatusItem } from "./ProcessTab.js"
 import { useMyClockIsAheadBy } from "../common/clock sync.js"
 import { BackendLaunchPhase } from "../common/Binder.js"
+import { useEventListener } from "../common/useEventListener.js"
 
 /**
  * @typedef PanelTabName
@@ -40,19 +41,20 @@ export let BottomRightPanel = ({
     const hidden = open_tab == null
 
     // Open panel when "open_bottom_right_panel" event is triggered
-    useEffect(() => {
-        let handler = (/** @type {CustomEvent} */ e) => {
+    useEventListener(
+        window,
+        "open_bottom_right_panel",
+        (/** @type {CustomEvent} */ e) => {
             console.log(e.detail)
             // https://github.com/fonsp/Pluto.jl/issues/321
             focus_docs_on_open_ref.current = false
             set_open_tab(e.detail)
             if (window.getComputedStyle(container_ref.current).display === "none") {
                 alert("This browser window is too small to show docs.\n\nMake the window bigger, or try zooming out.")
             }
-        }
-        window.addEventListener("open_bottom_right_panel", handler)
-        return () => window.removeEventListener("open_bottom_right_panel", handler)
-    }, [])
+        },
+        [set_open_tab]
+    )
 
     const status = useWithBackendStatus(notebook, backend_launch_phase)
 

frontend/components/Cell.js

@@ -9,6 +9,7 @@ import { cl } from "../common/ClassTable.js"
 import { PlutoActionsContext } from "../common/PlutoContext.js"
 import { open_pluto_popup } from "../common/open_pluto_popup.js"
 import { SafePreviewOutput } from "./SafePreviewUI.js"
+import { useEventListener } from "../common/useEventListener.js"
 
 const useCellApi = (node_ref, published_object_keys, pluto_actions) => {
     const [cell_api_ready, set_cell_api_ready] = useState(false)
@@ -145,30 +146,34 @@ export const Cell = ({
     const [cm_highlighted_line, set_cm_highlighted_line] = useState(null)
     const [cm_diagnostics, set_cm_diagnostics] = useState([])
 
-    useEffect(() => {
-        const diagnosticListener = (e) => {
+    useEventListener(
+        window,
+        "cell_diagnostics",
+        (e) => {
             if (e.detail.cell_id === cell_id) {
                 set_cm_diagnostics(e.detail.diagnostics)
             }
-        }
-        window.addEventListener("cell_diagnostics", diagnosticListener)
-        return () => window.removeEventListener("cell_diagnostics", diagnosticListener)
-    }, [cell_id])
+        },
+        [cell_id, set_cm_diagnostics]
+    )
 
-    useEffect(() => {
-        const highlightRangeListener = (e) => {
+    useEventListener(
+        window,
+        "cell_highlight_range",
+        (e) => {
             if (e.detail.cell_id == cell_id && e.detail.from != null && e.detail.to != null) {
                 set_cm_highlighted_range({ from: e.detail.from, to: e.detail.to })
             } else {
                 set_cm_highlighted_range(null)
             }
-        }
-        window.addEventListener("cell_highlight_range", highlightRangeListener)
-        return () => window.removeEventListener("cell_highlight_range", highlightRangeListener)
-    }, [cell_id])
+        },
+        [cell_id]
+    )
 
-    useEffect(() => {
-        const focusListener = (e) => {
+    useEventListener(
+        window,
+        "cell_focus",
+        useCallback((e) => {
             if (e.detail.cell_id === cell_id) {
                 if (e.detail.line != null) {
                     const ch = e.detail.ch
@@ -187,13 +192,8 @@ export const Cell = ({
                     }
                 }
             }
-        }
-        window.addEventListener("cell_focus", focusListener)
-        // cleanup
-        return () => {
-            window.removeEventListener("cell_focus", focusListener)
-        }
-    }, [])
+        }, [])
+    )
 
     // When you click to run a cell, we use `waiting_to_run` to immediately set the cell's traffic light to 'queued', while waiting for the backend to catch up.
     const [waiting_to_run, set_waiting_to_run] = useState(false)
@@ -219,13 +219,14 @@ export const Cell = ({
     disable_input_ref.current = disable_input
     const should_set_waiting_to_run_ref = useRef(true)
     should_set_waiting_to_run_ref.current = !running_disabled && !depends_on_disabled_cells
-    useEffect(() => {
-        const handler = (e) => {
+    useEventListener(
+        window,
+        "set_waiting_to_run_smart",
+        (e) => {
             if (e.detail.cell_ids.includes(cell_id)) set_waiting_to_run(should_set_waiting_to_run_ref.current)
-        }
-        window.addEventListener("set_waiting_to_run_smart", handler)
-        return () => window.removeEventListener("set_waiting_to_run_smart", handler)
-    }, [cell_id])
+        },
+        [cell_id, set_waiting_to_run]
+    )
 
     const cell_api_ready = useCellApi(node_ref, published_object_keys, pluto_actions)
     const on_delete = useCallback(() => {

frontend/components/CellInput/tab_help_plugin.js

@@ -56,10 +56,8 @@ export const tab_help_plugin = ViewPlugin.define(
                 this.setready(false)
             },
             keydown: function (event, view) {
-                console.error(event.key, view.state.field(TabHelp), view.state.field(LastFocusWasForced))
                 if (event.key == "Tab") {
                     if (view.state.field(TabHelp) && !view.state.field(LastFocusWasForced) && !view.state.readOnly) {
-                        console.log(123)
                         open_pluto_popup({
                             type: "info",
                             source_element: view.dom,

frontend/components/ExportBanner.js

@@ -1,3 +1,4 @@
+import { useEventListener } from "../common/useEventListener.js"
 import { html, useEffect, useRef } from "../imports/Preact.js"
 
 const Circle = ({ fill }) => html`
@@ -57,22 +58,24 @@ export const ExportBanner = ({ notebook_id, print_title, open, onClose, notebook
 
     //
     let print_old_title_ref = useRef("")
-    useEffect(() => {
-        let a = () => {
+    useEventListener(
+        window,
+        "beforeprint",
+        () => {
             console.log("beforeprint")
             print_old_title_ref.current = document.title
             document.title = print_title.replace(/\.jl$/, "").replace(/\.plutojl$/, "")
-        }
-        let b = () => {
+        },
+        [print_title]
+    )
+    useEventListener(
+        window,
+        "afterprint",
+        () => {
             document.title = print_old_title_ref.current
-        }
-        window.addEventListener("beforeprint", a)
-        window.addEventListener("afterprint", b)
-        return () => {
-            window.removeEventListener("beforeprint", a)
-            window.removeEventListener("afterprint", b)
-        }
-    }, [print_title])
+        },
+        [print_title]
+    )
 
     return html`
         <aside id="export" inert=${!open}>

frontend/components/FrontmatterInput.js

@@ -1,14 +1,14 @@
-import { html, Component, useRef, useLayoutEffect, useState, useEffect } from "../imports/Preact.js"
+import { html, useRef, useLayoutEffect, useState, useEffect, useCallback } from "../imports/Preact.js"
 import { has_ctrl_or_cmd_pressed } from "../common/KeyboardShortcuts.js"
 import _ from "../imports/lodash.js"
 
 import "https://cdn.jsdelivr.net/gh/fonsp/[email protected]/lib/rebel-tag-input.mjs"
 
 //@ts-ignore
-import dialogPolyfill from "https://cdn.jsdelivr.net/npm/[email protected]/dist/dialog-polyfill.esm.min.js"
 import immer from "../imports/immer.js"
 import { useDialog } from "../common/useDialog.js"
 import { FeaturedCard } from "./welcome/FeaturedCard.js"
+import { useEventListener } from "../common/useEventListener.js"
 
 /**
  * @param {{
@@ -24,10 +24,6 @@ export const FrontMatterInput = ({ filename, remote_frontmatter, set_remote_fron
         set_frontmatter(remote_frontmatter ?? {})
     }, [remote_frontmatter])
 
-    // useEffect(() => {
-    //     console.log("New frontmatter:", frontmatter)
-    // }, [frontmatter])
-
     const fm_setter = (key) => (value) =>
         set_frontmatter(
             immer((fm) => {
@@ -41,43 +37,23 @@ export const FrontMatterInput = ({ filename, remote_frontmatter, set_remote_fron
         set_frontmatter(remote_frontmatter ?? {})
         close()
     }
-    const submit = () => {
+    const submit = useCallback(() => {
         set_remote_frontmatter(clean_data(frontmatter) ?? {}).then(() =>
             alert("Frontmatter synchronized ✔\n\nThese parameters will be used in future exports.")
         )
         close()
-    }
-
-    const clean_data = (obj) => {
-        let a = _.isPlainObject(obj)
-            ? Object.fromEntries(
-                  Object.entries(obj)
-                      .map(([key, val]) => [key, clean_data(val)])
-                      .filter(([key, val]) => val != null)
-              )
-            : _.isArray(obj)
-            ? obj.map(clean_data).filter((x) => x != null)
-            : obj
-
-        return _.isEmpty(a) ? null : a
-    }
+    }, [clean_data, set_remote_frontmatter, frontmatter, close])
 
-    useLayoutEffect(() => {
-        window.addEventListener("open pluto frontmatter", open)
-        return () => {
-            window.removeEventListener("open pluto frontmatter", open)
-        }
-    }, [])
+    useEventListener(window, "open pluto frontmatter", open)
 
-    useLayoutEffect(() => {
-        const listener = (e) => {
+    useEventListener(
+        window,
+        "keydown",
+        (e) => {
             if (dialog_ref.current != null) if (dialog_ref.current.contains(e.target)) if (e.key === "Enter" && has_ctrl_or_cmd_pressed(e)) submit()
-        }
-        window.addEventListener("keydown", listener)
-        return () => {
-            window.removeEventListener("keydown", listener)
-        }
-    }, [])
+        },
+        [dialog_ref, submit]
+    )
 
     const frontmatter_with_defaults = {
         title: null,
@@ -188,6 +164,20 @@ export const FrontMatterInput = ({ filename, remote_frontmatter, set_remote_fron
     </dialog>`
 }
 
+const clean_data = (obj) => {
+    let a = _.isPlainObject(obj)
+        ? Object.fromEntries(
+              Object.entries(obj)
+                  .map(([key, val]) => [key, clean_data(val)])
+                  .filter(([key, val]) => val != null)
+          )
+        : _.isArray(obj)
+        ? obj.map(clean_data).filter((x) => x != null)
+        : obj
+
+    return _.isEmpty(a) ? null : a
+}
+
 const special_field_names = ["tags", "date", "license", "url", "color"]
 
 const field_type = (name) => {

frontend/components/HackySideStuff/HijackExternalLinksToOpenInNewTab.js

@@ -1,21 +1,24 @@
+import { useEventListener } from "../../common/useEventListener.js"
 import { useEffect } from "../../imports/Preact.js"
 
 /**
  * Time flies when you're building Pluto...
  * At one moment you self-assignee to issue number #1, next moment we're approaching issue #2000...
- * 
+ *
  * We can't just put `<base target="_blank">` in the `<head>`, because this also opens hash links
  * like `#id` in a new tab...
- * 
+ *
  * This components takes every click event on an <a> that points to another origin (i.e. not `#id`)
  * and sneakily puts in a `target="_blank"` attribute so it opens in a new tab.
  *
  * Fixes https://github.com/fonsp/Pluto.jl/issues/1
  * Based on https://stackoverflow.com/a/12552017/2681964
  */
 export let HijackExternalLinksToOpenInNewTab = () => {
-    useEffect(() => {
-        let handler = (event) => {
+    useEventListener(
+        document,
+        "click",
+        (event) => {
             if (event.defaultPrevented) return
 
             const origin = event.target.closest(`a`)
@@ -26,10 +29,9 @@ export let HijackExternalLinksToOpenInNewTab = () => {
                     origin.target = "_blank"
                 }
             }
-        }
-        document.addEventListener("click", handler)
-        return () => document.removeEventListener("click", handler)
-    })
+        },
+        []
+    )
 
     return null
 }

frontend/components/NonCellOutput.js

@@ -1,3 +1,4 @@
+import { useEventListener } from "../common/useEventListener.js"
 import { html, useEffect, useRef, useState } from "../imports/Preact.js"
 /**
  * Sometimes, we want to render HTML outside of the Cell Output,
@@ -20,17 +21,18 @@ import { html, useEffect, useRef, useState } from "../imports/Preact.js"
 export const NonCellOutput = ({ environment_component, notebook_id }) => {
     const surely_the_latest_updated_set = useRef()
     const [component_set, update_component_set] = useState({})
-    useEffect(() => {
-        const hn = (e) => {
+    useEventListener(
+        document,
+        "eexperimental_add_node_non_cell_output",
+        (e) => {
             try {
                 const { name, node, order } = e.detail
                 surely_the_latest_updated_set.current = { ...surely_the_latest_updated_set.current, [name]: { node, order } }
                 update_component_set(surely_the_latest_updated_set.current)
             } catch (e) {}
-        }
-        document.addEventListener("experimental_add_node_non_cell_output", hn)
-        return () => document.removeEventListener("experimental_add_node_non_cell_output", hn)
-    }, [surely_the_latest_updated_set])
+        },
+        [surely_the_latest_updated_set, update_component_set]
+    )
 
     let components = Object.values(component_set)
     components.sort(({ order: o1 }, { order: o2 }) => o1 - o2)