diff --git a/src/graph.test.ts b/src/graph.test.ts index ff7037ca..80c8c3f5 100644 --- a/src/graph.test.ts +++ b/src/graph.test.ts @@ -58,3 +58,43 @@ describe("Graph colors runtime merge behavior", () => { expect(graph.graphColors.anchor?.background).toBe("#222222"); }); }); + +describe("setEntities + waitUsableRectUpdate (pendingEntitiesUpdate fix)", () => { + function makeBlock(id: string, x = 0, y = 0): TBlock { + return { id, is: "Block", x, y, width: 100, height: 50, selected: false, name: id, anchors: [] }; + } + + it("resolves waitUsableRectUpdate after setEntities with same block positions", (done) => { + const node = document.createElement("div"); + const block = makeBlock("b1", 10, 20); + const graph = new Graph({ blocks: [block], connections: [] }, node); + graph.start(); + + // Wait for initial hitboxes to settle + graph.hitTest.waitUsableRectUpdate(() => { + // Call setEntities with the exact same block (same position) + graph.setEntities({ blocks: [block], connections: [] }); + + // Must resolve after setEntities even though usableRect doesn't change + graph.hitTest.waitUsableRectUpdate((rect) => { + expect(rect.width).toBeGreaterThan(0); + done(); + }); + }); + }, 5000); + + it("resolves waitUsableRectUpdate after setEntities with new blocks", (done) => { + const node = document.createElement("div"); + const graph = new Graph({ blocks: [makeBlock("b1")], connections: [] }, node); + graph.start(); + + graph.hitTest.waitUsableRectUpdate(() => { + graph.setEntities({ blocks: [makeBlock("b2", 200, 200)], connections: [] }); + + graph.hitTest.waitUsableRectUpdate((rect) => { + expect(rect.x).toBeGreaterThanOrEqual(200); + done(); + }); + }); + }, 5000); +}); diff --git a/src/graph.ts b/src/graph.ts index 1fb92510..723ec362 100644 --- a/src/graph.ts +++ b/src/graph.ts @@ -304,11 +304,10 @@ export class Graph { blocks?: TBlock[]; connections?: TConnection[]; }>) { - // Reset usableRect so that waitUsableRectUpdate (used by zoomToViewPort) - // treats hitTest as unstable and waits for new block components to register - // their hitboxes. Without this, a stale usableRect can make hitTest appear - // stable, causing the zoom callback to fire immediately with wrong data. - this.hitTest.resetUsableRect(); + // Mark hitTest as pending so waitUsableRectUpdate (used by zoomToViewPort) + // waits for block components to settle. Unlike resetUsableRect, this does not + // clear usableRectTracker, so blocks that haven't moved don't need to re-register. + this.hitTest.markPendingUpdate(); batch(() => { this.rootStore.blocksList.setBlocks(blocks || []); this.rootStore.connectionsList.setConnections(connections || []); diff --git a/src/services/HitTest.test.ts b/src/services/HitTest.test.ts new file mode 100644 index 00000000..63f83682 --- /dev/null +++ b/src/services/HitTest.test.ts @@ -0,0 +1,89 @@ +import { Graph } from "../graph"; + +import { HitBox, HitTest } from "./HitTest"; + +function makeHitTest(hasBlocks = false): HitTest { + const mockGraph = { + rootStore: { + blocksList: { $blocks: { value: hasBlocks ? [{}] : [] } }, + connectionsList: { $connections: { value: [] } }, + }, + } as unknown as Graph; + return new HitTest(mockGraph); +} + +function seedUsableRect(ht: HitTest): void { + const fakeHitBox = { + affectsUsableRect: true, + destroyed: false, + minX: 0, + minY: 0, + maxX: 100, + maxY: 100, + x: 0, + y: 0, + } as unknown as HitBox; + (ht as unknown as { usableRectTracker: { add(h: HitBox): void } }).usableRectTracker.add(fakeHitBox); + (ht as unknown as { updateUsableRect(): void }).updateUsableRect(); +} + +/** + * Trigger processQueue by queueing a fake hitbox update and flushing. + * This mirrors real production code: hitbox updates always precede processQueue. + */ +function triggerProcessQueue(ht: HitTest): void { + const fakeHitBox = { + affectsUsableRect: true, + destroyed: false, + minX: 0, + minY: 0, + maxX: 100, + maxY: 100, + x: 0, + y: 0, + updateRect(_bbox: unknown): void { + // no-op for test fake + }, + } as unknown as HitBox; + ht.update(fakeHitBox, { minX: 0, minY: 0, maxX: 100, maxY: 100, x: 0, y: 0 }); + (ht as unknown as { processQueue: { flush(): void } }).processQueue.flush(); +} + +describe("HitTest.markPendingUpdate", () => { + it("makes isUnstable true immediately after call", () => { + const ht = makeHitTest(true); + // Seed non-zero usableRect so zero-rect heuristic doesn't interfere + seedUsableRect(ht); + expect(ht.isUnstable).toBe(false); + ht.markPendingUpdate(); + expect(ht.isUnstable).toBe(true); + }); + + it("isUnstable becomes false after processQueue flushes with a hitbox update", () => { + const ht = makeHitTest(true); + seedUsableRect(ht); + ht.markPendingUpdate(); + expect(ht.isUnstable).toBe(true); + // processQueue fires naturally when hitbox updates arrive; simulate that here + triggerProcessQueue(ht); + expect(ht.isUnstable).toBe(false); + }); + + it("waitUsableRectUpdate resolves when flag clears, even if usableRect did not change", () => { + const ht = makeHitTest(true); + seedUsableRect(ht); + + ht.markPendingUpdate(); + expect(ht.isUnstable).toBe(true); + + let called = false; + ht.waitUsableRectUpdate(() => { + called = true; + }); + expect(called).toBe(false); // still waiting + + // processQueue fires when hitbox updates arrive (simulated here) + triggerProcessQueue(ht); + expect(called).toBe(true); // resolved after flag cleared + }); +}); diff --git a/src/services/HitTest.ts b/src/services/HitTest.ts index c6acd30e..4dd53cb0 100644 --- a/src/services/HitTest.ts +++ b/src/services/HitTest.ts @@ -59,6 +59,10 @@ export class HitTest extends Emitter { public readonly $usableRect = signal({ x: 0, y: 0, width: 0, height: 0 }); + // Explicit pending flag set by setEntities; cleared when processQueue completes. + // Avoids the "zero usableRect = unstable" heuristic for entity replacement. + private readonly $pendingEntitiesUpdate = signal(false); + // Single queue replaces all complex state tracking protected queue = new Map(); @@ -90,10 +94,10 @@ export class HitTest extends Emitter { // If graph has no elements, it's stable even with zero usableRect if (hasZeroUsableRect && !this.hasGraphElements()) { - return hasProcessingQueue; + return hasProcessingQueue || this.$pendingEntitiesUpdate.value; } - return hasProcessingQueue || hasZeroUsableRect; + return hasProcessingQueue || hasZeroUsableRect || this.$pendingEntitiesUpdate.value; } /** @@ -148,6 +152,7 @@ export class HitTest extends Emitter { this.queue.clear(); this.updateUsableRect(); this.emit("update", this); + this.$pendingEntitiesUpdate.value = false; }, { priority: ESchedulerPriority.LOWEST, @@ -176,6 +181,7 @@ export class HitTest extends Emitter { this.interactiveTree.clear(); this.usableRectTracker.clear(); this.updateUsableRect(); + this.$pendingEntitiesUpdate.value = false; } /** @@ -188,6 +194,20 @@ export class HitTest extends Emitter { this.updateUsableRect(); } + /** + * Mark hitTest as pending an entity update (called by setEntities). + * Makes isUnstable = true until processQueue completes. + * Does NOT clear usableRectTracker — existing data stays valid. + * Does NOT eagerly schedule processQueue — it fires naturally when hitbox + * updates arrive (new components register) or when updateBlock forces a hitbox + * refresh on existing components. Eager scheduling caused processQueue to run + * in the same rAF frame as markPendingUpdate (before MEDIUM/component renders), + * clearing $pendingEntitiesUpdate before new block hitboxes were queued. + */ + public markPendingUpdate(): void { + this.$pendingEntitiesUpdate.value = true; + } + /** * Add new HitBox item * @param item HitBox item to add @@ -213,15 +233,25 @@ export class HitTest extends Emitter { } if (this.isUnstable) { - const removeListener = this.$usableRect.subscribe(() => { + let cleaned = false; + let unsubRect: () => void = noop; + let unsubPending: () => void = noop; + const cleanup = () => { + if (cleaned) return; + cleaned = true; + unsubRect(); + unsubPending(); + }; + const check = () => { if (!this.isUnstable) { - removeListener(); + cleanup(); + // eslint-disable-next-line callback-return callback(this.$usableRect.value); - return; } - return; - }); - return removeListener; + }; + unsubRect = this.$usableRect.subscribe(check); + unsubPending = this.$pendingEntitiesUpdate.subscribe(check); + return cleanup; } callback(this.$usableRect.value); return noop; diff --git a/src/utils/utils/schedule.ts b/src/utils/utils/schedule.ts index 70974b1e..6851d64a 100644 --- a/src/utils/utils/schedule.ts +++ b/src/utils/utils/schedule.ts @@ -108,9 +108,20 @@ export const debounce = ) => void>( }; const flush = () => { - if (isScheduled && latestArgs) { - fn(...latestArgs); - cancel(); + if (isScheduled) { + const currentRemoveScheduler = removeScheduler; + isScheduled = false; + frameCounter = 0; + startTime = 0; + removeScheduler = null; + const args = latestArgs; + latestArgs = undefined; + // Remove the old scheduler handle BEFORE calling fn() so that any + // re-scheduling triggered inside fn() is not accidentally canceled. + if (currentRemoveScheduler) { + currentRemoveScheduler(); + } + fn(...((args ?? []) as Parameters)); } };