fix(router-generator): make buildRouteTree route ordering deterministic#7751
fix(router-generator): make buildRouteTree route ordering deterministic#77510x80 wants to merge 3 commits into
Conversation
The final sort tiebreaker in buildRouteTree compared whole route-node objects ((d) => d). Two objects both coerce to "[object Object]", so the comparator returns -1 for both (a, b) and (b, a) — an inconsistent comparator. Routes tying on segment-count and index-token were then left in an engine- and input-order-dependent order, producing non-deterministic routeTree.gen.ts diffs across machines and Node versions. Compare routePath instead, mirroring the pre-sort's final accessor, for a stable total order.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe route-tree generator now uses a shared sort helper with a routePath tiebreaker. The package export surface and tests were updated, and many generated route-tree snapshots were reordered to match the new deterministic output. ChangesDeterministic Route Tree Sort
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR fixes non-deterministic route ordering during route-tree generation in @tanstack/router-generator by ensuring buildRouteTree uses a consistent, total ordering when sorting route nodes. This prevents semantically-identical routeTree.gen.ts churn across different Node/V8 versions and machines.
Changes:
- Replace the final
multiSortBytiebreaker inbuildRouteTreefrom comparing whole route-node objects to comparingroutePath. - Ensure sibling routes that tie on earlier sort keys still sort deterministically by a stable string key.
- Add a patch changeset for
@tanstack/router-generatordocumenting the determinism fix.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/router-generator/src/generator.ts | Fixes buildRouteTree sorting by using routePath as the final tiebreaker for deterministic generation. |
| .changeset/deterministic-route-tree-sort.md | Adds a patch changeset describing the deterministic route ordering fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
can you please add unit tests that prove stable sorting? |
…apshots Extract the buildRouteTree tiebreaker sort into an exported pure `sortRouteNodes` helper and cover it with unit tests. The key test feeds a route set that ties on every key before `routePath` through all input permutations and asserts identical output, proving the sort is order- and engine-independent. Regenerate the 38 generator snapshots whose ordering changes under the deterministic tiebreaker. The diffs are pure emission-order reordering (every route's id/path/parentRoute is unchanged) -- the same semantically-identical churn the fix eliminates going forward.
|
@schiller-manuel added — pushed in 85b7e95. To make the sort testable I pulled the Heads-up: the deterministic order also changes route emission order, so I had to regenerate the 38 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/router-generator/tests/utils.test.ts`:
- Around line 982-984: The one-line `if` in `permutations<T>` violates the
project’s brace-required control-flow style. Update the `if (arr.length <= 1)`
branch in `permutations` to use curly braces around its body, keeping the same
return behavior while matching the coding guidelines for all `if`/`else`
statements.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ceac5fba-5751-4ad3-8b46-5c6144b193f8
📒 Files selected for processing (42)
packages/router-generator/src/generator.tspackages/router-generator/src/index.tspackages/router-generator/src/utils.tspackages/router-generator/tests/generator/add-extensions-custom/routeTree.snapshot.tspackages/router-generator/tests/generator/custom-scaffolding/routeTree.snapshot.tspackages/router-generator/tests/generator/custom-tokens/routeTree.snapshot.tspackages/router-generator/tests/generator/dot-escaped/routeTree.snapshot.tspackages/router-generator/tests/generator/escaped-custom-tokens/routeTree.snapshot.tspackages/router-generator/tests/generator/escaped-special-strings/routeTree.snapshot.tspackages/router-generator/tests/generator/export-variations/routeTree.snapshot.tspackages/router-generator/tests/generator/file-modification/routeTree.snapshot.tspackages/router-generator/tests/generator/flat-route-group/routeTree.snapshot.tspackages/router-generator/tests/generator/flat/routeTree.snapshot.tspackages/router-generator/tests/generator/invalid-param-names/routeTree.snapshot.tspackages/router-generator/tests/generator/nested-layouts/routeTree.snapshot.tspackages/router-generator/tests/generator/nested-route-groups-with-layouts-before-physical/routeTree.snapshot.tspackages/router-generator/tests/generator/nested/routeTree.snapshot.tspackages/router-generator/tests/generator/numbers-in-path/routeTree.snapshot.tspackages/router-generator/tests/generator/physical-pathless-layout-escaped-underscore/routeTree.snapshot.tspackages/router-generator/tests/generator/prefix-suffix/routeTree.snapshot.tspackages/router-generator/tests/generator/regex-tokens-inline/routeTree.snapshot.tspackages/router-generator/tests/generator/regex-tokens-json/routeTree.snapshot.tspackages/router-generator/tests/generator/regex-tokens-plus-json/routeTree.snapshot.tspackages/router-generator/tests/generator/root-export-specifier/routeTree.snapshot.tspackages/router-generator/tests/generator/route-groups/routeTree.snapshot.tspackages/router-generator/tests/generator/routeFileIgnore/routeTree.snapshot.tspackages/router-generator/tests/generator/routeFilePrefix/routeTree.snapshot.tspackages/router-generator/tests/generator/single-level/routeTree.snapshot.tspackages/router-generator/tests/generator/types-disabled/routeTree.snapshot.jspackages/router-generator/tests/generator/virtual-config-file-default-export/routeTree.snapshot.tspackages/router-generator/tests/generator/virtual-config-file-named-export/routeTree.snapshot.tspackages/router-generator/tests/generator/virtual-config-file-tsconfig-paths/routeTree.snapshot.tspackages/router-generator/tests/generator/virtual-inside-nested/routeTree.snapshot.tspackages/router-generator/tests/generator/virtual-inside-with-escaped-underscore/routeTree.snapshot.tspackages/router-generator/tests/generator/virtual-nested-layouts-with-virtual-route/routeTree.snapshot.tspackages/router-generator/tests/generator/virtual-pathless-layout-escaped-underscore/routeTree.snapshot.tspackages/router-generator/tests/generator/virtual-physical-empty-path-merge/routeTree.snapshot.tspackages/router-generator/tests/generator/virtual-physical-layout-and-index/routeTree.snapshot.tspackages/router-generator/tests/generator/virtual-physical-no-prefix/routeTree.snapshot.tspackages/router-generator/tests/generator/virtual-with-escaped-underscore/routeTree.snapshot.tspackages/router-generator/tests/generator/virtual/routeTree.snapshot.tspackages/router-generator/tests/utils.test.ts
✅ Files skipped from review due to trivial changes (34)
- packages/router-generator/tests/generator/escaped-custom-tokens/routeTree.snapshot.ts
- packages/router-generator/tests/generator/single-level/routeTree.snapshot.ts
- packages/router-generator/tests/generator/regex-tokens-inline/routeTree.snapshot.ts
- packages/router-generator/tests/generator/virtual-physical-empty-path-merge/routeTree.snapshot.ts
- packages/router-generator/tests/generator/add-extensions-custom/routeTree.snapshot.ts
- packages/router-generator/tests/generator/virtual-with-escaped-underscore/routeTree.snapshot.ts
- packages/router-generator/tests/generator/types-disabled/routeTree.snapshot.js
- packages/router-generator/tests/generator/export-variations/routeTree.snapshot.ts
- packages/router-generator/tests/generator/regex-tokens-json/routeTree.snapshot.ts
- packages/router-generator/tests/generator/custom-scaffolding/routeTree.snapshot.ts
- packages/router-generator/tests/generator/invalid-param-names/routeTree.snapshot.ts
- packages/router-generator/tests/generator/regex-tokens-plus-json/routeTree.snapshot.ts
- packages/router-generator/tests/generator/virtual-physical-layout-and-index/routeTree.snapshot.ts
- packages/router-generator/tests/generator/virtual-nested-layouts-with-virtual-route/routeTree.snapshot.ts
- packages/router-generator/tests/generator/routeFileIgnore/routeTree.snapshot.ts
- packages/router-generator/tests/generator/numbers-in-path/routeTree.snapshot.ts
- packages/router-generator/tests/generator/file-modification/routeTree.snapshot.ts
- packages/router-generator/tests/generator/flat-route-group/routeTree.snapshot.ts
- packages/router-generator/tests/generator/root-export-specifier/routeTree.snapshot.ts
- packages/router-generator/tests/generator/virtual-inside-with-escaped-underscore/routeTree.snapshot.ts
- packages/router-generator/tests/generator/nested-route-groups-with-layouts-before-physical/routeTree.snapshot.ts
- packages/router-generator/tests/generator/virtual-config-file-default-export/routeTree.snapshot.ts
- packages/router-generator/tests/generator/virtual/routeTree.snapshot.ts
- packages/router-generator/tests/generator/escaped-special-strings/routeTree.snapshot.ts
- packages/router-generator/tests/generator/virtual-config-file-named-export/routeTree.snapshot.ts
- packages/router-generator/tests/generator/dot-escaped/routeTree.snapshot.ts
- packages/router-generator/tests/generator/flat/routeTree.snapshot.ts
- packages/router-generator/tests/generator/virtual-pathless-layout-escaped-underscore/routeTree.snapshot.ts
- packages/router-generator/tests/generator/virtual-config-file-tsconfig-paths/routeTree.snapshot.ts
- packages/router-generator/tests/generator/custom-tokens/routeTree.snapshot.ts
- packages/router-generator/tests/generator/nested/routeTree.snapshot.ts
- packages/router-generator/tests/generator/nested-layouts/routeTree.snapshot.ts
- packages/router-generator/tests/generator/physical-pathless-layout-escaped-underscore/routeTree.snapshot.ts
- packages/router-generator/tests/generator/route-groups/routeTree.snapshot.ts
Follow the AGENTS.md control-statement convention (no one-line if bodies).
Fixes #7750.
Problem
buildRouteTreesorts route nodes withmultiSortBy, whose final accessor was(d) => d— comparing whole route-node objects:multiSortBy's comparator ends withreturn ao > bo ? 1 : -1. For two distinct route-node objects, both coerce to"[object Object]", soao === boisfalseandao > boisfalse→ it returns-1for both(a, b)and(b, a). That's an inconsistent comparator, soArray.prototype.sortleaves routes that tie on the earlier keys (same segment-count, same index-token status — e.g./account/beta-featuresvs/account/busy-guard) in an order that depends on the engine's sort implementation, the input order, and the array size. The result:routeTree.gen.tsreorders non-deterministically across machines and Node versions, dirtying committed route trees with semantically-identical churn.Fix
Change the final tiebreaker from
(d) => dto(d) => d.routePath, mirroring the pre-sort earlier in the same file (which already ends with(d) => d.routePath).routePathis unique per node, so this gives a total order and makes generation byte-identical regardless of Node version or route-set composition.A changeset is included (
@tanstack/router-generator, patch).Tests
The
buildRouteTreetiebreaker sort is extracted into an exported puresortRouteNodes(routeNodes, indexTokenSegmentRegex)helper (behavior-identical to the previous inlinemultiSortBycall) so it can be unit-tested directly. New tests inrouter-generator/tests/utils.test.ts:routePath(e.g./account/beta-featuresvs/account/busy-guard,/aboutvs/posts) through every input permutation and asserts identical output, proving the sort is independent of input order, engine sort implementation, and array size. This test fails against the old(d) => dtiebreaker and passes against(d) => d.routePath.routePath, and that root / segment-count / index-token precedence still apply ahead of the tiebreaker.Regenerated snapshots
The deterministic tiebreaker changes the order in which route nodes are emitted, so the 38 committed
routeTree.snapshot.*files undertests/generator/are regenerated. This is expected: the previous snapshots captured the arbitrary order the inconsistent comparator happened to produce on the machine that generated them. The buggy sort was stable run-to-run for a fixed input order and engine, so CI kept passing — the non-determinism only appeared across machines/Node versions (issue #7750).The snapshot diffs are pure emission-order reordering: every route's
id,path, andparentRouteis unchanged, and the route-tree structure (built fromacc.routeTree, not the sorted list) is untouched — only the order of imports and route declarations shifts. In other words, the regenerated snapshots are exactly the semantically-identical churn this fix eliminates going forward, applied once.Summary by CodeRabbit