fix(canvas): prune lastFitSubtreeIdsRef on stale roots (#2070)
Closes #2070. The Map<rootId, Set<nodeId>> in useCanvasViewport.ts accumulated entries indefinitely — adds on every successful auto-fit, never deletes when a root left state.nodes (cascade delete or manual remove). Operationally invisible until thousands of imports, but the fix is cheap. Adds pruneStaleSubtreeIds(map, liveNodeIds) — a pure helper exported alongside the existing shouldFitGrowing helper, called at the top of runFit before any read or write to the map. Bounds the map to "roots present right now" instead of "every root ever auto-fitted in this session." O(map_size) per fit; runs only at user-driven cadence. Tests in __tests__/useCanvasViewport.test.ts cover the four cases: delete-some / no-op / clear-all / never-add. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
b0a33d9ebf
commit
69edc0bf92
@ -1,5 +1,5 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { shouldFitGrowing } from "../useCanvasViewport";
|
||||
import { pruneStaleSubtreeIds, shouldFitGrowing } from "../useCanvasViewport";
|
||||
|
||||
// Tests cover the auto-fit gate in isolation. The hook itself is
|
||||
// effects + refs + React Flow handles, awkward to exercise directly —
|
||||
@ -51,3 +51,38 @@ describe("shouldFitGrowing", () => {
|
||||
expect(shouldFitGrowing(["root", "a", "b"], prev, 5_000, 1_000)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("pruneStaleSubtreeIds (#2070)", () => {
|
||||
it("drops entries whose root is no longer in the live node set", () => {
|
||||
const map = new Map<string, Set<string>>([
|
||||
["root-1", new Set(["root-1", "a"])],
|
||||
["root-2", new Set(["root-2", "b"])],
|
||||
["root-3", new Set(["root-3", "c"])],
|
||||
]);
|
||||
pruneStaleSubtreeIds(map, new Set(["root-1", "root-3"]));
|
||||
expect([...map.keys()].sort()).toEqual(["root-1", "root-3"]);
|
||||
});
|
||||
|
||||
it("is a no-op when every root is still live", () => {
|
||||
const map = new Map<string, Set<string>>([
|
||||
["root-1", new Set(["root-1"])],
|
||||
["root-2", new Set(["root-2"])],
|
||||
]);
|
||||
pruneStaleSubtreeIds(map, new Set(["root-1", "root-2"]));
|
||||
expect(map.size).toBe(2);
|
||||
});
|
||||
|
||||
it("clears the map when no live roots remain", () => {
|
||||
const map = new Map<string, Set<string>>([
|
||||
["root-1", new Set(["root-1"])],
|
||||
]);
|
||||
pruneStaleSubtreeIds(map, new Set());
|
||||
expect(map.size).toBe(0);
|
||||
});
|
||||
|
||||
it("does not add new entries — only deletes stale ones", () => {
|
||||
const map = new Map<string, Set<string>>();
|
||||
pruneStaleSubtreeIds(map, new Set(["root-1", "root-2"]));
|
||||
expect(map.size).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
@ -40,6 +40,24 @@ export function shouldFitGrowing(
|
||||
return userPannedAt <= lastAutoFitAt;
|
||||
}
|
||||
|
||||
/**
|
||||
* Drop entries from the last-fit snapshot map whose root id no longer
|
||||
* exists in the live node set. Called on every fit attempt — the cost
|
||||
* is O(map_size) and runs only at user-driven cadence (deploys), so
|
||||
* the map is bounded by "roots present right now" instead of growing
|
||||
* unbounded across long sessions of import-then-delete cycles. (#2070)
|
||||
*/
|
||||
export function pruneStaleSubtreeIds(
|
||||
map: Map<string, Set<string>>,
|
||||
liveNodeIds: ReadonlySet<string>,
|
||||
): void {
|
||||
for (const rootId of map.keys()) {
|
||||
if (!liveNodeIds.has(rootId)) {
|
||||
map.delete(rootId);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Wires the two canvas-wide CustomEvent listeners and the viewport
|
||||
* save/restore bookkeeping so Canvas.tsx doesn't have to.
|
||||
@ -251,10 +269,9 @@ export function useCanvasViewport() {
|
||||
// brand-new node landed off-screen. The id-set sees the new id
|
||||
// wasn't in the snapshot and forces the fit.
|
||||
//
|
||||
// Map is keyed by root id and never pruned. Acceptable today because
|
||||
// org roots are UUIDs (no collisions on retry / template re-import),
|
||||
// canvas sessions are per-tab, and entries are tiny. Worth a sweep
|
||||
// if long-lived sessions ever start importing hundreds of orgs.
|
||||
// Map is keyed by root id. Pruned at the top of `runFit` against the
|
||||
// live node set, so deleted roots don't accumulate across long
|
||||
// sessions of import-then-delete cycles (#2070).
|
||||
const lastFitSubtreeIdsRef = useRef<Map<string, Set<string>>>(new Map());
|
||||
useEffect(() => {
|
||||
const runFit = () => {
|
||||
@ -262,6 +279,10 @@ export function useCanvasViewport() {
|
||||
pendingFitRootRef.current = null;
|
||||
if (!rootCandidate) return;
|
||||
const state = useCanvasStore.getState();
|
||||
pruneStaleSubtreeIds(
|
||||
lastFitSubtreeIdsRef.current,
|
||||
new Set(state.nodes.map((n) => n.id)),
|
||||
);
|
||||
// Climb to the true root — the event's rootId is the just-
|
||||
// landed child's direct parent, which may itself be nested.
|
||||
let topId = rootCandidate;
|
||||
|
||||
Loading…
Reference in New Issue
Block a user