From 1b4ac1aa57fbbfc18fb1c5c8a27c168748584889 Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Sun, 8 Mar 2026 03:06:43 -0700 Subject: [PATCH 1/8] Clean up component setup/tear-down side effects --- frontend/src/components/Editor.svelte | 2 + .../components/floating-menus/MenuList.svelte | 20 ++++++--- .../src/components/layout/FloatingMenu.svelte | 19 ++++++-- .../src/components/panels/Document.svelte | 8 ++-- .../widgets/inputs/NumberInput.svelte | 40 ++++++++++++----- .../widgets/inputs/ScrollbarInput.svelte | 6 ++- .../widgets/inputs/SpectrumInput.svelte | 6 ++- frontend/src/state-providers/tooltip.ts | 44 ++++++++++++------- 8 files changed, 104 insertions(+), 41 deletions(-) diff --git a/frontend/src/components/Editor.svelte b/frontend/src/components/Editor.svelte index 0f14408298..d2f3a1ac3a 100644 --- a/frontend/src/components/Editor.svelte +++ b/frontend/src/components/Editor.svelte @@ -56,6 +56,8 @@ onDestroy(() => { // Call the destructor for each manager inputManagerDestructor(); + // Clean up tooltip event listeners + tooltip.destroy(); }); diff --git a/frontend/src/components/floating-menus/MenuList.svelte b/frontend/src/components/floating-menus/MenuList.svelte index aeb196f736..d7f042f5d0 100644 --- a/frontend/src/components/floating-menus/MenuList.svelte +++ b/frontend/src/components/floating-menus/MenuList.svelte @@ -47,6 +47,7 @@ let reactiveEntries = entries; let highlighted: MenuListEntry | undefined = activeEntry; let virtualScrollingEntriesStart = 0; + let keydownListenerAdded = false; // `watchOpen` is called only when `open` is changed from outside this component $: watchOpen(open); @@ -67,11 +68,13 @@ // TODO: The current approach is hacky and blocks the allowances for shortcuts like the key to open the browser's dev tools. onMount(async () => { await tick(); - if (open && !inNestedMenuList()) addEventListener("keydown", keydown); + if (open && !inNestedMenuList() && !keydownListenerAdded) { + addEventListener("keydown", keydown); + keydownListenerAdded = true; + } }); - onDestroy(async () => { - await tick(); - if (!inNestedMenuList()) removeEventListener("keydown", keydown); + onDestroy(() => { + removeEventListener("keydown", keydown); }); function inNestedMenuList(): boolean { @@ -129,8 +132,13 @@ } function watchOpen(open: boolean) { - if (open && !inNestedMenuList()) addEventListener("keydown", keydown); - else if (!inNestedMenuList()) removeEventListener("keydown", keydown); + if (open && !inNestedMenuList() && !keydownListenerAdded) { + addEventListener("keydown", keydown); + keydownListenerAdded = true; + } else if (!open && !inNestedMenuList() && keydownListenerAdded) { + removeEventListener("keydown", keydown); + keydownListenerAdded = false; + } highlighted = activeEntry; dispatch("open", open); diff --git a/frontend/src/components/layout/FloatingMenu.svelte b/frontend/src/components/layout/FloatingMenu.svelte index ba69a4bab0..4dece0b91e 100644 --- a/frontend/src/components/layout/FloatingMenu.svelte +++ b/frontend/src/components/layout/FloatingMenu.svelte @@ -17,7 +17,7 @@ diff --git a/frontend/src/components/widgets/inputs/NumberInput.svelte b/frontend/src/components/widgets/inputs/NumberInput.svelte index 3f5a71d74f..caf0c0a166 100644 --- a/frontend/src/components/widgets/inputs/NumberInput.svelte +++ b/frontend/src/components/widgets/inputs/NumberInput.svelte @@ -87,6 +87,8 @@ let shiftKeyDown = false; // Track whether the Ctrl key is currently held down. let ctrlKeyDown = false; + // Cleanup function for active drag interactions, called on destroy to prevent leaked listeners + let activeDragCleanup: (() => void) | undefined; $: watchValue(value, unit); $: sliderStepValue = isInteger ? (step === undefined ? 1 : step) : "any"; @@ -107,10 +109,21 @@ addEventListener("mousemove", trackShiftAndCtrl); }); onDestroy(() => { + clearTimeout(repeatTimeout); + + activeDragCleanup?.(); + + // Clean up any listeners related to tracking the Shift and Ctrl keys removeEventListener("keydown", trackShiftAndCtrl); removeEventListener("keyup", trackShiftAndCtrl); removeEventListener("mousemove", trackShiftAndCtrl); - clearTimeout(repeatTimeout); + + // Clean up any slider-related listeners that may be active + removeEventListener("mousedown", sliderAbortFromMousedown); + removeEventListener("keydown", sliderAbortFromMousedown); + removeEventListener("pointermove", sliderAbortFromDragging); + removeEventListener("keydown", sliderAbortFromDragging); + removeEventListener("keydown", incrementPressAbort); }); // =============================== @@ -333,10 +346,9 @@ alreadyActedGuard = true; isDragging = true; - beginDrag(e); - removeEventListener("pointermove", onMove); - removeEventListener("pointerup", onUp); + activeDragCleanup?.(); + beginDrag(e); }; // If it's a mouseup, we'll begin editing the text field. const onUp = () => { @@ -346,11 +358,15 @@ isDragging = false; self?.focus(); - removeEventListener("pointermove", onMove); - removeEventListener("pointerup", onUp); + activeDragCleanup?.(); }; addEventListener("pointermove", onMove); addEventListener("pointerup", onUp); + activeDragCleanup = () => { + removeEventListener("pointermove", onMove); + removeEventListener("pointerup", onUp); + activeDragCleanup = undefined; + }; } function beginDrag(e: PointerEvent) { @@ -449,16 +465,20 @@ cumulativeDragDelta = 0; // Clean up the event listeners. - removeEventListener("pointerup", pointerUp); - removeEventListener("pointermove", pointerMove); - removeEventListener("pointerlockmove", pointerLockMove); - if (usePointerLock) document.removeEventListener("pointerlockchange", pointerLockChange); + activeDragCleanup?.(); }; addEventListener("pointerup", pointerUp); addEventListener("pointermove", pointerMove); addEventListener("pointerlockmove", pointerLockMove); if (usePointerLock) document.addEventListener("pointerlockchange", pointerLockChange); + activeDragCleanup = () => { + removeEventListener("pointerup", pointerUp); + removeEventListener("pointermove", pointerMove); + removeEventListener("pointerlockmove", pointerLockMove); + if (usePointerLock) document.removeEventListener("pointerlockchange", pointerLockChange); + activeDragCleanup = undefined; + }; } function pointerLockMoveUpdate(delta: number, slow: boolean, snapping: boolean, initialValue: number) { diff --git a/frontend/src/components/widgets/inputs/ScrollbarInput.svelte b/frontend/src/components/widgets/inputs/ScrollbarInput.svelte index 99de85d703..4a0e3efc38 100644 --- a/frontend/src/components/widgets/inputs/ScrollbarInput.svelte +++ b/frontend/src/components/widgets/inputs/ScrollbarInput.svelte @@ -1,5 +1,5 @@ diff --git a/frontend/src/components/panels/Document.svelte b/frontend/src/components/panels/Document.svelte index 737b7f3b64..45fb941bf2 100644 --- a/frontend/src/components/panels/Document.svelte +++ b/frontend/src/components/panels/Document.svelte @@ -10,7 +10,7 @@ import { pasteFile } from "@graphite/utility-functions/files"; import { textInputCleanup } from "@graphite/utility-functions/keyboard-entry"; import { rasterizeSVGCanvas } from "@graphite/utility-functions/rasterization"; - import { setupViewportResizeObserver, cleanupViewportResizeObserver } from "@graphite/utility-functions/viewports"; + import { setupViewportResizeObserver } from "@graphite/utility-functions/viewports"; import ColorPicker from "@graphite/components/floating-menus/ColorPicker.svelte"; import EyedropperPreview, { ZOOM_WINDOW_DIMENSIONS } from "@graphite/components/floating-menus/EyedropperPreview.svelte"; @@ -76,6 +76,7 @@ let devicePixelRatio: number | undefined; let removeUpdatePixelRatio: (() => void) | undefined; let viewportResizeObserver: ResizeObserver | undefined; + let cleanupViewportResizeObserver: (() => void) | undefined; // Dimension is rounded up to the nearest even number because resizing is centered, and dividing an odd number by 2 for centering causes antialiasing $: canvasWidthRoundedToEven = canvasWidth && (canvasWidth % 2 === 1 ? canvasWidth + 1 : canvasWidth); @@ -526,7 +527,7 @@ // Setup ResizeObserver for pixel-perfect viewport tracking with physical dimensions // This must happen in onMount to ensure the viewport container element exists - setupViewportResizeObserver(editor); + cleanupViewportResizeObserver = setupViewportResizeObserver(editor); // Also observe the inner viewport for canvas sizing and ruler updates viewportResizeObserver = new ResizeObserver(() => { @@ -536,9 +537,21 @@ }); onDestroy(() => { - cleanupViewportResizeObserver(); + cleanupViewportResizeObserver?.(); viewportResizeObserver?.disconnect(); removeUpdatePixelRatio?.(); + + editor.subscriptions.unsubscribeFrontendMessage("UpdateDocumentArtwork"); + editor.subscriptions.unsubscribeFrontendMessage("UpdateEyedropperSamplingState"); + editor.subscriptions.unsubscribeFrontendMessage("UpdateGradientStopColorPickerPosition"); + editor.subscriptions.unsubscribeFrontendMessage("UpdateDocumentScrollbars"); + editor.subscriptions.unsubscribeFrontendMessage("UpdateDocumentRulers"); + editor.subscriptions.unsubscribeFrontendMessage("UpdateMouseCursor"); + editor.subscriptions.unsubscribeFrontendMessage("TriggerTextCommit"); + editor.subscriptions.unsubscribeFrontendMessage("DisplayEditableTextbox"); + editor.subscriptions.unsubscribeFrontendMessage("DisplayEditableTextboxUpdateFontData"); + editor.subscriptions.unsubscribeFrontendMessage("DisplayEditableTextboxTransform"); + editor.subscriptions.unsubscribeFrontendMessage("DisplayRemoveEditableTextbox"); }); diff --git a/frontend/src/components/widgets/inputs/NumberInput.svelte b/frontend/src/components/widgets/inputs/NumberInput.svelte index caf0c0a166..dec0de773a 100644 --- a/frontend/src/components/widgets/inputs/NumberInput.svelte +++ b/frontend/src/components/widgets/inputs/NumberInput.svelte @@ -89,6 +89,10 @@ let ctrlKeyDown = false; // Cleanup function for active drag interactions, called on destroy to prevent leaked listeners let activeDragCleanup: (() => void) | undefined; + // Track the slider abort state for cleanup on destroy + let sliderResetAbortHandler: (() => void) | undefined; + let sliderAbortTimeout1: ReturnType | undefined; + let sliderAbortTimeout2: ReturnType | undefined; $: watchValue(value, unit); $: sliderStepValue = isInteger ? (step === undefined ? 1 : step) : "any"; @@ -110,6 +114,8 @@ }); onDestroy(() => { clearTimeout(repeatTimeout); + clearTimeout(sliderAbortTimeout1); + clearTimeout(sliderAbortTimeout2); activeDragCleanup?.(); @@ -124,6 +130,7 @@ removeEventListener("pointermove", sliderAbortFromDragging); removeEventListener("keydown", sliderAbortFromDragging); removeEventListener("keydown", incrementPressAbort); + if (sliderResetAbortHandler) removeEventListener("pointerup", sliderResetAbortHandler); }); // =============================== @@ -310,7 +317,7 @@ pressingArrow = false; clearTimeout(repeatTimeout); updateValue(initialValueBeforeDragging); - removeEventListener("keydown", onIncrementPointerUp); + removeEventListener("keydown", incrementPressAbort); } // ======================================= @@ -677,7 +684,7 @@ // End the user's drag by instantaneously disabling and re-enabling the range input element if (inputRangeElement) inputRangeElement.disabled = true; - setTimeout(() => { + sliderAbortTimeout1 = setTimeout(() => { if (inputRangeElement) inputRangeElement.disabled = false; }, 0); @@ -700,11 +707,13 @@ // dragging the slider, hitting Escape, then releasing the mouse button. This results in being transferred by `onSliderInput()` to the // "Deciding" state when we should remain in the "Ready" state as set here. (For debugging, this can be visualized in CSS by // recoloring the fake slider handle, which is shown in the "Deciding" state.) - setTimeout(() => (rangeSliderClickDragState = "Ready"), 0); + sliderAbortTimeout2 = setTimeout(() => (rangeSliderClickDragState = "Ready"), 0); // Clean up the event listener that was used to call this function. removeEventListener("pointerup", sliderResetAbort); + sliderResetAbortHandler = undefined; }; + sliderResetAbortHandler = sliderResetAbort; addEventListener("pointerup", sliderResetAbort); // Clean up the event listeners that were for tracking an abort while dragging the slider, now that we're no longer dragging it. diff --git a/frontend/src/components/window/StatusBar.svelte b/frontend/src/components/window/StatusBar.svelte index 75003e6f7e..a246d3ef2f 100644 --- a/frontend/src/components/window/StatusBar.svelte +++ b/frontend/src/components/window/StatusBar.svelte @@ -1,5 +1,5 @@ diff --git a/frontend/src/components/window/TitleBar.svelte b/frontend/src/components/window/TitleBar.svelte index c7b7935093..052c17485d 100644 --- a/frontend/src/components/window/TitleBar.svelte +++ b/frontend/src/components/window/TitleBar.svelte @@ -1,5 +1,5 @@ diff --git a/frontend/src/io-managers/clipboard.ts b/frontend/src/io-managers/clipboard.ts index af493ea7d9..f6da7a188a 100644 --- a/frontend/src/io-managers/clipboard.ts +++ b/frontend/src/io-managers/clipboard.ts @@ -1,6 +1,6 @@ import type { Editor } from "@graphite/editor"; -export function createClipboardManager(editor: Editor) { +export function createClipboardManager(editor: Editor): () => void { // Subscribe to process backend event editor.subscriptions.subscribeFrontendMessage("TriggerClipboardWrite", (data) => { // If the Clipboard API is supported in the browser, copy text to the clipboard @@ -12,6 +12,12 @@ export function createClipboardManager(editor: Editor) { editor.subscriptions.subscribeFrontendMessage("TriggerSelectionWrite", async (data) => { insertAtCaret(data.content); }); + + return () => { + editor.subscriptions.unsubscribeFrontendMessage("TriggerClipboardWrite"); + editor.subscriptions.unsubscribeFrontendMessage("TriggerSelectionRead"); + editor.subscriptions.unsubscribeFrontendMessage("TriggerSelectionWrite"); + }; } function readAtCaret(cut: boolean): string | undefined { diff --git a/frontend/src/io-managers/fonts.ts b/frontend/src/io-managers/fonts.ts index eafd09bc06..213f57234f 100644 --- a/frontend/src/io-managers/fonts.ts +++ b/frontend/src/io-managers/fonts.ts @@ -4,7 +4,7 @@ type ApiResponse = { family: string; variants: string[]; files: Record void { // Subscribe to process backend events editor.subscriptions.subscribeFrontendMessage("TriggerFontCatalogLoad", async () => { const response = await fetch(FONT_LIST_API); @@ -40,4 +40,9 @@ export function createFontsManager(editor: Editor) { console.error("Failed to load font:", error); } }); + + return () => { + editor.subscriptions.unsubscribeFrontendMessage("TriggerFontCatalogLoad"); + editor.subscriptions.unsubscribeFrontendMessage("TriggerFontDataLoad"); + }; } diff --git a/frontend/src/io-managers/hyperlink.ts b/frontend/src/io-managers/hyperlink.ts index 90055bd55c..b617c546b5 100644 --- a/frontend/src/io-managers/hyperlink.ts +++ b/frontend/src/io-managers/hyperlink.ts @@ -1,8 +1,12 @@ import type { Editor } from "@graphite/editor"; -export function createHyperlinkManager(editor: Editor) { +export function createHyperlinkManager(editor: Editor): () => void { // Subscribe to process backend event editor.subscriptions.subscribeFrontendMessage("TriggerVisitLink", async (data) => { window.open(data.url, "_blank"); }); + + return () => { + editor.subscriptions.unsubscribeFrontendMessage("TriggerVisitLink"); + }; } diff --git a/frontend/src/io-managers/input.ts b/frontend/src/io-managers/input.ts index 3032398e34..0529f64960 100644 --- a/frontend/src/io-managers/input.ts +++ b/frontend/src/io-managers/input.ts @@ -508,7 +508,11 @@ export function createInputManager(editor: Editor, dialog: DialogState, portfoli bindListeners(); // Return the destructor - return unbindListeners; + return () => { + unbindListeners(); + editor.subscriptions.unsubscribeFrontendMessage("TriggerClipboardRead"); + editor.subscriptions.unsubscribeFrontendMessage("WindowPointerLockMove"); + }; } function targetIsTextField(target: EventTarget | HTMLElement | undefined): boolean { diff --git a/frontend/src/io-managers/localization.ts b/frontend/src/io-managers/localization.ts index 7142942cf4..85ea29c6b9 100644 --- a/frontend/src/io-managers/localization.ts +++ b/frontend/src/io-managers/localization.ts @@ -1,11 +1,15 @@ import type { Editor } from "@graphite/editor"; -export function createLocalizationManager(editor: Editor) { +export function createLocalizationManager(editor: Editor): () => void { // Subscribe to process backend event editor.subscriptions.subscribeFrontendMessage("TriggerAboutGraphiteLocalizedCommitDate", (data) => { const localized = localizeTimestamp(data.commitDate); editor.handle.requestAboutGraphiteDialogWithLocalizedCommitDate(localized.timestamp, localized.year); }); + + return () => { + editor.subscriptions.unsubscribeFrontendMessage("TriggerAboutGraphiteLocalizedCommitDate"); + }; } function localizeTimestamp(utc: string): { timestamp: string; year: string } { diff --git a/frontend/src/io-managers/panic.ts b/frontend/src/io-managers/panic.ts index da3cc1df3f..8e6f100654 100644 --- a/frontend/src/io-managers/panic.ts +++ b/frontend/src/io-managers/panic.ts @@ -3,7 +3,7 @@ import type { DialogState } from "@graphite/state-providers/dialog"; import { browserVersion, operatingSystem } from "@graphite/utility-functions/platform"; import { stripIndents } from "@graphite/utility-functions/strip-indents"; -export function createPanicManager(editor: Editor, dialogState: DialogState) { +export function createPanicManager(editor: Editor, dialogState: DialogState): () => void { // Code panic dialog and console error editor.subscriptions.subscribeFrontendMessage("DisplayDialogPanic", (data) => { // `Error.stackTraceLimit` is only available in V8/Chromium @@ -16,6 +16,10 @@ export function createPanicManager(editor: Editor, dialogState: DialogState) { dialogState.createCrashDialog(panicDetails); }); + + return () => { + editor.subscriptions.unsubscribeFrontendMessage("DisplayDialogPanic"); + }; } export function githubUrl(panicDetails: string): string { @@ -28,7 +32,7 @@ export function githubUrl(panicDetails: string): string { **Steps To Reproduce** Describe precisely how the crash occurred, step by step, starting with a new editor window. - 1. Open the Graphite editor at https://editor.graphite.art + 1. Open the Graphite editor at https://dev.graphite.art — IMPORTANT! Confirm you have tested in this development version. It may have already been fixed since the last stable release. 2. 3. 4. diff --git a/frontend/src/io-managers/persistence.ts b/frontend/src/io-managers/persistence.ts index 45b725b925..bd15837a4d 100644 --- a/frontend/src/io-managers/persistence.ts +++ b/frontend/src/io-managers/persistence.ts @@ -7,7 +7,7 @@ import type { MessageBody } from "@graphite/subscription-router"; const graphiteStore = createStore("graphite", "store"); -export function createPersistenceManager(editor: Editor, portfolio: PortfolioState) { +export function createPersistenceManager(editor: Editor, portfolio: PortfolioState): () => void { // DOCUMENTS async function storeDocumentOrder() { @@ -200,6 +200,17 @@ export function createPersistenceManager(editor: Editor, portfolio: PortfolioSta await storeCurrentDocumentId(documentId); } }); + + return () => { + editor.subscriptions.unsubscribeFrontendMessage("TriggerSavePreferences"); + editor.subscriptions.unsubscribeFrontendMessage("TriggerLoadPreferences"); + editor.subscriptions.unsubscribeFrontendMessage("TriggerPersistenceWriteDocument"); + editor.subscriptions.unsubscribeFrontendMessage("TriggerPersistenceRemoveDocument"); + editor.subscriptions.unsubscribeFrontendMessage("TriggerLoadFirstAutoSaveDocument"); + editor.subscriptions.unsubscribeFrontendMessage("TriggerLoadRestAutoSaveDocuments"); + editor.subscriptions.unsubscribeFrontendMessage("TriggerOpenLaunchDocuments"); + editor.subscriptions.unsubscribeFrontendMessage("TriggerSaveActiveDocument"); + }; } export async function wipeDocuments() { diff --git a/frontend/src/state-providers/app-window.ts b/frontend/src/state-providers/app-window.ts index ff9ff04295..9cd5016612 100644 --- a/frontend/src/state-providers/app-window.ts +++ b/frontend/src/state-providers/app-window.ts @@ -50,8 +50,17 @@ export function createAppWindowState(editor: Editor) { }); }); + function destroy() { + editor.subscriptions.unsubscribeFrontendMessage("UpdatePlatform"); + editor.subscriptions.unsubscribeFrontendMessage("UpdateMaximized"); + editor.subscriptions.unsubscribeFrontendMessage("UpdateFullscreen"); + editor.subscriptions.unsubscribeFrontendMessage("UpdateViewportHolePunch"); + editor.subscriptions.unsubscribeFrontendMessage("UpdateUIScale"); + } + return { subscribe, + destroy, }; } export type AppWindowState = ReturnType; diff --git a/frontend/src/state-providers/dialog.ts b/frontend/src/state-providers/dialog.ts index a534c9321b..45ccd36f6a 100644 --- a/frontend/src/state-providers/dialog.ts +++ b/frontend/src/state-providers/dialog.ts @@ -96,10 +96,20 @@ export function createDialogState(editor: Editor) { editor.handle.requestLicensesThirdPartyDialogWithLicenseText(licenseText); }); + function destroy() { + editor.subscriptions.unsubscribeFrontendMessage("DisplayDialog"); + editor.subscriptions.unsubscribeFrontendMessage("DialogClose"); + editor.subscriptions.unsubscribeFrontendMessage("TriggerDisplayThirdPartyLicensesDialog"); + editor.subscriptions.unsubscribeLayoutUpdate("DialogButtons"); + editor.subscriptions.unsubscribeLayoutUpdate("DialogColumn1"); + editor.subscriptions.unsubscribeLayoutUpdate("DialogColumn2"); + } + return { subscribe, dismissDialog, createCrashDialog, + destroy, }; } export type DialogState = ReturnType; diff --git a/frontend/src/state-providers/document.ts b/frontend/src/state-providers/document.ts index 3946e17291..cb8423667f 100644 --- a/frontend/src/state-providers/document.ts +++ b/frontend/src/state-providers/document.ts @@ -79,8 +79,19 @@ export function createDocumentState(editor: Editor) { }); }); + function destroy() { + editor.subscriptions.unsubscribeFrontendMessage("UpdateGraphFadeArtwork"); + editor.subscriptions.unsubscribeFrontendMessage("UpdateGraphViewOverlay"); + editor.subscriptions.unsubscribeLayoutUpdate("ToolOptions"); + editor.subscriptions.unsubscribeLayoutUpdate("DocumentBar"); + editor.subscriptions.unsubscribeLayoutUpdate("ToolShelf"); + editor.subscriptions.unsubscribeLayoutUpdate("WorkingColors"); + editor.subscriptions.unsubscribeLayoutUpdate("NodeGraphControlBar"); + } + return { subscribe, + destroy, }; } export type DocumentState = ReturnType; diff --git a/frontend/src/state-providers/fullscreen.ts b/frontend/src/state-providers/fullscreen.ts index aeac8b5646..8df7d10c87 100644 --- a/frontend/src/state-providers/fullscreen.ts +++ b/frontend/src/state-providers/fullscreen.ts @@ -52,12 +52,17 @@ export function createFullscreenState(editor: Editor) { toggleFullscreen(); }); + function destroy() { + editor.subscriptions.unsubscribeFrontendMessage("WindowFullscreen"); + } + return { subscribe, fullscreenModeChanged, enterFullscreen, exitFullscreen, toggleFullscreen, + destroy, }; } export type FullscreenState = ReturnType; diff --git a/frontend/src/state-providers/node-graph.ts b/frontend/src/state-providers/node-graph.ts index ae96b2c470..5bb29972d1 100644 --- a/frontend/src/state-providers/node-graph.ts +++ b/frontend/src/state-providers/node-graph.ts @@ -185,9 +185,31 @@ export function createNodeGraphState(editor: Editor) { }); }); + function destroy() { + editor.subscriptions.unsubscribeFrontendMessage("SendUIMetadata"); + editor.subscriptions.unsubscribeFrontendMessage("UpdateBox"); + editor.subscriptions.unsubscribeFrontendMessage("UpdateClickTargets"); + editor.subscriptions.unsubscribeFrontendMessage("UpdateContextMenuInformation"); + editor.subscriptions.unsubscribeFrontendMessage("UpdateImportReorderIndex"); + editor.subscriptions.unsubscribeFrontendMessage("UpdateExportReorderIndex"); + editor.subscriptions.unsubscribeFrontendMessage("UpdateImportsExports"); + editor.subscriptions.unsubscribeFrontendMessage("UpdateInSelectedNetwork"); + editor.subscriptions.unsubscribeFrontendMessage("UpdateLayerWidths"); + editor.subscriptions.unsubscribeFrontendMessage("UpdateNodeGraphNodes"); + editor.subscriptions.unsubscribeFrontendMessage("UpdateNodeGraphErrorDiagnostic"); + editor.subscriptions.unsubscribeFrontendMessage("UpdateVisibleNodes"); + editor.subscriptions.unsubscribeFrontendMessage("UpdateNodeGraphWires"); + editor.subscriptions.unsubscribeFrontendMessage("ClearAllNodeGraphWires"); + editor.subscriptions.unsubscribeFrontendMessage("UpdateNodeGraphSelection"); + editor.subscriptions.unsubscribeFrontendMessage("UpdateNodeGraphTransform"); + editor.subscriptions.unsubscribeFrontendMessage("UpdateNodeThumbnail"); + editor.subscriptions.unsubscribeFrontendMessage("UpdateWirePathInProgress"); + } + return { subscribe, closeContextMenu, + destroy, }; } export type NodeGraphState = ReturnType; diff --git a/frontend/src/state-providers/portfolio.ts b/frontend/src/state-providers/portfolio.ts index 5dd28ada2a..de26d78eb0 100644 --- a/frontend/src/state-providers/portfolio.ts +++ b/frontend/src/state-providers/portfolio.ts @@ -99,8 +99,23 @@ export function createPortfolioState(editor: Editor) { }); }); + function destroy() { + editor.subscriptions.unsubscribeFrontendMessage("UpdateOpenDocumentsList"); + editor.subscriptions.unsubscribeFrontendMessage("UpdateActiveDocument"); + editor.subscriptions.unsubscribeFrontendMessage("TriggerFetchAndOpenDocument"); + editor.subscriptions.unsubscribeFrontendMessage("TriggerOpen"); + editor.subscriptions.unsubscribeFrontendMessage("TriggerImport"); + editor.subscriptions.unsubscribeFrontendMessage("TriggerSaveDocument"); + editor.subscriptions.unsubscribeFrontendMessage("TriggerSaveFile"); + editor.subscriptions.unsubscribeFrontendMessage("TriggerExportImage"); + editor.subscriptions.unsubscribeFrontendMessage("UpdateDataPanelState"); + editor.subscriptions.unsubscribeFrontendMessage("UpdatePropertiesPanelState"); + editor.subscriptions.unsubscribeFrontendMessage("UpdateLayersPanelState"); + } + return { subscribe, + destroy, }; } export type PortfolioState = ReturnType; diff --git a/frontend/src/state-providers/tooltip.ts b/frontend/src/state-providers/tooltip.ts index 866414d078..f80d78d359 100644 --- a/frontend/src/state-providers/tooltip.ts +++ b/frontend/src/state-providers/tooltip.ts @@ -79,12 +79,17 @@ export function createTooltipState(editor: Editor) { function destroy() { if (tooltipTimeout) clearTimeout(tooltipTimeout); + document.removeEventListener("mouseover", onMouseOver); document.removeEventListener("mousemove", onMouseMove); document.removeEventListener("mouseleave", onMouseLeave); document.removeEventListener("mousedown", closeTooltip); document.removeEventListener("keydown", closeTooltip); document.removeEventListener("wheel", closeTooltip); + + editor.subscriptions.unsubscribeFrontendMessage("SendShortcutShiftClick"); + editor.subscriptions.unsubscribeFrontendMessage("SendShortcutAltClick"); + editor.subscriptions.unsubscribeFrontendMessage("SendShortcutFullscreen"); } document.addEventListener("mouseover", onMouseOver); diff --git a/frontend/src/utility-functions/viewports.ts b/frontend/src/utility-functions/viewports.ts index 670f2b1bea..fc6d7b3603 100644 --- a/frontend/src/utility-functions/viewports.ts +++ b/frontend/src/utility-functions/viewports.ts @@ -1,20 +1,13 @@ import type { Editor } from "@graphite/editor"; -let resizeObserver: ResizeObserver | undefined; - -export function setupViewportResizeObserver(editor: Editor) { - // Clean up existing observer if any - if (resizeObserver) { - resizeObserver.disconnect(); - } - +export function setupViewportResizeObserver(editor: Editor): () => void { const viewports = Array.from(window.document.querySelectorAll("[data-viewport-container]")); - if (viewports.length <= 0) return; + if (viewports.length <= 0) return () => {}; const viewport = viewports[0]; - if (!(viewport instanceof HTMLElement)) return; + if (!(viewport instanceof HTMLElement)) return () => {}; - resizeObserver = new ResizeObserver((entries) => { + const resizeObserver = new ResizeObserver((entries) => { for (const entry of entries) { const devicePixelRatio = window.devicePixelRatio || 1; @@ -52,11 +45,8 @@ export function setupViewportResizeObserver(editor: Editor) { }); resizeObserver.observe(viewport); -} -export function cleanupViewportResizeObserver() { - if (resizeObserver) { + return () => { resizeObserver.disconnect(); - resizeObserver = undefined; - } + }; } From 11b500857604ad9fdc6ad411ac840152e01bd1e1 Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Sun, 8 Mar 2026 05:01:16 -0700 Subject: [PATCH 3/8] Remove nonfunctional debouncer --- .../src/components/widgets/WidgetSpan.svelte | 5 ++-- frontend/src/utility-functions/debounce.ts | 30 ------------------- 2 files changed, 2 insertions(+), 33 deletions(-) delete mode 100644 frontend/src/utility-functions/debounce.ts diff --git a/frontend/src/components/widgets/WidgetSpan.svelte b/frontend/src/components/widgets/WidgetSpan.svelte index 6a402b067e..43aba10b9b 100644 --- a/frontend/src/components/widgets/WidgetSpan.svelte +++ b/frontend/src/components/widgets/WidgetSpan.svelte @@ -4,7 +4,6 @@ import type { LayoutTarget, Widget, WidgetInstance } from "@graphite/../wasm/pkg/graphite_wasm"; import type { Editor } from "@graphite/editor"; import { parseFillChoice } from "@graphite/utility-functions/colors"; - import { debouncer } from "@graphite/utility-functions/debounce"; import NodeCatalog from "@graphite/components/floating-menus/NodeCatalog.svelte"; import BreadcrumbTrailButtons from "@graphite/components/widgets/buttons/BreadcrumbTrailButtons.svelte"; @@ -137,7 +136,7 @@ getProps: (props, index) => ({ ...props, $$events: { - value: (e: CustomEvent) => debouncer((value: unknown) => widgetValueCommitAndUpdate(index, value, false), { debounceTime: 120 }).debounceUpdateValue(e.detail), + value: (e: CustomEvent) => widgetValueCommitAndUpdate(index, e.detail, false), }, }), }, @@ -202,7 +201,7 @@ incrementCallbackIncrease: () => widgetValueCommitAndUpdate(index, "Increment", false), incrementCallbackDecrease: () => widgetValueCommitAndUpdate(index, "Decrement", false), $$events: { - value: (e: CustomEvent) => debouncer((value: unknown) => widgetValueUpdate(index, value, true)).debounceUpdateValue(e.detail), + value: (e: CustomEvent) => widgetValueUpdate(index, e.detail, true), startHistoryTransaction: () => widgetValueCommit(index, props.value), }, }), diff --git a/frontend/src/utility-functions/debounce.ts b/frontend/src/utility-functions/debounce.ts deleted file mode 100644 index 8eb8fa4076..0000000000 --- a/frontend/src/utility-functions/debounce.ts +++ /dev/null @@ -1,30 +0,0 @@ -export type Debouncer = ReturnType; - -export type DebouncerOptions = { - debounceTime: number; -}; - -export function debouncer(callFn: (value: T) => unknown, { debounceTime = 60 }: Partial = {}) { - let currentValue: T | undefined; - let recentlyUpdated: boolean = false; - - const debounceEmitValue = () => { - recentlyUpdated = false; - if (currentValue === undefined) return; - debounceUpdateValue(currentValue); - }; - - const debounceUpdateValue = (newValue: T) => { - if (recentlyUpdated) { - currentValue = newValue; - return; - } - - callFn(newValue); - recentlyUpdated = true; - currentValue = undefined; - setTimeout(debounceEmitValue, debounceTime); - }; - - return { debounceUpdateValue }; -} From 08bebb20b843489a14afe6423d84340ca36b65ad Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Sun, 8 Mar 2026 16:20:15 -0700 Subject: [PATCH 4/8] Clean up even more component setup/tear-down side effects --- frontend/src/App.svelte | 3 +- .../floating-menus/EyedropperPreview.svelte | 10 +++-- .../components/floating-menus/MenuList.svelte | 5 ++- .../src/components/layout/FloatingMenu.svelte | 25 +++++++----- .../src/components/panels/Document.svelte | 10 ++++- .../widgets/inputs/NumberInput.svelte | 7 ++++ .../widgets/labels/TextLabel.svelte | 5 ++- .../src/components/window/Workspace.svelte | 9 ++++- frontend/src/editor.ts | 11 +++++- frontend/src/io-managers/fonts.ts | 39 ++++++++++++------- frontend/src/io-managers/panic.ts | 2 + frontend/src/main.ts | 9 ++++- frontend/src/subscription-router.ts | 24 +++++++----- 13 files changed, 109 insertions(+), 50 deletions(-) diff --git a/frontend/src/App.svelte b/frontend/src/App.svelte index 6e98a39bc5..55ff5d4d9d 100644 --- a/frontend/src/App.svelte +++ b/frontend/src/App.svelte @@ -15,8 +15,7 @@ }); onDestroy(() => { - // Destroy the Wasm editor handle - editor?.handle.free(); + editor?.destroy(); }); diff --git a/frontend/src/components/floating-menus/EyedropperPreview.svelte b/frontend/src/components/floating-menus/EyedropperPreview.svelte index e7d8744920..2457ceee35 100644 --- a/frontend/src/components/floating-menus/EyedropperPreview.svelte +++ b/frontend/src/components/floating-menus/EyedropperPreview.svelte @@ -12,10 +12,7 @@ import FloatingMenu from "@graphite/components/layout/FloatingMenu.svelte"; - const temporaryCanvas = document.createElement("canvas"); - temporaryCanvas.width = ZOOM_WINDOW_DIMENSIONS; - temporaryCanvas.height = ZOOM_WINDOW_DIMENSIONS; - + let temporaryCanvas: HTMLCanvasElement | undefined; let zoomPreviewCanvas: HTMLCanvasElement | undefined; export let imageData: ImageData | undefined = undefined; @@ -31,6 +28,11 @@ if (!zoomPreviewCanvas) return; const context = zoomPreviewCanvas.getContext("2d"); + if (!temporaryCanvas) { + temporaryCanvas = document.createElement("canvas"); + temporaryCanvas.width = ZOOM_WINDOW_DIMENSIONS; + temporaryCanvas.height = ZOOM_WINDOW_DIMENSIONS; + } const temporaryContext = temporaryCanvas.getContext("2d"); if (!imageData || !context || !temporaryContext) return; diff --git a/frontend/src/components/floating-menus/MenuList.svelte b/frontend/src/components/floating-menus/MenuList.svelte index d7f042f5d0..6c639f90ff 100644 --- a/frontend/src/components/floating-menus/MenuList.svelte +++ b/frontend/src/components/floating-menus/MenuList.svelte @@ -48,6 +48,7 @@ let highlighted: MenuListEntry | undefined = activeEntry; let virtualScrollingEntriesStart = 0; let keydownListenerAdded = false; + let destroyed = false; // `watchOpen` is called only when `open` is changed from outside this component $: watchOpen(open); @@ -68,13 +69,15 @@ // TODO: The current approach is hacky and blocks the allowances for shortcuts like the key to open the browser's dev tools. onMount(async () => { await tick(); - if (open && !inNestedMenuList() && !keydownListenerAdded) { + if (!destroyed && open && !inNestedMenuList() && !keydownListenerAdded) { addEventListener("keydown", keydown); keydownListenerAdded = true; } }); onDestroy(() => { removeEventListener("keydown", keydown); + // Set the destroyed status in the closure kept by the awaited `tick()` in `onMount` in case that delayed run occurs after the component is destroyed + destroyed = true; }); function inNestedMenuList(): boolean { diff --git a/frontend/src/components/layout/FloatingMenu.svelte b/frontend/src/components/layout/FloatingMenu.svelte index 4dece0b91e..c3cd7932fb 100644 --- a/frontend/src/components/layout/FloatingMenu.svelte +++ b/frontend/src/components/layout/FloatingMenu.svelte @@ -81,16 +81,7 @@ // Called only when `open` is changed from outside this component async function watchOpenChange(isOpen: boolean) { - // Mitigate a Safari rendering bug which clips the floating menu extending beyond a scrollable container. - // The bug is possibly related to , but in our case it happens when `overflow` of a parent is `auto` rather than `hidden`. - if (browserVersion().toLowerCase().includes("safari")) { - const scrollable = self?.closest("[data-scrollable-x], [data-scrollable-y]"); - if (scrollable instanceof HTMLElement) { - // The issue exists when the container is set to `overflow: auto` but fine when `overflow: hidden`. So this workaround temporarily sets - // the scrollable container to `overflow: hidden`, thus removing the scrollbars and ability to scroll until the floating menu is closed. - scrollable.style.overflow = isOpen ? "hidden" : ""; - } - } + setSafariScrollableOverflow(isOpen); // Switching from closed to open if (isOpen && !wasOpen) { @@ -131,6 +122,16 @@ wasOpen = isOpen; } + // Mitigate a Safari rendering bug which clips the floating menu extending beyond a scrollable container. The bug is possibly related to + // , but in our case it happens when `overflow` of a parent is `auto` rather than `hidden`. + // The issue exists when the container is set to `overflow: auto` but fine when `overflow: hidden`. So this workaround temporarily sets + // the scrollable container to `overflow: hidden`, thus removing the scrollbars and ability to scroll until the floating menu is closed. + function setSafariScrollableOverflow(hidden: boolean) { + if (!browserVersion().toLowerCase().includes("safari")) return; + const scrollable = self?.closest("[data-scrollable-x], [data-scrollable-y]"); + if (scrollable instanceof HTMLElement) scrollable.style.overflow = hidden ? "hidden" : ""; + } + onMount(() => { // Measure the content and round up its width and height to the nearest even integer. // This solves antialiasing issues when the content isn't cleanly divisible by 2 and gets translated by (-50%, -50%) causing all its content to be blurry. @@ -166,6 +167,10 @@ window.removeEventListener("keydown", keyDownHandler); window.removeEventListener("pointerdown", pointerDownHandler); window.removeEventListener("pointerup", pointerUpHandler); + window.removeEventListener("click", clickHandlerCapture, true); + + // Revert Safari overflow workaround if the menu was open when destroyed + if (open) setSafariScrollableOverflow(false); }); afterUpdate(() => { diff --git a/frontend/src/components/panels/Document.svelte b/frontend/src/components/panels/Document.svelte index 45fb941bf2..165ec7afe8 100644 --- a/frontend/src/components/panels/Document.svelte +++ b/frontend/src/components/panels/Document.svelte @@ -77,6 +77,7 @@ let removeUpdatePixelRatio: (() => void) | undefined; let viewportResizeObserver: ResizeObserver | undefined; let cleanupViewportResizeObserver: (() => void) | undefined; + let addedFontFaces: FontFace[] = []; // Dimension is rounded up to the nearest even number because resizing is centered, and dividing an odd number by 2 for centering causes antialiasing $: canvasWidthRoundedToEven = canvasWidth && (canvasWidth % 2 === 1 ? canvasWidth + 1 : canvasWidth); @@ -378,7 +379,9 @@ if (data.fontData.length > 0 && data.fontData.buffer instanceof ArrayBuffer) { const fontView = new Uint8Array(data.fontData.buffer, data.fontData.byteOffset, data.fontData.byteLength); - window.document.fonts.add(new FontFace("text-font", fontView)); + const face = new FontFace("text-font", fontView); + window.document.fonts.add(face); + addedFontFaces.push(face); textInput.style.fontFamily = "text-font"; } @@ -512,7 +515,9 @@ if (textInput && data.fontData.length > 0 && data.fontData.buffer instanceof ArrayBuffer) { const fontView = new Uint8Array(data.fontData.buffer, data.fontData.byteOffset, data.fontData.byteLength); - window.document.fonts.add(new FontFace("text-font", fontView)); + const face = new FontFace("text-font", fontView); + window.document.fonts.add(face); + addedFontFaces.push(face); textInput.style.fontFamily = "text-font"; } }); @@ -540,6 +545,7 @@ cleanupViewportResizeObserver?.(); viewportResizeObserver?.disconnect(); removeUpdatePixelRatio?.(); + addedFontFaces.forEach((face) => window.document.fonts.delete(face)); editor.subscriptions.unsubscribeFrontendMessage("UpdateDocumentArtwork"); editor.subscriptions.unsubscribeFrontendMessage("UpdateEyedropperSamplingState"); diff --git a/frontend/src/components/widgets/inputs/NumberInput.svelte b/frontend/src/components/widgets/inputs/NumberInput.svelte index dec0de773a..ef3a6d6164 100644 --- a/frontend/src/components/widgets/inputs/NumberInput.svelte +++ b/frontend/src/components/widgets/inputs/NumberInput.svelte @@ -119,6 +119,13 @@ activeDragCleanup?.(); + // Exit pointer lock if active (non-Safari path) + if (document.pointerLockElement) document.exitPointerLock(); + + // Remove Safari cursor-hidden workaround class if present + const isSafari = browserVersion().toLowerCase().includes("safari"); + if (isSafari) document.body.classList.remove("cursor-hidden"); + // Clean up any listeners related to tracking the Shift and Ctrl keys removeEventListener("keydown", trackShiftAndCtrl); removeEventListener("keyup", trackShiftAndCtrl); diff --git a/frontend/src/components/widgets/labels/TextLabel.svelte b/frontend/src/components/widgets/labels/TextLabel.svelte index e0ac6c2e4b..558e16d578 100644 --- a/frontend/src/components/widgets/labels/TextLabel.svelte +++ b/frontend/src/components/widgets/labels/TextLabel.svelte @@ -62,7 +62,10 @@ } onMount(() => watchForCheckbox(forCheckbox)); - onDestroy(() => watchForCheckbox(undefined)); + onDestroy(() => { + handlePointerLeave(); + watchForCheckbox(undefined); + });