From c00089bd5728188ce554303b5b18754467c97c85 Mon Sep 17 00:00:00 2001 From: Jacky Zhao Date: Thu, 1 Feb 2024 20:07:14 -0800 Subject: [PATCH] chore: add window.addCleanup() for cleaning up handlers --- docs/advanced/creating components.md | 5 ++-- globals.d.ts | 1 + quartz/components/scripts/callout.inline.ts | 24 ++++++++--------- quartz/components/scripts/darkmode.inline.ts | 2 +- quartz/components/scripts/explorer.inline.ts | 6 ++--- quartz/components/scripts/graph.inline.ts | 2 +- quartz/components/scripts/popover.inline.ts | 2 +- quartz/components/scripts/search.inline.ts | 27 +++++-------------- quartz/components/scripts/spa.inline.ts | 7 +++++ quartz/components/scripts/toc.inline.ts | 2 +- quartz/components/scripts/util.ts | 4 +-- quartz/plugins/emitters/componentResources.ts | 14 +++++----- 12 files changed, 47 insertions(+), 49 deletions(-) diff --git a/docs/advanced/creating components.md b/docs/advanced/creating components.md index 1496b15..27369ab 100644 --- a/docs/advanced/creating components.md +++ b/docs/advanced/creating components.md @@ -156,12 +156,13 @@ document.addEventListener("nav", () => { // do page specific logic here // e.g. attach event listeners const toggleSwitch = document.querySelector("#switch") as HTMLInputElement - toggleSwitch.removeEventListener("change", switchTheme) toggleSwitch.addEventListener("change", switchTheme) + window.addCleanup(() => toggleSwitch.removeEventListener("change", switchTheme)) }) ``` -It is best practice to also unmount any existing event handlers to prevent memory leaks. +It is best practice to track any event handlers via `window.addCleanup` to prevent memory leaks. +This will get called on page navigation. #### Importing Code diff --git a/globals.d.ts b/globals.d.ts index 0509f26..ee13005 100644 --- a/globals.d.ts +++ b/globals.d.ts @@ -8,5 +8,6 @@ export declare global { } interface Window { spaNavigate(url: URL, isBack: boolean = false) + addCleanup(fn: (...args: any[]) => void) } } diff --git a/quartz/components/scripts/callout.inline.ts b/quartz/components/scripts/callout.inline.ts index d8cf518..8f63df3 100644 --- a/quartz/components/scripts/callout.inline.ts +++ b/quartz/components/scripts/callout.inline.ts @@ -1,21 +1,21 @@ function toggleCallout(this: HTMLElement) { const outerBlock = this.parentElement! - outerBlock.classList.toggle(`is-collapsed`) - const collapsed = outerBlock.classList.contains(`is-collapsed`) + outerBlock.classList.toggle("is-collapsed") + const collapsed = outerBlock.classList.contains("is-collapsed") const height = collapsed ? this.scrollHeight : outerBlock.scrollHeight - outerBlock.style.maxHeight = height + `px` + outerBlock.style.maxHeight = height + "px" // walk and adjust height of all parents let current = outerBlock let parent = outerBlock.parentElement while (parent) { - if (!parent.classList.contains(`callout`)) { + if (!parent.classList.contains("callout")) { return } - const collapsed = parent.classList.contains(`is-collapsed`) + const collapsed = parent.classList.contains("is-collapsed") const height = collapsed ? parent.scrollHeight : parent.scrollHeight + current.scrollHeight - parent.style.maxHeight = height + `px` + parent.style.maxHeight = height + "px" current = parent parent = parent.parentElement @@ -30,15 +30,15 @@ function setupCallout() { const title = div.firstElementChild if (title) { - title.removeEventListener(`click`, toggleCallout) - title.addEventListener(`click`, toggleCallout) + title.addEventListener("click", toggleCallout) + window.addCleanup(() => title.removeEventListener("click", toggleCallout)) - const collapsed = div.classList.contains(`is-collapsed`) + const collapsed = div.classList.contains("is-collapsed") const height = collapsed ? title.scrollHeight : div.scrollHeight - div.style.maxHeight = height + `px` + div.style.maxHeight = height + "px" } } } -document.addEventListener(`nav`, setupCallout) -window.addEventListener(`resize`, setupCallout) +document.addEventListener("nav", setupCallout) +window.addEventListener("resize", setupCallout) diff --git a/quartz/components/scripts/darkmode.inline.ts b/quartz/components/scripts/darkmode.inline.ts index 86735e3..4c671aa 100644 --- a/quartz/components/scripts/darkmode.inline.ts +++ b/quartz/components/scripts/darkmode.inline.ts @@ -19,8 +19,8 @@ document.addEventListener("nav", () => { // Darkmode toggle const toggleSwitch = document.querySelector("#darkmode-toggle") as HTMLInputElement - toggleSwitch.removeEventListener("change", switchTheme) toggleSwitch.addEventListener("change", switchTheme) + window.addCleanup(() => toggleSwitch.removeEventListener("change", switchTheme)) if (currentTheme === "dark") { toggleSwitch.checked = true } diff --git a/quartz/components/scripts/explorer.inline.ts b/quartz/components/scripts/explorer.inline.ts index 12546bb..3eb25ea 100644 --- a/quartz/components/scripts/explorer.inline.ts +++ b/quartz/components/scripts/explorer.inline.ts @@ -57,20 +57,20 @@ function setupExplorer() { for (const item of document.getElementsByClassName( "folder-button", ) as HTMLCollectionOf) { - item.removeEventListener("click", toggleFolder) item.addEventListener("click", toggleFolder) + window.addCleanup(() => item.removeEventListener("click", toggleFolder)) } } - explorer.removeEventListener("click", toggleExplorer) explorer.addEventListener("click", toggleExplorer) + window.addCleanup(() => explorer.removeEventListener("click", toggleExplorer)) // Set up click handlers for each folder (click handler on folder "icon") for (const item of document.getElementsByClassName( "folder-icon", ) as HTMLCollectionOf) { - item.removeEventListener("click", toggleFolder) item.addEventListener("click", toggleFolder) + window.addCleanup(() => item.removeEventListener("click", toggleFolder)) } // Get folder state from local storage diff --git a/quartz/components/scripts/graph.inline.ts b/quartz/components/scripts/graph.inline.ts index a76409c..c991e16 100644 --- a/quartz/components/scripts/graph.inline.ts +++ b/quartz/components/scripts/graph.inline.ts @@ -325,6 +325,6 @@ document.addEventListener("nav", async (e: CustomEventMap["nav"]) => { await renderGraph("graph-container", slug) const containerIcon = document.getElementById("global-graph-icon") - containerIcon?.removeEventListener("click", renderGlobalGraph) containerIcon?.addEventListener("click", renderGlobalGraph) + window.addCleanup(() => containerIcon?.removeEventListener("click", renderGlobalGraph)) }) diff --git a/quartz/components/scripts/popover.inline.ts b/quartz/components/scripts/popover.inline.ts index 4d51e2a..0251834 100644 --- a/quartz/components/scripts/popover.inline.ts +++ b/quartz/components/scripts/popover.inline.ts @@ -76,7 +76,7 @@ async function mouseEnterHandler( document.addEventListener("nav", () => { const links = [...document.getElementsByClassName("internal")] as HTMLLinkElement[] for (const link of links) { - link.removeEventListener("mouseenter", mouseEnterHandler) link.addEventListener("mouseenter", mouseEnterHandler) + window.addCleanup(() => link.removeEventListener("mouseenter", mouseEnterHandler)) } }) diff --git a/quartz/components/scripts/search.inline.ts b/quartz/components/scripts/search.inline.ts index 8ead5c9..797685a 100644 --- a/quartz/components/scripts/search.inline.ts +++ b/quartz/components/scripts/search.inline.ts @@ -13,14 +13,13 @@ interface Item { // Can be expanded with things like "term" in the future type SearchType = "basic" | "tags" - -// Current searchType let searchType: SearchType = "basic" -// Current search term // TODO: exact match let currentSearchTerm: string = "" -// index for search let index: FlexSearch.Document | undefined = undefined +const p = new DOMParser() +const encoder = (str: string) => str.toLowerCase().split(/([^a-z]|[^\x00-\x7F])/) +const fetchContentCache: Map = new Map() const contextWindowWords = 30 const numSearchResults = 8 const numTagResults = 5 @@ -79,7 +78,6 @@ function highlight(searchTerm: string, text: string, trim?: boolean) { } function highlightHTML(searchTerm: string, el: HTMLElement) { - // try to highlight longest tokens first const p = new DOMParser() const tokenizedTerms = tokenizeTerm(searchTerm) const html = p.parseFromString(el.innerHTML, "text/html") @@ -117,12 +115,6 @@ function highlightHTML(searchTerm: string, el: HTMLElement) { return html.body } -const p = new DOMParser() -const encoder = (str: string) => str.toLowerCase().split(/([^a-z]|[^\x00-\x7F])/) -let prevShortcutHandler: ((e: HTMLElementEventMap["keydown"]) => void) | undefined = undefined - -const fetchContentCache: Map = new Map() - document.addEventListener("nav", async (e: CustomEventMap["nav"]) => { const currentSlug = e.detail.url @@ -496,16 +488,12 @@ document.addEventListener("nav", async (e: CustomEventMap["nav"]) => { await displayResults(finalResults) } - if (prevShortcutHandler) { - document.removeEventListener("keydown", prevShortcutHandler) - } - document.addEventListener("keydown", shortcutHandler) - prevShortcutHandler = shortcutHandler - searchIcon?.removeEventListener("click", () => showSearch("basic")) + window.addCleanup(() => document.removeEventListener("keydown", shortcutHandler)) searchIcon?.addEventListener("click", () => showSearch("basic")) - searchBar?.removeEventListener("input", onType) + window.addCleanup(() => searchIcon?.removeEventListener("click", () => showSearch("basic"))) searchBar?.addEventListener("input", onType) + window.addCleanup(() => searchBar?.removeEventListener("input", onType)) // setup index if it hasn't been already if (!index) { @@ -546,13 +534,12 @@ document.addEventListener("nav", async (e: CustomEventMap["nav"]) => { async function fillDocument(index: FlexSearch.Document, data: any) { let id = 0 for (const [slug, fileData] of Object.entries(data)) { - await index.addAsync(id, { + await index.addAsync(id++, { id, slug: slug as FullSlug, title: fileData.title, content: fileData.content, tags: fileData.tags, }) - id++ } } diff --git a/quartz/components/scripts/spa.inline.ts b/quartz/components/scripts/spa.inline.ts index c2a44c9..1790bca 100644 --- a/quartz/components/scripts/spa.inline.ts +++ b/quartz/components/scripts/spa.inline.ts @@ -39,6 +39,9 @@ function notifyNav(url: FullSlug) { document.dispatchEvent(event) } +const cleanupFns: Set<(...args: any[]) => void> = new Set() +window.addCleanup = (fn) => cleanupFns.add(fn) + let p: DOMParser async function navigate(url: URL, isBack: boolean = false) { p = p || new DOMParser() @@ -57,6 +60,10 @@ async function navigate(url: URL, isBack: boolean = false) { if (!contents) return + // cleanup old + cleanupFns.forEach((fn) => fn()) + cleanupFns.clear() + const html = p.parseFromString(contents, "text/html") normalizeRelativeURLs(html, url) diff --git a/quartz/components/scripts/toc.inline.ts b/quartz/components/scripts/toc.inline.ts index 2e1e52b..546859e 100644 --- a/quartz/components/scripts/toc.inline.ts +++ b/quartz/components/scripts/toc.inline.ts @@ -29,8 +29,8 @@ function setupToc() { const content = toc.nextElementSibling as HTMLElement | undefined if (!content) return content.style.maxHeight = collapsed ? "0px" : content.scrollHeight + "px" - toc.removeEventListener("click", toggleToc) toc.addEventListener("click", toggleToc) + window.addCleanup(() => toc.removeEventListener("click", toggleToc)) } } diff --git a/quartz/components/scripts/util.ts b/quartz/components/scripts/util.ts index 5fcabad..4ffff29 100644 --- a/quartz/components/scripts/util.ts +++ b/quartz/components/scripts/util.ts @@ -12,10 +12,10 @@ export function registerEscapeHandler(outsideContainer: HTMLElement | null, cb: cb() } - outsideContainer?.removeEventListener("click", click) outsideContainer?.addEventListener("click", click) - document.removeEventListener("keydown", esc) + window.addCleanup(() => outsideContainer?.removeEventListener("click", click)) document.addEventListener("keydown", esc) + window.addCleanup(() => document.removeEventListener("keydown", esc)) } export function removeAllChildren(node: HTMLElement) { diff --git a/quartz/plugins/emitters/componentResources.ts b/quartz/plugins/emitters/componentResources.ts index accc261..5eb9718 100644 --- a/quartz/plugins/emitters/componentResources.ts +++ b/quartz/plugins/emitters/componentResources.ts @@ -131,9 +131,11 @@ function addGlobalPageResources( componentResources.afterDOMLoaded.push(spaRouterScript) } else { componentResources.afterDOMLoaded.push(` - window.spaNavigate = (url, _) => window.location.assign(url) - const event = new CustomEvent("nav", { detail: { url: document.body.dataset.slug } }) - document.dispatchEvent(event)`) + window.spaNavigate = (url, _) => window.location.assign(url) + window.addCleanup = () => {} + const event = new CustomEvent("nav", { detail: { url: document.body.dataset.slug } }) + document.dispatchEvent(event) + `) } let wsUrl = `ws://localhost:${ctx.argv.wsPort}` @@ -147,9 +149,9 @@ function addGlobalPageResources( loadTime: "afterDOMReady", contentType: "inline", script: ` - const socket = new WebSocket('${wsUrl}') - socket.addEventListener('message', () => document.location.reload()) - `, + const socket = new WebSocket('${wsUrl}') + socket.addEventListener('message', () => document.location.reload()) + `, }) } }