From 4b07ab9f0ec2ecbb885f1266c42ba46ce3225ef3 Mon Sep 17 00:00:00 2001 From: u8array Date: Fri, 16 Oct 2026 01:48:45 +0200 Subject: [PATCH] chore(lint): warnings fail the run and the compiler lowers every hook --- eslint.config.js | 9 +- package.json | 2 +- src/components/Canvas/LabelCanvas.tsx | 9 +- .../Canvas/hooks/useKonvaTransformer.ts | 90 ++++++++++--------- src/hooks/useMcpBridge.ts | 9 +- src/hooks/useNativeMenu.ts | 10 ++- src/hooks/useServerDbConnectActions.ts | 4 +- src/lib/tryFinally.ts | 9 ++ 8 files changed, 76 insertions(+), 66 deletions(-) create mode 100644 src/lib/tryFinally.ts diff --git a/eslint.config.js b/eslint.config.js index 9e53a8e2..2169cafb 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -22,17 +22,16 @@ export default defineConfig([ }, rules: { '@typescript-eslint/no-explicit-any': 'error', - '@typescript-eslint/no-non-null-assertion': 'warn', + '@typescript-eslint/no-non-null-assertion': 'error', '@typescript-eslint/consistent-type-imports': ['error', { prefer: 'type-imports' }], // Omit-via-rest (`const { drop, ...keep } = obj`) is the intended pattern. '@typescript-eslint/no-unused-vars': ['error', { ignoreRestSiblings: true }], - // Syntax the compiler cannot lower, so the enclosing function silently loses its memoization. - // Warn, not error: the hooks that predate the rule are their own cleanup scope. - 'react-hooks/todo': 'warn', + // Syntax the compiler cannot lower, so the whole hook silently loses its memoization. + 'react-hooks/todo': 'error', }, }, { - files: ['**/*.test.ts', '**/*.test.tsx', 'src/test/**'], + files: ['**/*.test.ts', '**/*.test.tsx', 'src/test/**', 'tests/**/*.ts', 'e2e/**/*.ts'], rules: { '@typescript-eslint/no-non-null-assertion': 'off', }, diff --git a/package.json b/package.json index 36595a11..661e4db5 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,7 @@ "test": "vitest run", "test:watch": "vitest", "test:coverage": "vitest run --coverage", - "lint": "eslint .", + "lint": "eslint . --max-warnings 0", "licenses:check": "node scripts/check-licenses.mjs", "attribution:gen": "node scripts/gen-attribution.mjs", "attribution:check": "node scripts/gen-attribution.mjs --check", diff --git a/src/components/Canvas/LabelCanvas.tsx b/src/components/Canvas/LabelCanvas.tsx index 6a78de0d..bed345c3 100644 --- a/src/components/Canvas/LabelCanvas.tsx +++ b/src/components/Canvas/LabelCanvas.tsx @@ -1,3 +1,4 @@ +import { tryFinally } from "../../lib/tryFinally"; import { forwardRef, useImperativeHandle, @@ -1827,13 +1828,7 @@ export const LabelCanvas = forwardRef(function LabelCa }} onTransform={onTransform} boundBoxFunc={boundBoxFunc} - onTransformEnd={() => { - try { - onTransformEnd(); - } finally { - transformActiveRef.current = false; - } - }} + onTransformEnd={() => tryFinally(onTransformEnd, () => { transformActiveRef.current = false; })} borderStroke={colors.selection} anchorStroke={colors.selection} anchorFill="#ffffff" diff --git a/src/components/Canvas/hooks/useKonvaTransformer.ts b/src/components/Canvas/hooks/useKonvaTransformer.ts index c8fbcf6e..ddb345d3 100644 --- a/src/components/Canvas/hooks/useKonvaTransformer.ts +++ b/src/components/Canvas/hooks/useKonvaTransformer.ts @@ -1,4 +1,5 @@ import { editBounds, moduleScaledProps, wireBounds, type Range } from "@zplab/core/types/propSpec"; +import { tryFinally } from "../../../lib/tryFinally"; import { useRef, useEffect, useLayoutEffect } from "react"; import { flushSync } from "react-dom"; import type Konva from "konva"; @@ -553,16 +554,19 @@ export function useKonvaTransformer({ node?.scaleX(1); node?.scaleY(1); const temporal = useLabelStore.temporal.getState(); - try { - if (id && commit) { - updateObject(id, commit.restore); + tryFinally( + () => { + if (id && commit) { + updateObject(id, commit.restore); + temporal.resume(); + updateObject(id, commit.final); + } + }, + () => { temporal.resume(); - updateObject(id, commit.final); - } - } finally { - temporal.resume(); - cleanupTransformState(); - } + cleanupTransformState(); + }, + ); } const onTransformStart = () => { @@ -1568,40 +1572,42 @@ export function useKonvaTransformer({ blockJustify?: BlockJustify; }; const temporal = useLabelStore.temporal.getState(); - // try/finally so a throw from flushSync/getClientRect can't leave undo - // recording globally paused (resume is idempotent). + // A throw from flushSync or getClientRect must not leave undo recording paused. temporal.pause(); - try { - // Konva left the node at the drag position; the font-only commit does - // not change x/y so react-konva won't reset it. Move it back to the - // model origin so the re-measure reflects the block at obj.x/y. - node.x(objectsOffsetX + dotsToPx(obj.x, scale, dpmm)); - node.y(labelOffsetY + dotsToPx(obj.y, scale, dpmm)); - flushSync(() => updateObject(singleId, { props: propChanges })); - const newRect = stageRect(node, stageRef.current); - const edges = activeEdgesRef.current; - const justify = fp.blockJustify ?? "L"; - const anchorArgs = { rotation: fp.rotation, justify, edges, centeredStacking: centeredScaling }; - const startA = blockGlyphAnchorPoint({ rect: start, ...anchorArgs }); - const newA = blockGlyphAnchorPoint({ rect: newRect, ...anchorArgs }); - const model = modelPositionFromRenderedTopLeft( - obj, - pxToDots(node.x() + (startA.x - newA.x) - objectsOffsetX, scale, dpmm), - pxToDots(node.y() + (startA.y - newA.y) - labelOffsetY, scale, dpmm), - ); - // Restore pre-drag state while paused, resume, then apply the final as - // one tracked change so undo records pre-drag -> final once. - updateObject(singleId, { - x: obj.x, - y: obj.y, - props: { fontWidth: fp.fontWidth, fontHeight: fp.fontHeight }, - }); - temporal.resume(); - updateObject(singleId, { x: model.x, y: model.y, props: propChanges }); - } finally { - temporal.resume(); - cleanupTransformState(); - } + tryFinally( + () => { + // Konva left the node at the drag position; the font-only commit does + // not change x/y so react-konva won't reset it. Move it back to the + // model origin so the re-measure reflects the block at obj.x/y. + node.x(objectsOffsetX + dotsToPx(obj.x, scale, dpmm)); + node.y(labelOffsetY + dotsToPx(obj.y, scale, dpmm)); + flushSync(() => updateObject(singleId, { props: propChanges })); + const newRect = stageRect(node, stageRef.current); + const edges = activeEdgesRef.current; + const justify = fp.blockJustify ?? "L"; + const anchorArgs = { rotation: fp.rotation, justify, edges, centeredStacking: centeredScaling }; + const startA = blockGlyphAnchorPoint({ rect: start, ...anchorArgs }); + const newA = blockGlyphAnchorPoint({ rect: newRect, ...anchorArgs }); + const model = modelPositionFromRenderedTopLeft( + obj, + pxToDots(node.x() + (startA.x - newA.x) - objectsOffsetX, scale, dpmm), + pxToDots(node.y() + (startA.y - newA.y) - labelOffsetY, scale, dpmm), + ); + // Restore pre-drag state while paused, resume, then apply the final as + // one tracked change so undo records pre-drag -> final once. + updateObject(singleId, { + x: obj.x, + y: obj.y, + props: { fontWidth: fp.fontWidth, fontHeight: fp.fontHeight }, + }); + temporal.resume(); + updateObject(singleId, { x: model.x, y: model.y, props: propChanges }); + }, + () => { + temporal.resume(); + cleanupTransformState(); + }, + ); return; } updateObject(singleId, { ...pos, props: propChanges }); diff --git a/src/hooks/useMcpBridge.ts b/src/hooks/useMcpBridge.ts index 2d0937e7..8c924a93 100644 --- a/src/hooks/useMcpBridge.ts +++ b/src/hooks/useMcpBridge.ts @@ -13,6 +13,10 @@ import { } from "../lib/mcpServer"; import { isDesktopShell } from "../lib/platform"; +// Module level, since the compiler cannot lower an import expression inside the hook. +const tauriEvent = () => import("@tauri-apps/api/event"); +const tauriCore = () => import("@tauri-apps/api/core"); + /** Register the sidecar listeners, then have Rust flush the events it * queued while none existed (see event_buffer in mcp.rs). */ export function useMcpBridge(): void { @@ -24,10 +28,7 @@ export function useMcpBridge(): void { // teardown withdraws its own and never the one that replaced it. let attachedSession: string | null = null; void (async () => { - const [{ listen }, { invoke }] = await Promise.all([ - import("@tauri-apps/api/event"), - import("@tauri-apps/api/core"), - ]); + const [{ listen }, { invoke }] = await Promise.all([tauriEvent(), tauriCore()]); const fns = await Promise.all([ listen<[number, string]>("mcp://open-draft", (e) => { void respondToOpenDraft(e.payload[0], e.payload[1]).catch(() => undefined); diff --git a/src/hooks/useNativeMenu.ts b/src/hooks/useNativeMenu.ts index d5b34f19..d8d12c16 100644 --- a/src/hooks/useNativeMenu.ts +++ b/src/hooks/useNativeMenu.ts @@ -93,6 +93,8 @@ let latest: MenuData | null = null; let installed: InstalledMenu | null = null; let updateGen = 0; let queue: Promise = Promise.resolve(); +// A function, since the compiler cannot lower ++ on a module variable inside the hook. +const nextUpdateGen = (): number => ++updateGen; /** The OS toggles a clicked CheckMenuItem's checkmark itself; after a jump we * re-assert every step's checkmark so that stray toggle can't linger. */ let checkmarkDirty = false; @@ -104,7 +106,7 @@ if (import.meta.hot) { import.meta.hot.dispose(() => { // Invalidate queued rebuilds and drain the in-flight update before closing, // so a mid-await patch/rebuild can't touch freed handles. - updateGen += 1; + nextUpdateGen(); queue = queue.then(() => { if (installed) closeAll(installed.resources); installed = null; @@ -384,7 +386,7 @@ async function patchMenu(inst: InstalledMenu, d: MenuData): Promise { // for the next unrelated signature change. if (installed) closeAll(installed.resources); installed = null; - const gen = ++updateGen; + const gen = nextUpdateGen(); queue = queue.then(() => runUpdate(gen)).catch(() => undefined); } } @@ -434,12 +436,12 @@ export function useNativeMenu( useEffect(() => { if (!isDesktopShell) return; - const gen = ++updateGen; + const gen = nextUpdateGen(); queue = queue.then(() => runUpdate(gen)).catch(() => undefined); // Invalidate a queued-but-unstarted update on re-run/unmount (StrictMode // remounts included); the re-run queues a fresh one. return () => { - updateGen += 1; + nextUpdateGen(); }; }, [signature]); } diff --git a/src/hooks/useServerDbConnectActions.ts b/src/hooks/useServerDbConnectActions.ts index 5dfeae78..ff30e903 100644 --- a/src/hooks/useServerDbConnectActions.ts +++ b/src/hooks/useServerDbConnectActions.ts @@ -94,10 +94,8 @@ export function useServerDbConnectActions(driver: 'postgres' | 'mysql') { if (draftEpoch.current === epoch) setTables(listed); } catch (e) { if (draftEpoch.current === epoch) setListError(String(e)); - } finally { - setBusy(false); } - })(); + })().finally(() => setBusy(false)); }; /** Run a load attempt via `run` (the wizard marks a success as its own load) diff --git a/src/lib/tryFinally.ts b/src/lib/tryFinally.ts new file mode 100644 index 00000000..c7b66d45 --- /dev/null +++ b/src/lib/tryFinally.ts @@ -0,0 +1,9 @@ +/** The React Compiler cannot lower a try/finally inside a hook, so the finally lives here. + * `body` is a closure, so a return inside it leaves only the closure. */ +export function tryFinally(body: () => void, cleanup: () => void): void { + try { + body(); + } finally { + cleanup(); + } +}