chore(simplify): generalize prune helper + add value-identity test
Simplify pass on top of #2070 fix: - Rename pruneStaleSubtreeIds → pruneStaleKeys, generalize to Map<string, T> so the same shape can absorb other keyed-by-node-id caches (ProvisioningTimeout.tsx tracking map is the obvious next caller — left as a follow-up to keep this PR scoped). - Trim the helper docstring to remove implementation-detail rot (O(map_size), cadence claims). The ref-block comment carries the rationale where it actually matters (at the call site). - Add identity-preservation test: survivors must keep their original Set reference. Guards against a future "rebuild instead of delete" regression that would silently invalidate downstream === checks. No behaviour change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
69edc0bf92
commit
570890dab6
@ -1,5 +1,5 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { pruneStaleSubtreeIds, shouldFitGrowing } from "../useCanvasViewport";
|
||||
import { pruneStaleKeys, shouldFitGrowing } from "../useCanvasViewport";
|
||||
|
||||
// Tests cover the auto-fit gate in isolation. The hook itself is
|
||||
// effects + refs + React Flow handles, awkward to exercise directly —
|
||||
@ -52,37 +52,47 @@ describe("shouldFitGrowing", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("pruneStaleSubtreeIds (#2070)", () => {
|
||||
it("drops entries whose root is no longer in the live node set", () => {
|
||||
describe("pruneStaleKeys (#2070)", () => {
|
||||
it("drops entries whose key is no longer in the live key 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"]));
|
||||
pruneStaleKeys(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", () => {
|
||||
it("is a no-op when every key 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"]));
|
||||
pruneStaleKeys(map, new Set(["root-1", "root-2"]));
|
||||
expect(map.size).toBe(2);
|
||||
});
|
||||
|
||||
it("clears the map when no live roots remain", () => {
|
||||
it("clears the map when no live keys remain", () => {
|
||||
const map = new Map<string, Set<string>>([
|
||||
["root-1", new Set(["root-1"])],
|
||||
]);
|
||||
pruneStaleSubtreeIds(map, new Set());
|
||||
pruneStaleKeys(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"]));
|
||||
pruneStaleKeys(map, new Set(["root-1", "root-2"]));
|
||||
expect(map.size).toBe(0);
|
||||
});
|
||||
|
||||
it("preserves value identity for survivors (no rebuild)", () => {
|
||||
const survivor = new Set(["root-1", "a"]);
|
||||
const map = new Map<string, Set<string>>([
|
||||
["root-1", survivor],
|
||||
["root-2", new Set(["root-2", "b"])],
|
||||
]);
|
||||
pruneStaleKeys(map, new Set(["root-1"]));
|
||||
expect(map.get("root-1")).toBe(survivor);
|
||||
});
|
||||
});
|
||||
|
||||
@ -41,19 +41,16 @@ export function shouldFitGrowing(
|
||||
}
|
||||
|
||||
/**
|
||||
* 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)
|
||||
* Drop entries from `map` whose key isn't in `liveKeys`. Generic so the
|
||||
* same shape can be reused for any keyed-by-node-id cache (#2070).
|
||||
*/
|
||||
export function pruneStaleSubtreeIds(
|
||||
map: Map<string, Set<string>>,
|
||||
liveNodeIds: ReadonlySet<string>,
|
||||
export function pruneStaleKeys<T>(
|
||||
map: Map<string, T>,
|
||||
liveKeys: ReadonlySet<string>,
|
||||
): void {
|
||||
for (const rootId of map.keys()) {
|
||||
if (!liveNodeIds.has(rootId)) {
|
||||
map.delete(rootId);
|
||||
for (const key of map.keys()) {
|
||||
if (!liveKeys.has(key)) {
|
||||
map.delete(key);
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -269,9 +266,11 @@ 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. 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).
|
||||
// Map is keyed by root id. Pruned in `runFit` against the live node
|
||||
// set so deleted roots don't accumulate across long sessions of
|
||||
// import-then-delete cycles (#2070). Bounded to "roots present right
|
||||
// now" by that prune; cleanup runs only at user-driven cadence
|
||||
// (deploys), so the rate naturally tracks growth.
|
||||
const lastFitSubtreeIdsRef = useRef<Map<string, Set<string>>>(new Map());
|
||||
useEffect(() => {
|
||||
const runFit = () => {
|
||||
@ -279,7 +278,7 @@ export function useCanvasViewport() {
|
||||
pendingFitRootRef.current = null;
|
||||
if (!rootCandidate) return;
|
||||
const state = useCanvasStore.getState();
|
||||
pruneStaleSubtreeIds(
|
||||
pruneStaleKeys(
|
||||
lastFitSubtreeIdsRef.current,
|
||||
new Set(state.nodes.map((n) => n.id)),
|
||||
);
|
||||
|
||||
Loading…
Reference in New Issue
Block a user