Break out helper functions from the frontend's managers and stores

[Keavon]

No description provided.

[cubic-dev-ai]

7 issues found across 19 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="frontend/src/utility-functions/time.ts">

<violation number="1" location="frontend/src/utility-functions/time.ts:7">
P2: Timezone name is computed from the current date instead of the input timestamp, which can produce incorrect labels across DST boundaries.</violation>
</file>

<file name="frontend/src/utility-functions/clipboard.ts">

<violation number="1" location="frontend/src/utility-functions/clipboard.ts:101">
P1: Using `Promise.any()` here can return `false` early from the first unsupported item and incorrectly fail paste even when a later clipboard item is valid.</violation>

<violation number="2" location="frontend/src/utility-functions/clipboard.ts:105">
P1: The SVG branch requests `text/plain` instead of the detected SVG MIME type, which can fail when plain text is unavailable.</violation>
</file>

<file name="frontend/src/utility-functions/persistence.ts">

<violation number="1" location="frontend/src/utility-functions/persistence.ts:9">
P1: `createStore` opens a new IndexedDB connection on each call. It should be called once at module level and reused across all functions. Repeated calls waste connections and can cause race conditions (documented in idb-keyval#178) when concurrent operations overlap.</violation>

<violation number="2" location="frontend/src/utility-functions/persistence.ts:73">
P2: The `documentId` string→bigint migration block is duplicated in three functions. Extract it into a shared helper (e.g., `migrateDocumentIds`) so the TODO removal is a single-site change and the migration logic stays consistent.</violation>
</file>

<file name="frontend/src/utility-functions/input.ts">

<violation number="1" location="frontend/src/utility-functions/input.ts:250">
P1: The `async` `beforeunload` handler sets `e.returnValue` and calls `e.preventDefault()` after `await` statements. Browsers process `beforeunload` synchronously — code after `await` is deferred to the microtask queue and the browser may unload the page before the unsaved-changes dialog is triggered. Either make the checks synchronous or move `e.preventDefault()` before the first `await` and cancel it if the early-out conditions apply.</violation>

<violation number="2" location="frontend/src/utility-functions/input.ts:272">
P2: `forEach` does not await or catch async callbacks. If `pasteFile` rejects, the error becomes an unhandled promise rejection. Use a `for...of` loop instead, or collect the promises and handle them with `Promise.all`.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[cubic-dev-ai]

P1: The SVG branch requests text/plain instead of the detected SVG MIME type, which can fail when plain text is unavailable.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/utility-functions/clipboard.ts, line 105:

<comment>The SVG branch requests `text/plain` instead of the detected SVG MIME type, which can fail when plain text is unavailable.</comment>

<file context>
@@ -0,0 +1,175 @@
+			Array.from(clipboardItems).map(async (item) => {
+				// Read plain text and, if it is a layer, pass it to the editor
+				if (item.types.includes("text/plain")) {
+					const blob = await item.getType("text/plain");
+					const reader = new FileReader();
+					reader.onload = () => {
</file context>

Suggested change

	const blob = await item.getType("text/plain");
	const blob = await item.getType(imageType);

[cubic-dev-ai]

P1: Using Promise.any() here can return false early from the first unsupported item and incorrectly fail paste even when a later clipboard item is valid.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/utility-functions/clipboard.ts, line 101:

<comment>Using `Promise.any()` here can return `false` early from the first unsupported item and incorrectly fail paste even when a later clipboard item is valid.</comment>

<file context>
@@ -0,0 +1,175 @@
+		if (!clipboardItems) throw new Error("Clipboard API unsupported");
+
+		// Read any layer data or images from the clipboard
+		const success = await Promise.any(
+			Array.from(clipboardItems).map(async (item) => {
+				// Read plain text and, if it is a layer, pass it to the editor
</file context>

[cubic-dev-ai]

P1: createStore opens a new IndexedDB connection on each call. It should be called once at module level and reused across all functions. Repeated calls waste connections and can cause race conditions (documented in idb-keyval#178) when concurrent operations overlap.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/utility-functions/persistence.ts, line 9:

<comment>`createStore` opens a new IndexedDB connection on each call. It should be called once at module level and reused across all functions. Repeated calls waste connections and can cause race conditions (documented in idb-keyval#178) when concurrent operations overlap.</comment>

<file context>
@@ -0,0 +1,193 @@
+import type { MessageBody } from "@graphite/subscription-router";
+
+export async function storeCurrentDocumentId(documentId: string) {
+	const indexedDbStorage = idb.createStore("graphite", "store");
+
+	await idb.set("current_document_id", String(documentId), indexedDbStorage);
</file context>

[cubic-dev-ai]

P1: The async beforeunload handler sets e.returnValue and calls e.preventDefault() after await statements. Browsers process beforeunload synchronously — code after await is deferred to the microtask queue and the browser may unload the page before the unsaved-changes dialog is triggered. Either make the checks synchronous or move e.preventDefault() before the first await and cancel it if the early-out conditions apply.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/utility-functions/input.ts, line 250:

<comment>The `async` `beforeunload` handler sets `e.returnValue` and calls `e.preventDefault()` after `await` statements. Browsers process `beforeunload` synchronously — code after `await` is deferred to the microtask queue and the browser may unload the page before the unsaved-changes dialog is triggered. Either make the checks synchronous or move `e.preventDefault()` before the first `await` and cancel it if the early-out conditions apply.</comment>

<file context>
@@ -0,0 +1,362 @@
+
+// Window events
+
+export async function onBeforeUnload(e: BeforeUnloadEvent, editor: Editor, portfolioStore: PortfolioStore) {
+	const activeDocument = get(portfolioStore).documents[get(portfolioStore).activeDocumentIndex];
+	if (activeDocument && !activeDocument.details.isAutoSaved) editor.handle.triggerAutoSave(activeDocument.id);
</file context>

[cubic-dev-ai]

P2: Timezone name is computed from the current date instead of the input timestamp, which can produce incorrect labels across DST boundaries.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/utility-functions/time.ts, line 7:

<comment>Timezone name is computed from the current date instead of the input timestamp, which can produce incorrect labels across DST boundaries.</comment>

<file context>
@@ -0,0 +1,14 @@
+	if (Number.isNaN(date.getTime())) return { timestamp: utc, year: `${new Date().getFullYear()}` };
+
+	const timezoneName = Intl.DateTimeFormat(undefined, { timeZoneName: "longGeneric" })
+		.formatToParts(new Date())
+		.find((part) => part.type === "timeZoneName");
+
</file context>

[cubic-dev-ai]

P2: The documentId string→bigint migration block is duplicated in three functions. Extract it into a shared helper (e.g., migrateDocumentIds) so the TODO removal is a single-site change and the migration logic stays consistent.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/utility-functions/persistence.ts, line 73:

<comment>The `documentId` string→bigint migration block is duplicated in three functions. Extract it into a shared helper (e.g., `migrateDocumentIds`) so the TODO removal is a single-site change and the migration logic stays consistent.</comment>

<file context>
@@ -0,0 +1,193 @@
+
+	const previouslySavedDocuments = await idb.get<Record<string, MessageBody<"TriggerPersistenceWriteDocument">>>("documents", indexedDbStorage);
+
+	// TODO: Eventually remove this document upgrade code
+	// Migrate TriggerPersistenceWriteDocument.documentId from string to bigint if the browser is storing the old format as strings
+	if (previouslySavedDocuments) {
</file context>

[cubic-dev-ai]

P2: forEach does not await or catch async callbacks. If pasteFile rejects, the error becomes an unhandled promise rejection. Use a for...of loop instead, or collect the promises and handle them with Promise.all.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/utility-functions/input.ts, line 272:

<comment>`forEach` does not await or catch async callbacks. If `pasteFile` rejects, the error becomes an unhandled promise rejection. Use a `for...of` loop instead, or collect the promises and handle them with `Promise.all`.</comment>

<file context>
@@ -0,0 +1,362 @@
+	if (!dataTransfer || targetIsTextField(e.target || undefined)) return;
+	e.preventDefault();
+
+	Array.from(dataTransfer.items).forEach(async (item) => {
+		if (item.type === "text/plain") item.getAsString((text) => editor.handle.pasteText(text));
+		await pasteFile(item, editor);
</file context>

[Keavon]

/gemini summary

[gemini-code-assist]

Summary of Changes

This pull request focuses on a significant frontend cleanup and refactoring effort. The primary goal was to enhance code organization and maintainability by extracting various utility functions and event handling logic from core manager and store files into dedicated utility-functions modules. This change also standardizes the initialization and destruction patterns for managers and stores, particularly improving their behavior with Hot Module Replacement.

Highlights

• Refactored Manager and Store Initialization: Implemented destroyManager() calls at the beginning of createManager() functions across various managers and stores (e.g., clipboard, fonts, hyperlink, localization, panic, persistence, app-window, dialog, document, fullscreen, node-graph, portfolio, tooltip). This ensures proper cleanup and re-initialization, especially beneficial for Hot Module Replacement (HMR) during development.
• Extracted Utility Functions: Moved common and reusable functions related to clipboard operations, input event handling, persistence logic, and time localization into dedicated utility-functions files. This significantly improves modularity, reduces code duplication, and centralizes concerns.
• Centralized Input Event Handling: Consolidated all global input event listeners and their associated state from input.ts into a new utility-functions/input.ts file. This makes the input management more organized and easier to maintain.

Changelog

• frontend/src/components/floating-menus/Dialog.svelte
  • Updated the import path for wipeDocuments to reflect its new location in utility-functions/persistence.
• frontend/src/managers/clipboard.ts
  • Moved readAtCaret, insertAtCaret, and triggerClipboardRead functions to utility-functions/clipboard.ts.
  • Updated imports to use the new utility functions.
  • Added destroyClipboardManager() call at the start of createClipboardManager() for proper re-initialization.
  • Removed redundant destroyClipboardManager() call from the HMR accept block.
• frontend/src/managers/fonts.ts
  • Added destroyFontsManager() call at the start of createFontsManager().
  • Removed redundant destroyFontsManager() call from the HMR accept block.
• frontend/src/managers/hyperlink.ts
  • Added destroyHyperlinkManager() call at the start of createHyperlinkManager().
  • Removed redundant destroyHyperlinkManager() call from the HMR accept block.
• frontend/src/managers/input.ts
  • Moved numerous input event handler functions (e.g., onBeforeUnload, onKeyUp, onPointerMove) and related state to utility-functions/input.ts.
  • Updated the listeners array to reference the newly imported utility functions and pass necessary store references.
  • Inlined the logic of bindListeners and unbindListeners directly into createInputManager and destroyInputManager.
  • Added destroyInputManager() call at the start of createInputManager().
  • Updated triggerClipboardRead call to pass the editor instance.
• frontend/src/managers/localization.ts
  • Moved localizeTimestamp function to utility-functions/time.ts.
  • Updated imports to use the new utility function.
  • Added destroyLocalizationManager() call at the start of createLocalizationManager().
  • Removed redundant destroyLocalizationManager() call from the HMR accept block.
• frontend/src/managers/panic.ts
  • Added destroyPanicManager() call at the start of createPanicManager().
  • Removed redundant destroyPanicManager() call from the HMR accept block.
• frontend/src/managers/persistence.ts
  • Moved persistence-related functions (e.g., storeCurrentDocumentId, storeDocument, loadFirstDocument) to utility-functions/persistence.ts.
  • Updated imports to use the new utility functions.
  • Added destroyPersistenceManager() call at the start of createPersistenceManager().
  • Removed redundant destroyPersistenceManager() call from the HMR accept block.
• frontend/src/stores/app-window.ts
  • Added destroyAppWindowStore() call at the start of createAppWindowStore().
• frontend/src/stores/dialog.ts
  • Added destroyDialogStore() call at the start of createDialogStore().
  • Formatted subscription calls for consistency.
• frontend/src/stores/document.ts
  • Added destroyDocumentStore() call at the start of createDocumentStore().
  • Formatted subscription calls for consistency.
• frontend/src/stores/fullscreen.ts
  • Added destroyFullscreenStore() call at the start of createFullscreenStore().
• frontend/src/stores/node-graph.ts
  • Added destroyNodeGraphStore() call at the start of createNodeGraphStore().
• frontend/src/stores/portfolio.ts
  • Added destroyPortfolioStore() call at the start of createPortfolioStore().
• frontend/src/stores/tooltip.ts
  • Moved tooltipTimeout to a module-level variable.
  • Refactored tooltip event listeners into a dedicated array and managed their addition/removal using forEach.
  • Added destroyTooltipStore() call at the start of createTooltipStore().
• frontend/src/utility-functions/clipboard.ts
  • Added new file containing readAtCaret, insertAtCaret, and triggerClipboardRead functions.
• frontend/src/utility-functions/input.ts
  • Added new file containing common input event handlers and related state variables previously in managers/input.ts.
• frontend/src/utility-functions/persistence.ts
  • Added new file containing IndexedDB persistence logic for documents and preferences, including storeCurrentDocumentId, storeDocument, removeDocument, loadFirstDocument, loadRestDocuments, saveActiveDocument, saveEditorPreferences, loadEditorPreferences, and wipeDocuments.
• frontend/src/utility-functions/time.ts
  • Added new file containing the localizeTimestamp function.

Activity

• A bot attempted to generate a summary but encountered an error.
• The author, Keavon, manually requested a summary from the bot.
• Review comments from cubic-dev-ai[bot] highlighted several potential issues in the newly created utility files:
• In frontend/src/utility-functions/clipboard.ts:
• The SVG branch incorrectly requests text/plain instead of the detected SVG MIME type, which could lead to failures.
• The use of Promise.any() might prematurely fail clipboard paste operations if an unsupported item is encountered before a valid one.
• In frontend/src/utility-functions/persistence.ts:
• The idb-keyval createStore function is called repeatedly, potentially causing race conditions and wasting IndexedDB connections.
• The documentId string-to-bigint migration logic is duplicated across multiple functions, suggesting a need for a shared helper.
• In frontend/src/utility-functions/input.ts:
• The async onBeforeUnload handler's e.returnValue and e.preventDefault() calls occur after await statements, which might prevent the browser from displaying the unsaved changes dialog synchronously.
• The forEach loop in onPaste does not properly handle async callbacks, potentially leading to unhandled promise rejections if pasteFile fails.
• In frontend/src/utility-functions/time.ts:
• The localizeTimestamp function computes the timezone name from the current date instead of the input timestamp, which could result in incorrect labels across Daylight Saving Time (DST) boundaries.