Skip to content

fix(joint-react): register paper features synchronously during render#3306

Merged
kumilingus merged 3 commits into
clientIO:devfrom
kumilingus:fix/feature-render-phase-registration
May 12, 2026
Merged

fix(joint-react): register paper features synchronously during render#3306
kumilingus merged 3 commits into
clientIO:devfrom
kumilingus:fix/feature-render-phase-registration

Conversation

@kumilingus
Copy link
Copy Markdown
Contributor

@kumilingus kumilingus commented May 5, 2026

Summary

Fixes a class of bugs where features registered by hooks called inside a <Paper> subtree, or through a wrapping component (e.g. <PaperScroller> wrapping <Paper>), were not visible to sibling consumers in the same render pass. Adds adjacent fixes that surfaced while validating the main change.

Changes

1. Synchronous render-phase feature registration (use-create-features.ts)

Previously useCreateFeature deferred registration to useLayoutEffect. When a feature-registering component (e.g. <Snaplines />) mounted inside <Paper>, sibling consumers reading usePaperFeatures(paperId) in the same render captured an empty snapshot in their onLoad / imperative-API closures and never picked up the feature.

Fix: when target === 'paper' AND a paperStore is reachable AND the feature isn't already cached, register synchronously during render via createAndRegisterFeature + registerFeature. Mirrors the existing render-phase featureContext.features.set(...) path used when no paperStore exists. The featureRef.current guard in the existing useLayoutEffect re-register branch keeps onAddFeature firing exactly once.

2. Fallback paperStore lookup (use-create-features.ts)

When the hook is called outside a Paper subtree (e.g. <PaperScroller> wraps <Paper>, so the scroller sits above PaperStoreContext), usePaperStore(OPTIONAL) returns null. The deferred-queue path may have already registered the feature inside some paperStore, but useCreateFeature can't see it via context.

Fix: scan graphStore.papers for a paperStore whose features bag contains our id, fall back to it. Update / load paths now have a paperStore to operate on regardless of where the hook is mounted.

3. Existing-feature adoption (use-create-features.ts)

When the fallback paperStore resolves and a feature was already created via the deferred-queue path, the create-effect would have called onAddFeature again, producing duplicates. Adoption path: resolveExistingFeature(...) — if a feature with id already exists, store it in featureRef.current and skip creation.

4. <Paper> width/height routed through setDimensions (use-create-portal-paper.tsx)

width and height were spread into assignOptions(paper.options, ...), which mutated paper.options.width/height directly and bypassed the DOM update. Worse, it tripped setDimensions's early-exit guard the next time it ran (e.g. <PaperScroller> calling paper.setDimensions(...) after the prop change).

Fix: strip width/height out of the assignOptions spread and add a dedicated useEffect that calls paper.setDimensions(width, height). Keeps DOM CSS and paper.options in lockstep.

5. minPathMargin option in LinkRoutingOrthogonal preset (presets/link-routing.ts)

Surfaces the new minPathMargin option from the rightAngle router (joint-core #3266) on the orthogonal preset typings.

6. New @joint/react/stories subpath export

Adds a stories utility entry point so @joint/react-plus story files can reuse makeRootDocumentation, makeStory, and getAPILink without reaching into src/. Not part of the runtime public API.

  • package.json./stories exports map entry
  • rollup.config.tssrc/stories.ts added to entries
  • src/stories.ts (new) — re-exports
  • src/internal.ts — exposes OPTIONAL constant for downstream consumers

Test plan

  • yarn jest --testPathPatterns="features|use-create" (43/43 pass)
  • yarn typecheck (no new errors)
  • Manual: <Snaplines /> mounted as child of <Paper> is detected by sibling <Stencil /> (validated downstream in joint-plus)
  • Manual: <PaperScroller> wrapping <Paper> — feature registration / update / unmount paths all hit the same paperStore
  • Manual: changing width/height props on <Paper> resizes the DOM, and <PaperScroller> setDimensions calls afterwards still work

… when paperStore exists

When `<X />` mounts inside `<Paper>` (so PaperStoreContext is populated)
and calls `useCreateFeature('paper', ...)`, the previous code only
registered the feature in the commit phase via `useLayoutEffect`. A
sibling `<Y />` that reads `usePaperFeatures(paperId)` in the same render
would not see the feature, because:

1. `<Paper>` renders, isReady flips to true, children render.
2. The child component (e.g. `<Snaplines />`) invokes `useCreateFeature`.
   Registration is deferred to its useLayoutEffect.
3. Sibling consumer (e.g. `<Stencil />`) renders, `usePaperFeatures` reads
   `paperStore.features` — empty, no feature.
4. Commit phase: child effect registers the feature, version bumps.
5. Re-render — but consumers that captured features in their `onLoad` /
   imperative-api closures from step 3 still have the stale value.

Fix: when `target === 'paper'` AND `paperStore` exists at render time
AND the feature isn't already cached, register synchronously during
render (mirrors the existing render-phase deferred-registration path
for the no-paperStore branch). The existing `featureRef.current` guard
in the useLayoutEffect re-register branch ensures `onAddFeature` is
only called once.

Effect: feature registration is observable to sibling consumers in the
same render pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@kumilingus kumilingus requested a review from samuelgja May 5, 2026 18:05
Comment thread packages/joint-react/package.json
@samuelgja samuelgja marked this pull request as ready for review May 12, 2026 06:45
@kumilingus kumilingus merged commit ddaf162 into clientIO:dev May 12, 2026
@kumilingus kumilingus deleted the fix/feature-render-phase-registration branch May 12, 2026 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants