fix(sharedResizeObserver): independent subscriptions per element - #5817
Merged
freddymeta merged 6 commits intoSep 2, 2026
Conversation
The module held ONE callback per element — `callbacks.set(element, cb)` overwrote — so a second hook observing the same node silently replaced the first, and either one calling `unobserveResize(element)` blinded the other. Two hooks on one element is ordinary: a TabList root, a useOverflow container and a useTruncation target are all nodes another hook may reasonably watch. `observeResize` now returns an unsubscribe that removes only its own registration, and all seven callers in the package use it. `unobserveResize(element, callback)` does the same by hand; the callback-less form still drops every callback on the element and stays for a caller that owns its element outright. Dispatch iterates a copy of the set, so a callback may unsubscribe while the batch is running without skipping its neighbour. AST-010 §Implementation-requirements 9 asks for this and asks that it stay independently reviewable, so it is its own change rather than part of the percentage-sizing work that needs it. 8534 core tests pass. The four new observer tests each fail against the old module.
freddymeta
requested review from
cixzhang,
humbertovirtudes and
imdreamrunner
as code owners
September 1, 2026 06:21
meta-cla
Bot
added
the
CLA Signed
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
community
Contributor
PR Analysis Report📚 Storybook PreviewView Storybook for this PR 🧪 Sandbox PreviewView Sandbox for this PR Modified ComponentsAppShell (@astryxdesign/core) · View in Storybook
Chat (@astryxdesign/core) · View in Storybook
TabList (@astryxdesign/core) · View in Storybook
Table (@astryxdesign/core) · View in Storybook
Text (@astryxdesign/core) · View in Storybook
Tokenizer (@astryxdesign/core) · View in Storybook
Bundle Size Summary
Accessibility AuditStatus: 2 accessibility violation(s) found — 2 serious. Text - 1 issue(s)
Tokenizer - 1 issue(s)
Visual RegressionStatus: Skipped — Broad stable scope is deferred to the daily release gate. It covers 4332 trusted baseline shots instead of recapturing them for this PR. View the report Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
…erver-independent-callbacks
cixzhang
requested changes
Sep 2, 2026
cixzhang
left a comment
cixzhang
left a comment
Contributor
There was a problem hiding this comment.
Thanks for separating this prerequisite. A still-mounted region can stop following container resizes after a sibling unmounts: two observeResize calls sharing one callback collapse in the Set, and either cleanup removes both. Exact-head Chromium and Vitest both got zero callbacks where one should remain. Could each call keep an independent lifetime, with this duplicate-reference case added as a regression test?
[Reviewed by Robohands]
Contributor
Author
|
Addressed the duplicate-reference lifetime bug from the latest review.
Verification: 11/11 observer tests; 83/83 observer + Tokenizer tests; — Puck, on behalf of Freddy CI follow-up ( |
…ae139 # Conflicts: # apps/storybook/rtl-audit/rtl-audit.mjs
cixzhang
approved these changes
Sep 2, 2026
cixzhang
left a comment
cixzhang
left a comment
Contributor
There was a problem hiding this comment.
Thanks, this closes the shared-callback lifetime gap. The prior-head regression now fails before and passes here, including duplicate callback references and idempotent cleanup.
[Reviewed by Robohands]
github-actions
Bot
removed
the
needs:code-review
github-actions
Bot
deleted the
fix/shared-resize-observer-independent-callbacks
branch
September 3, 2026 06:22
Geervan
pushed a commit
to Geervan/astryx
that referenced
this pull request
Sep 3, 2026
* fix(useResizable): percentage configuration with an explicit basis (AST-010) Implements the accepted AST-010 contract. Percentages configure pixel state; they do not create a responsive selection mode. - `minSize` / `maxSize` accept the same exact number, `Npx`, or 0–100 `N%` vocabulary as `defaultSize`. Deprecated pixel aliases remain exact mutually-exclusive unions; unified props win untyped conflicts. - caller-owned `containerRef` selects the percentage basis. It follows replacement elements, measures the content-box active axis, and releases the element left behind. Without a ref, the released one-time viewport / 1200px server fallback remains. - defaults resolve once into pixel selection. Bounds re-resolve and clamp that selection permanently; a clamped default cannot revive when its basis grows. - hidden, detached, and not-yet-laid-out containers report zero, which is treated as unmeasured. Temporary fallback geometry is not persisted over a legal saved size. - pointer, keyboard, snaps, collapse, callbacks, persistence, ARIA, and `resize(number)` stay pixel-based. A trusted pointercancel, lost capture, or unmount releases the basis frozen for the gesture without claiming a completed resize. - ResizeHandle warns when its direction disagrees with the hook. The new `_direction` and `_onResizeCancel` fields are optional because `ResizableProps` is exported and old object literals must keep compiling. Browser evidence on this commit: replacement: basis 400->800, max 200->400, size 200->200 hidden mount: saved 321px remains 321px; storage unchanged touchCancel: max 500 -> frozen 500 -> 300; size=300; drag styles clear The touch-cancel probe uses CDP's real touchCancel. An untrusted synthetic PointerEvent did not reach React and was rejected as a harness bug rather than product evidence. Restacked on corrected facebook#5817 head 490cbfa; this layer touches no shared observer or migrated-observer consumer files. * fix(useResizable): keep the one-pass pixel-only mount, and measure Resizable/Layout RTL A pixel-only configuration needs no container measurement, so it must reach its size on the first render pass. Latching the selection lazily made the render-phase adjustment run at mount for every such region, doubling hook work: 1 pass to 2 at one mount, 50 to 100 at fifty. The selection is now initialized outright whenever the basis is already real; only a supplied container still defers, because its measurement does not exist until after commit, and that correction is unchanged. Adds the curated RTL targets this change needs: Layout D2 (start/end panel order) and Resizable D4 (handle side), closing both scoped coverage gaps. * fix(useResizable): add structured percentage sizing Add Table-aligned pixel() and percent() descriptors with one pixel floor or ceiling while keeping state, persistence, callbacks, paint, and ARIA in resolved pixels. * Fix Resizable ref and direction regressions --------- Co-authored-by: freddymeta <freddymeta@users.noreply.github.com> Co-authored-by: freddymeta <170298698+freddymeta@users.noreply.github.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
AST-010 §Implementation-requirements 9 asks for this and asks that it stay independently reviewable, so it is split out of #5783 rather than riding along in it.
The bug
callbackswas aMap<Element, ResizeCallback>— one callback per element.callbacks.set(element, cb)overwrote, so a second hook observing the same node silently replaced the first, and either one callingunobserveResize(element)blinded the other.Two hooks on one element is ordinary rather than exotic: a
TabListroot, auseOverflowcontainer and auseTruncationtarget are all nodes another hook may reasonably watch. It is also unavoidable for AST-010 — FR11 puts every region of a multi-region hook on one shared container.What changes
observeResizereturns an unsubscribe that removes only its own registration — all seven callers in the package now use it;unobserveResize(element, callback)does the same by hand;unobserveResize(element)still drops every callback on the element, kept for a caller that owns its element outright;What this replaces
A test named
replaces callback when same element is observed twiceasserted the old behaviour deliberately. I think that contract is the bug rather than a decision — it is what makes FR11 unimplementable — but it was explicit, so I am flagging the removal rather than quietly rewriting it.Verification
sharedResizeObserver.test.ts: 11/11 pass.pnpm lint:strict,pnpm build, and core typecheck pass on currentmain.pnpm testreaches the same two ContextMenu BottomSheet timing failures on clean currentmainand this branch;ContextMenu.test.tsxpasses 48/48 in isolation. CI is the clean-run check.