You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(joint-react): fire useOnElementsMeasured once per settled change - #3556
On master, useOnElementsMeasured calls back on every bump of a measureState counter. That counter is bumped by any element size change once some element has a non-zero size. So the callback fires:
too early: before elements that are still being measured have their size, so a layout runs on placeholder sizes;
too often: once per size change instead of once per change of the diagram;
for sizes the application writes itself, such as cell.resize();
never for a diagram of zero-sized elements;
with a stale isInitial after resetCells().
This PR ports the fix from dev (#3518 and its follow-up) to master. It does not bring dev's new state management, and it adds no new API.
createMeasurement({ graph, autoSizeOrigin, paperStores }), exposed as graphStore.measurement. It listens to the graph itself (add / remove / reset / change:size), owns the node registry, and creates its ResizeObserver on the first observe().
An element is outstanding from entering the graph until a positive signal settles it, never by reading its size:
its React content committed without registering a node: markRendered(), called from the element item's existing layout effect;
no paper renders it (viewport culling, cellVisibility): settleUnrendered() on each paper's render:done, or render:idle after a pass that only culled;
its registered node was measured;
it left the graph.
A settled element that starts measuring later (its content switches to <HTMLHost>, or it scrolls into view) waits for its node's first report too. That report holds back delivery and the mount replay, but it is a change only when it writes a size.
The sizing version changes once per settled change (an addition, a removal or a re-measurement), once nothing is outstanding.
It is delivered one scheduler round after the settling graph events, so the store already holds the measured sizes.
Removing the last paper re-checks the delivery. A pass delivered while no paper was mounted leaves its elements outstanding, and settling them later is a change of its own.
Sizes the application writes are not measurements.
A zero size is a legal final size.
Only a reset starts a new history (isInitial is reported again). It tracks the elements of every layer. A node that outlives its model (a reset, or the same id removed and added back in one task) is observed again, against the new model. Emptying the diagram is an ordinary change: an empty diagram fires nothing, and the version is kept.
graph-store.ts, graph-changes.ts and graph-projection.ts lose their measurement code. use-are-elements-measured.ts (internal) and two dead helpers in clear-view.ts are removed.
Hooks
useOnElementsMeasured: reads measurement, keeps its signature, and the JSDoc now states the contract. It subscribes before replaying, and on mount replays only a settled pass of a diagram that has elements; while a change waits, or the diagram is empty, the next delivery is the hook's initial event. A callback that throws is rethrown on a later task, so other subscribers of the same change are still notified.
The element item reports its content as rendered again when its portal target changes (a same-id reset whose views render before React commits).
useMeasureElement: registers through measurement.observe(). It clears the view cache of its own element when its size changes, replacing the sweep over every element on each measurement.
Tests
use-on-elements-measured-events.test.tsx (the 16-case spec from test(joint-react): specify how many events useOnElementsMeasured delivers #3520) and use-on-elements-measured-settling.test.tsx (an unrendered or entirely culled element, removal while waiting or settled, a content change that re-measures, emptying and refilling the diagram, a same-id reset, a hook or a paper mounted while a change waits, a hook mounted on an emptied diagram, a replayed callback that resets the graph, an HTML overlay element removed and added back in one task, a settled element that starts measuring).
14 of the first 24 cases fail on master, and all 37 pass here. The cases added during review each failed before the fix they cover.
use-on-elements-measured.compiler.test.tsx runs both suites again under React Compiler.
src/store/__tests__/measurement.test.ts: the observer and graph-change tests, ported one to one, plus the settled-state cases (an empty graph keeps the version, a reset on a nondefault layer, nodes re-observed after a reset, isSettled(), including while a settled element waits for its first report).
graph-store.test.ts covers removing the last paper while elements wait.
Results in packages/joint-react:
yarn test (typecheck + knip + coverage + React 18): 1156 tests with coverage on React 19 and 1115 on React 18, all passing, above the coverage floors.
yarn lint: 0 errors.
Storybook (master): every story that uses the hook renders sized elements without overlap and without console errors: automatic-layout, element-controls, collapsible-subtrees, dynamic-status-icons, flowchart, introduction-demo, automatic-layout-storage. Adding a node re-runs the layout.
Changesets: @joint/react patch - useOnElementsMeasured - fix to fire exactly once per settled change (an addition, a removal or a re-measurement), when no element is still waiting to be measured.
stories/examples/buttons-in-magnets
The paper had no height (className="size-full" inside a container without one), so the story rendered empty. It now sets height: 300, like the other examples.
The card chrome used a dark palette over the light theme's dark text. It now uses the theme's tokens (--jj-color-primary, --jj-color-surface, ...).
Motivation and Context
The settled-change fix is on dev (#3518) but dev is not being released yet, and the hook is broken in the current release. This PR is the same fix on master.
The changeset file has the same name and text as the unreleased one on dev, so the next master→dev sync merges it cleanly.
In the measuring files, the sync resolves to dev's version. The review fixes made here (reset on every layer, re-observing on reset, the mount replay, the empty diagram, culled-only passes) are to be ported to feat(joint-react): add useGraphState and useOnGraphStateChange #3532 first.
Behaviour change beyond the review threads: papers now also settle on render:idle. A pass that only culls updates no view, so core fires no render:done, and an addition the paper culled entirely used to hold the event back until some unrelated render.
Left for a follow-up: per-paper settling. With two papers on one graph where only one measures, the other one's commit settles an element early, so its hook can fire before the measurement and again after it.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Unresolved reset and subscription lifecycle defects can block measurement notifications, deliver them prematurely, or preserve stale initial-state history.
When an already-settled plain element switches to <HTMLHost>, registering its node does not make it pending again. If another element is removed before that node's first ResizeObserver report, the removal delivers using the old size, then the measurement delivers a second event. Track pending first reports until measurement or cleanup so other changes wait, while preserving no-event behavior for unchanged sizes and StrictMode re-registrations.
The reason will be displayed to describe this comment to others. Learn more.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
- store/measurement.ts replaces create-elements-size-observer.ts and the
measurement bookkeeping in graph-store, graph-changes and graph-projection;
it listens to the graph itself and creates its ResizeObserver lazily
- an element is outstanding from entering the graph until a positive signal
settles it: its content committed without registering a node, no paper
renders it (render:done), its node was measured, or it left the graph
- one event per settled change (an addition, a removal or a re-measurement),
announced once the measured sizes are in the store; application-written
sizes are not measurements; a zero size is a legal final size; a graph
reset or removing the last element starts a new history (isInitial)
- useMeasureElement clears the view cache of its own element when its size
changes, replacing the sweep on every measurement
The reason will be displayed to describe this comment to others. Learn more.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
On
master,useOnElementsMeasuredcalls back on every bump of ameasureStatecounter. That counter is bumped by any element size change once some element has a non-zero size. So the callback fires:cell.resize();isInitialafterresetCells().This PR ports the fix from
dev(#3518 and its follow-up) tomaster. It does not bringdev's new state management, and it adds no new API.src/store/measurement.ts(new, replacescreate-elements-size-observer.ts)createMeasurement({ graph, autoSizeOrigin, paperStores }), exposed asgraphStore.measurement. It listens to the graph itself (add/remove/reset/change:size), owns the node registry, and creates itsResizeObserveron the firstobserve().markRendered(), called from the element item's existing layout effect;cellVisibility):settleUnrendered()on each paper'srender:done, orrender:idleafter a pass that only culled;<HTMLHost>, or it scrolls into view) waits for its node's first report too. That report holds back delivery and the mount replay, but it is a change only when it writes a size.isInitialis reported again). It tracks the elements of every layer. A node that outlives its model (a reset, or the same id removed and added back in one task) is observed again, against the new model. Emptying the diagram is an ordinary change: an empty diagram fires nothing, and the version is kept.graph-store.ts,graph-changes.tsandgraph-projection.tslose their measurement code.use-are-elements-measured.ts(internal) and two dead helpers inclear-view.tsare removed.Hooks
useOnElementsMeasured: readsmeasurement, keeps its signature, and the JSDoc now states the contract. It subscribes before replaying, and on mount replays only a settled pass of a diagram that has elements; while a change waits, or the diagram is empty, the next delivery is the hook's initial event. A callback that throws is rethrown on a later task, so other subscribers of the same change are still notified.useMeasureElement: registers throughmeasurement.observe(). It clears the view cache of its own element when its size changes, replacing the sweep over every element on each measurement.Tests
use-on-elements-measured-events.test.tsx(the 16-case spec from test(joint-react): specify how many events useOnElementsMeasured delivers #3520) anduse-on-elements-measured-settling.test.tsx(an unrendered or entirely culled element, removal while waiting or settled, a content change that re-measures, emptying and refilling the diagram, a same-id reset, a hook or a paper mounted while a change waits, a hook mounted on an emptied diagram, a replayed callback that resets the graph, an HTML overlay element removed and added back in one task, a settled element that starts measuring).master, and all 37 pass here. The cases added during review each failed before the fix they cover.use-on-elements-measured.compiler.test.tsxruns both suites again under React Compiler.src/store/__tests__/measurement.test.ts: the observer and graph-change tests, ported one to one, plus the settled-state cases (an empty graph keeps the version, a reset on a nondefault layer, nodes re-observed after a reset,isSettled(), including while a settled element waits for its first report).graph-store.test.tscovers removing the last paper while elements wait.packages/joint-react:yarn test(typecheck + knip + coverage + React 18): 1156 tests with coverage on React 19 and 1115 on React 18, all passing, above the coverage floors.yarn lint: 0 errors.master): every story that uses the hook renders sized elements without overlap and without console errors:automatic-layout,element-controls,collapsible-subtrees,dynamic-status-icons,flowchart,introduction-demo,automatic-layout-storage. Adding a node re-runs the layout.Changesets:
@joint/reactpatch -useOnElementsMeasured - fix to fire exactly once per settled change (an addition, a removal or a re-measurement), when no element is still waiting to be measured.stories/examples/buttons-in-magnetsclassName="size-full"inside a container without one), so the story rendered empty. It now setsheight: 300, like the other examples.--jj-color-primary,--jj-color-surface, ...).Motivation and Context
The settled-change fix is on
dev(#3518) butdevis not being released yet, and the hook is broken in the current release. This PR is the same fix onmaster.dev, so the next master→dev sync merges it cleanly.dev's version. The review fixes made here (reset on every layer, re-observing on reset, the mount replay, the empty diagram, culled-only passes) are to be ported to feat(joint-react): add useGraphState and useOnGraphStateChange #3532 first.Notes
render:idle. A pass that only culls updates no view, so core fires norender:done, and an addition the paper culled entirely used to hold the event back until some unrelated render.dev: the graph-state reader, theuseCellschanges and the scheduler hardening (feat(joint-react): add useGraphState and useOnGraphStateChange #3532). This PR touches only measuring.Screenshots (if appropriate):