Skip to content

fix(joint-react): move element measuring into one module and settle useOnElementsMeasured - #3555

Closed
samuelgja wants to merge 1 commit into
clientIO:devfrom
samuelgja:fix/react-measurement-module
Closed

samuelgja wants to merge 1 commit into
clientIO:devfrom
samuelgja:fix/react-measurement-module

Conversation

@samuelgja

Copy link
Copy Markdown
Contributor

Description

Measuring elements was spread over graph-store.ts, graph-changes.ts, graph-projection.ts and create-elements-size-observer.ts. This PR moves it into one module that the store core no longer needs to know about, and fixes the cases where useOnElementsMeasured still fired too early, too late or never. The public API is unchanged: no hook or selector is added, removed or deprecated.

src/store/measurement.ts (new, replaces create-elements-size-observer.ts)

  • createMeasurement({ graph, autoSizeOrigin, paperStores }), exposed as graphStore.measurement. It owns its own mvc.Listener on the graph (add / remove / reset / change:size), the node registry and the ResizeObserver.
  • The ResizeObserver is created by the first observe(), so a diagram that never measures has none.
  • It keeps one sizing version: 0 while nothing is measured, otherwise a value that changes once per settled change and is never reused. useOnElementsMeasured reads it.
  • The settled event is delivered one scheduler round after the graph events that settle an element. That is after the projection has committed the measured sizes, so a callback that reads the store sees them. Before this, it ran ahead of that commit.
  • Removing a paper re-checks the delivery on the next scheduler pass. Elements that only waited for the last paper settle, and a paper re-created right away (StrictMode, a hot reload) keeps them waiting.
  • graph-store.ts, graph-changes.ts and graph-projection.ts no longer contain measurement code. use-are-elements-measured.ts (internal) and two dead helpers in clear-view.ts are deleted.

useOnElementsMeasured - contract

Unchanged from #3518:

  • one event per settled change;
  • sizes the application writes (cell.resize(), controlled cells sync) are not measurements.

Changed (the JSDoc states each):

  • an element removal is a settled change, so what is left can be laid out again;
  • a zero size is a legal final size: a diagram of zero-sized elements is measured;
  • isInitial is reported again after the last element is removed, as after a reset;
  • elements already in an external graph are tracked from the start;
  • an element whose measured node unmounts no longer blocks later events.

useCells

  • It now runs on an internal graph-state reader (src/store/graph-state.ts, use-graph-state.ts, not exported). The reader subscribes to exactly what the selector reads: one cell, the ids of a collection, or all cells.
  • An inline selector that captured a changed prop is run again on that render. Before, it returned the previous value until the next graph change.
  • A numeric id and its string form are different ids, as in dia.Graph.

Store callbacks (src/utils/scheduler.ts, src/store/graph-projection.ts)

These three bugs already exist on dev:

  • A callback that threw left the shared scheduler flushing forever, for every graph on the page. Each callback is now isolated, and the error is rethrown on a later task.
  • Callbacks that kept scheduling each other were drained in one endless microtask. A flush now does at most 100 rounds, continues on a later task, and logs one dev warning.
  • A graph change made by a subscriber during the notification was recorded and then wiped. The pending changes are now cleared before notifying.

Tests

  • src/store/__tests__/measurement.test.ts: the observer and graph-changes tests, ported one to one, plus the settled-state cases.
  • use-on-elements-measured-events.test.tsx: the 16 spec cases from test(joint-react): specify how many events useOnElementsMeasured delivers #3520, through useOnElementsMeasured.
  • use-on-elements-measured-settling.test.tsx: an unrendered element, removal while waiting or settled, a content change that re-measures.
  • graph-state.test.ts and use-graph-state.test.tsx cover the reader. They include regression tests for the event-before-commit ordering and a subscription that follows a render React throws away.
  • graph-store.test.ts covers removing the last paper.
  • use-cells.test.tsx covers the inline-selector fix and the hook name in the dev warning.
  • scheduler.test.ts covers the throwing callback, the cascade cap and changes made during notification.
  • Results in packages/joint-react:
    • yarn test (typecheck + knip + duplicity-detect + jest): 1150 tests on React 19 and 1146 on React 18, all passing.
    • yarn lint: 0 errors.
  • Storybook check on this branch:
    • automatic-layout: lays out without overlap, including after "Add node".
    • flowchart, collapsible-subtrees: render as before.

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. This rewrites the unreleased line from fix(joint-react): deliver useOnElementsMeasured once per settled change #3518.
  • @joint/react patch: <GraphProvider /> - fix a store callback that throws or changes the graph breaking later updates.
  • @joint/react patch: useCells - fix an inline selector that captures a changed prop returning a stale value until the next graph change.

Motivation and Context

Follow-up of #3518 (spec in #3520), split out of #3532 so the fixes can ship in the next patch release. #3532 is stacked on this branch and adds only the new public API on top: useGraphState, useOnGraphStateChange, useOnCellsChange, the sizing selectors, and the useOnElementsMeasured deprecation.

Notes

  • PRs to dev get no CI; the full suite was run locally.
  • Performance: yarn bench, with dev and this branch both run on the same machine, is within noise. A hook-level probe (2,000 elements, 2,000 per-cell useCells(id, selector) readers plus two all-cells readers) measures:
    • drag frame: 0.73–0.79 ms here, 0.75–0.80 ms on dev;
    • mount: 59–78 ms here, 61–75 ms on dev.
  • Left for a follow-up: per-paper settling (elements seeded before any paper mounts, two papers where the non-measuring one mounts first).
  • yarn test-coverage is below its floor on dev already; this PR does not change that.

Screenshots (if appropriate):

…seOnElementsMeasured

- store/measurement.ts replaces create-elements-size-observer.ts and the
  measurement code in graph-store, graph-changes and graph-projection; the
  ResizeObserver is created by the first observe()
- useOnElementsMeasured: one event per settled change, a removal included;
  zero-sized elements count as measured; elements of an external graph are
  tracked from the start; an unmounted measured node no longer blocks events;
  the event is announced once the measured sizes are in the store
- useCells runs on an internal graph-state reader that subscribes to what its
  selector reads; an inline selector capturing a changed prop is re-run
- scheduler: a throwing callback no longer stops the flush; cascading
  callbacks continue on a later task; a graph change made by a subscriber
  during notification is no longer dropped
@samuelgja

Copy link
Copy Markdown
Contributor Author

closed of favor of this master fix: #3556

@samuelgja samuelgja closed this Oct 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant