Repository navigation
fix(react-tunnel): order tunnel entries by their position in the tree - #3586
Merged
Merged
Conversation
The field shows the first `FieldError` in its error tunnel. The policy error was rendered before `children`, so it only came second because it mounted later and tunnel entries kept their mount order. Rendered after `children`, a consumer's error wins by its position in the tree. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An entry took its index once, from a counter that `TunnelProvider` reset on each of its renders. The exit sorts by that index, so its order depended on how often the provider rendered: - An entry mounting next to committed ones got the count since the provider's last render: `A, <Gate>B</Gate>, C` showed `ACB`, and a parent appending `C` to `A, B` showed `ACB`. - A keyed reorder kept the indices, so reordered `Select`/`ComboBox` options stayed in mount order. An entry now reports when it mounts next to committed entries or renders after a higher index in the same render pass. It then calls `reorderEntries` in a layout effect: every entry of the tunnel state renders again through `useSyncExternalStore`, in tree order, and takes its index anew before paint. The provider no longer resets the counter. The `Section` visual test showed the old order: a secondary header button shown later sat right after the first action instead of before `Primary`. Its darwin references are updated; the linux ones need the `update-screenshots` label. Closes #3487 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Coverage Report for ./packages/components/
File CoverageNo changed files found. |
Co-authored-by: mfal <4696750+mfal@users.noreply.github.com>
Contributor
🚀 Preview DeploymentPreview environments are ready:
Images:
|
A concurrent render yields between entries, and the microtask that clears the recorded indices runs at each yield. An inversion across a yield went unnoticed. A commit does not yield, so the check now runs in a layout effect. reorderEntries re-renders synchronously, before the microtask, so the reorder commit compared against the previous commit's indices and reordered again. Entries whose index belongs to an older generation now skip the check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mfal
marked this pull request as ready for review
October 27, 2026 11:54
mfal
enabled auto-merge
October 27, 2026 12:06
Lisa18289
reviewed
Oct 27, 2026
StrictMode replays the mount render with the ref already set, so an entry mounting next to committed ones lost `needsReorder` in the render that commits and kept its count-so-far index (`ACB`). The flag is now stored with the index it belongs to. `reorderEntries` takes the generation the entry rendered with and does nothing while a reorder is pending. Entries mounting in one commit no longer notify every listener once each. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Lisa18289
approved these changes
Oct 27, 2026
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
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.
What & why
A tunnel's exit now lists its entries in tree order. Before, each entry took its index once, from a counter that
TunnelProviderreset on each of its renders, so the order depended on how often the provider rendered:A, <Gate>B</Gate>, CshowedACB, and a parent appendingCtoA, BshowedACB.Select/ComboBoxoptions stayed in mount order (react-tunnel: reordered entries keep their mount order (Select/ComboBox options never reorder) #3487).Users see it in every component that collects children through a tunnel:
Navigation/NavigationGrouplinks,TabNavigation(its overflow menu slices the tunnel, so a wrong order shows the wrong links under "more"),Tabs,RadioGroup/CheckboxGroup,Select/ComboBox/Autocompleteoptions, header actions. It needs an entry that mounts after the others (permission check, Suspense, flag) or a reorder. Onnext, #3579 makes the provider re-render less often, so the old reset hides fewer of these cases.How: an entry calls
reorderEntriesin a layout effect when it mounts next to committed entries of its tunnel, or when it commits after a higher index in the same commit. The order check runs in the commit, not the render: a concurrent render yields between entries, a commit does not. Every entry of that tunnel state renders again throughuseSyncExternalStore, in tree order, and takes its index anew before paint. The provider no longer resets the counter. Entries rendernull, so the extra pass is cheap; it runs only on such a mount or reorder.PasswordCreationFieldshowed the firstFieldErrorin its tunnel and rendered its policy error beforechildren. The consumer's error only came first because the policy error mounted later. The policy error now renders afterchildren(own commit).Visual change: the
Sectiontest shows a secondary header button that appears later. The old reference had it right after the first action (Link Secondary Switch Primary); tree order isLink Switch Secondary Primary. The darwin and linux references are updated (linux viaupdate-screenshots).Limit: an entry that does not render in the reorder pass (inside a hidden
Activityor a suspended boundary) takes its index when it renders and is corrected by the next out-of-order check.Issue
Closes #3487
Tests
react-tunnel: an entry mounting later between others, a parent appending an entry, keyed reorders (ABC→CBA→BDCA). All three fail without the change.Select: reordered keyed options are listed in their new order. Fails without the change.Sectionchange above and 10 failures that also occur onmainwithout this change (missing or outdated darwin references for CartesianChart, ImageCropper, MessageThread, ModalconfirmOnClose, Section growing inside a LayoutCard).next; react-tunnel there: 60/60.Checklist
Closes #…pnpm lintis clean andpnpm affected:testpasses (browser tests if behavior changed)git diffis empty after the relevantbuild:*targets)de-DEanden-USlocale files — none addedupdate-screenshotslabel🤖 Generated with Claude Code