fix(joint-react): prevent stale layout:update entries from resurrecting removed cells - #3494
fix(joint-react): prevent stale layout:update entries from resurrecting removed cells#3494samuelgja wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Delayed updates can still overwrite replacement cells sharing the same ID, and the changeset violates repository formatting requirements.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Fixes stale graph projection records that prevent restored cells and links from rendering after undo.
Changes:
- Validates projection updates against graph membership and prunes orphaned links.
- Rechecks hidden pending links after portal commits.
- Adds regression coverage for external-graph delete/undo behavior.
File summaries
| File | Description |
|---|---|
graph-projection.ts |
Reconciles stale cell and link records. |
graph-projection-edge-cases.test.ts |
Updates orphan-link sweep coverage. |
use-create-portal-paper.tsx |
Rechecks pending links after React commits. |
external-graph-undo.test.tsx |
Adds end-to-end undo regression tests. |
.changeset/eleven-moments-heal.md |
Documents the patch release. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟡 Changes recommended
Two moderate correctness issues can leave links hidden or emit stale removal deltas.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 2
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
The pending-link observer and quadratic scanning issues must be addressed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 2
- Review effort level: Balanced
| } | ||
| } | ||
|
|
||
| if (this.pendingLinks.size === 0) this.disconnectPortalObserver(); |
| if (!isSourceReady || !isTargetReady) { | ||
| view.el.style.visibility = 'hidden'; | ||
| this.pendingLinks.add(cellId); | ||
| this.observePendingLinkEndpoints(); |
Description
Fixes a permanent render loss with externally-owned graphs (
<GraphProvider graph={graph}>+imperative mutations + CommandManager undo): after deleting an element that has a link and
undoing, the restored element came back positioned and sized but empty —
renderElementwasnever called for it again — and the restored link stayed
visibility: hidden.Mechanism:
layout:updatechange batches hold direct cell references and are producedasynchronously (paper view-mount re-broadcasts via
setPaperViews, app layout pipelinesapplying async results such as ELK). When a removal landed between producing and processing
such a batch, the projection wrote the entry into the cells container without checking that
the cell was still in the graph — resurrecting the removed cell's record. From then on the
container permanently disagreed with the graph: the undo's re-add of the byte-identical cell
merged into the stale record (
mergeCellRecordidentity fast path →isStrictEqualearlyreturn), so no version bump and no membership notification ever fired,
useContainerKeyskept its cached ids, and the cell never rendered again.
Changes in
graph-projection.ts:add/changeentries are gated on graph membership; a stale entry now also repairs thecontainer by deleting a lingering record.
removeentries only apply when the cell has actually left the graph, so a paperview-unmount notification (which can name a still-mounted cell, e.g. viewport culling)
can no longer delete a live cell's record.
(
graph.getConnectedLinkscannot name them at that point — the cells are already out ofthe graph adjacency). The sweep skips links the graph still holds, so links re-added
within the same batch survive.
Change in
use-create-portal-paper.tsx:pendingLinks(endpoint's React content not painted yet) were onlyrechecked from
insertViewand joint'safterRender; portal content mounting in a Reactcommit triggers neither, so a link could stay hidden permanently once its endpoint painted.
A
useEffectnow rechecks after every commit that (re)rendered the element portals. Theeffect is O(1) in steady state (
pendingLinks.size === 0early return) and its dependencydoes not change on cell data/position updates, so it never runs on drag frames.
Tests: new
external-graph-undo.test.tsxcovers the delete+undo scenario end to end(projection pruning, stale-entry resurrection, live-cell view-unmount safety, undo repaint
with link visibility). The
graph-projection-edge-casessweep test was updated to exercisethe sweep through a genuinely missed link removal instead of a synthetic remove entry for a
cell still in the graph (which is exactly the unsafe input this fix rejects).
Motivation and Context
Reported against 4.3.5 by a customer running an externally-owned
dia.Graphsubclass withimperative mutations (
fromJSON, stencil drops, layout) and CommandManager undo/redo:delete an element with a link, press Ctrl+Z, and the shape returns empty with its link
hidden, unrecoverably. Their measurement (graph at 2 cells while the projection held 4,
including the deleted element and its link) matches the resurrection mechanism above.
Screenshots (if appropriate):