feat: cleaner git diffs #698
Conversation
WalkthroughAdds a fifth generic parameter to RouteRecordInfo to represent child-route unions and switches emitted unions to multiline (leading-pipe) formatting across codegen, docs, playground, tests, and utils. Introduces TreeNodeNamed/isNamed and helper formatting utilities. No runtime behavior changes. (≤50 words) Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
commit: |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #698 +/- ##
==========================================
+ Coverage 60.90% 61.24% +0.33%
==========================================
Files 36 36
Lines 3379 3406 +27
Branches 618 623 +5
==========================================
+ Hits 2058 2086 +28
+ Misses 1314 1313 -1
Partials 7 7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
playground/typed-router.d.ts (3)
287-293: Use plain never for the 5th generic; it keeps intent clearFor RouteRecordInfo’s 5th generic, using plain never for leaves is idiomatic and still yields a one-line diff when a child is later added (switching to a piped union on that single line). I’d keep it as is.
462-465: Consider piping never in multiline union fields to stabilize future diffsFor _RouteFileInfoMap’s union-shaped fields (routes/views), using a piped never maintains the same block shape when going from no entries to one entry (only one line changes). Example:
- views: never + views: + | neverIf you agree, apply this pattern to all views (and any similar multiline union fields) in the generator.
454-461: Generator alignment: use the same block style for views everywhereThere are many views: never occurrences. If you switch to a piped never format in the generator, all these spots will benefit from cleaner future diffs. No need to hand-edit this file; just adjust the codegen once.
Also applies to: 466-471, 472-477, 478-483, 484-488, 489-493, 494-498, 499-503, 504-508, 509-513, 514-518, 519-523, 532-536, 537-541, 542-546, 547-551, 552-556, 557-562, 563-567, 568-572, 573-577, 578-582, 583-587, 588-592, 593-597, 598-602, 603-607, 608-612, 613-617, 618-622, 623-627, 628-632, 633-637, 638-642, 643-647, 648-652, 653-657, 658-666, 667-671, 672-676, 677-681, 682-686, 687-691, 692-696, 697-701, 702-706, 707-711, 712-716, 717-721, 722-726, 727-731, 732-736, 737-741, 742-746, 747-751, 752-756, 757-761, 762-776, 777-781
src/codegen/generateRouteMap.spec.ts (2)
700-704: Remove debug logging from testsconsole.log in snapshots adds noise to CI logs. Remove it.
- console.log(result1.replaceAll(/\n\s+\|/g, ' |')) // Verify the union type is alphabetically sorted expect(result1.replaceAll(/\n\s+\|/g, ' |')).toContain( "| '/parent/alpha' | '/parent/beta' | '/parent/zebra'" )
696-704: Optional: factor the union flattener into a small helperIf you keep normalizing unions in more tests, extract the replaceAll to a helper for clarity.
const result1 = createTree(order1) const result2 = createTree(order2) const result3 = createTree(order3) // All should be identical due to stable sorting expect(result1).toBe(result2) expect(result2).toBe(result3) - // Verify the union type is alphabetically sorted - expect(result1.replaceAll(/\n\s+\|/g, ' |')).toContain( + const flat = (s: string) => s.replaceAll(/\n\s+\|/g, ' |') + // Verify the union type is alphabetically sorted + expect(flat(result1)).toContain( "| '/parent/alpha' | '/parent/beta' | '/parent/zebra'" )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
docs/.vitepress/twoslash-files.ts(1 hunks)docs/guide/typescript.md(1 hunks)playground/typed-router.d.ts(2 hunks)src/codegen/generateRouteFileInfoMap.spec.ts(4 hunks)src/codegen/generateRouteFileInfoMap.ts(1 hunks)src/codegen/generateRouteMap.spec.ts(20 hunks)src/codegen/generateRouteMap.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
playground/typed-router.d.ts (1)
src/codegen/generateRouteParams.ts (4)
ParamValue(55-57)ParamValueOneOrMore(30-33)ParamValueZeroOrMore(39-41)ParamValueZeroOrOne(47-49)
src/codegen/generateRouteFileInfoMap.ts (3)
client.d.ts (1)
routes(7-7)src/core/extendRoutes.ts (2)
name(88-90)name(95-97)src/core/tree.ts (1)
name(238-245)
🔇 Additional comments (14)
src/codegen/generateRouteFileInfoMap.ts (1)
45-46: Consistent multiline formatting for routes and views.The new multiline union formatting with pipe-prefixed entries improves diff readability. The conditional handling correctly keeps
neveron a single line while formatting multiple values with leading pipes.src/codegen/generateRouteMap.ts (4)
30-31: Initialize child routes variable.Clean initialization of
childRouteNameswithneveras the default value.
43-46: Generate multiline union for child routes.The formatting correctly creates pipe-prefixed multiline unions for child routes, consistent with the PR's objective of cleaner Git diffs.
49-49: Add fifth type parameter for child routes.Correctly appends the child routes as the fifth generic parameter to
RouteRecordInfo.
51-51: Format type parameters on separate lines.The multiline formatting with proper indentation produces cleaner, more readable TypeScript output that will generate better Git diffs.
src/codegen/generateRouteFileInfoMap.spec.ts (2)
26-27: Test snapshots updated for single-route multiline format.The snapshots correctly expect even single routes to be formatted as multiline unions with pipe prefixes, maintaining consistency with the new formatting approach.
Also applies to: 31-32, 36-37, 41-42
57-62: Test snapshots updated for multi-route and view unions.The snapshots correctly reflect the new multiline union formatting for both routes and views, with proper pipe prefixing and indentation.
Also applies to: 64-65, 81-86, 88-90, 94-95, 115-117, 121-123
docs/guide/typescript.md (1)
66-66: Documentation updated to show pipe-prefixed union syntax.The documentation correctly demonstrates the new union format for child routes using the pipe prefix, consistent with the generated code changes.
docs/.vitepress/twoslash-files.ts (1)
34-34: Twoslash example updated to pipe-prefixed union format.The example correctly demonstrates the new fifth type parameter format using the pipe-prefixed union syntax for child routes.
playground/typed-router.d.ts (1)
21-27: 5th generic addition and multiline unions: looks goodRouteRecordInfo’s 5th type parameter (children) is correctly introduced and the multiline rendering is clear and consistent.
src/codegen/generateRouteMap.spec.ts (4)
24-31: Snapshots correctly reflect the 5th generic and multiline formattingThe updated snapshots accurately capture the new children parameter and the piped-union layout. Good coverage on root and static paths.
Also applies to: 31-52
368-399: Nested index case looks correctThe parent has a children union and both index routes are leaves with never. Matches the intended behavior.
410-427: Parent path override test is preciseChildren union and overridden path are asserted correctly. This guards against regressions in path overrides.
741-773: Children union filtering test reads wellEmpty-name children are excluded from the parent’s children union; assertions are clear and specific.
Anoesj
force-pushed
the
feat/cleaner-git-diffs
branch
from
fe29fc6 to
0841939
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/codegen/generateRouteMap.ts (2)
30-31: Default toneveris fine; consider| neverif you want single-line diffs when first child is addedUsing
neverkeeps the output cleaner. If you want to avoid the two-line change when adding the first child, emit| neverinstead to preserve the multiline-with-leading-pipe shape from the start.Apply if desired:
- let childRouteNames = 'never' + let childRouteNames = '| never'
43-45: Extract “leading-pipe union” formatting into a small helper to keep codegen consistentThis formatting logic appears in multiple generators. Centralizing it reduces drift and makes indentation changes trivial.
Use a helper:
// place near the top of the file (or in a shared util) function formatLeadingPipeUnion(items: string[], indent = 4): string { const pad = ' '.repeat(indent) return items.map((s) => `| ${s}`).join('\n' + pad) }Then update here:
- childRouteNames = deepNamedChildren - .map((childRouteName) => `| ${childRouteName}`) - .join('\n ') + childRouteNames = formatLeadingPipeUnion(deepNamedChildren, 4)playground/typed-router.d.ts (1)
454-460: Multiline unions forroutes/viewsreduce churn; consider decision onnevershapeThe consistent “leading-pipe” shape for unions is good. Decide whether
views: nevershould become:
views: never(current, clean), or- views:
| never
to keep the shape identical and minimize line changes when adding the first view.If you want to switch to the multiline shape, the generator can emit
| neverby default. Would you like me to prepare the patch across the file-info generator as well?Also applies to: 471-477, 524-531, 658-666
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/.vitepress/twoslash-files.ts(1 hunks)docs/guide/typescript.md(1 hunks)playground/typed-router.d.ts(2 hunks)src/codegen/generateRouteMap.spec.ts(20 hunks)src/codegen/generateRouteMap.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/.vitepress/twoslash-files.ts
- docs/guide/typescript.md
- src/codegen/generateRouteMap.spec.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
playground/typed-router.d.ts (1)
src/codegen/generateRouteParams.ts (4)
ParamValue(55-57)ParamValueOneOrMore(30-33)ParamValueZeroOrMore(39-41)ParamValueZeroOrOne(47-49)
🔇 Additional comments (4)
src/codegen/generateRouteMap.ts (1)
49-52: Multiline RouteRecordInfo formatting reads well and is stableThe indentation and comma placement produce tidy, low-churn diffs. Sorting child names ensures stable output.
playground/typed-router.d.ts (3)
21-28: RouteRecordInfo now carries child-route relationships cleanlyThe 5th type arg is emitted as either
neveror a leading-pipe union. This matches the PR’s goals and keeps diffs stable.Also applies to: 35-42
287-299: Representative example with a single child looks correctLeading-pipe union under the 5th generic parameter parses fine and matches the intended nest semantics.
21-441: Leading-pipe ‘|’ syntax supported since TS 2.1.3
TypeScript has accepted a leading|/&for unions/intersections since November 2016 (TS 2.1.3/2.1.4). If your codebase and tooling (Volar, tsserver, etc.) target TS >= 2.1.3, the generated leading pipes are safe. Otherwise, consider removing the leading|.
- Please verify your minimum TypeScript version (and any consumers’ tooling) is >= 2.1.3.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/utils/index.ts (2)
24-26: Prefer a simpler, safer pad implementationUsing padStart works but is a bit opaque and can behave unexpectedly with negative indents. A direct space repeat is clearer and guards negatives.
-export function pad(indent: number, str = ''): string { - return str.padStart(str.length + indent) -} +export function pad(indent: number, str = ''): string { + const n = Math.max(0, indent) + return ' '.repeat(n) + str +}
34-36: Guard empty unions to keep output stableIf items is ever empty, this returns an empty string. Given the PR’s goal (stable diffs and explicit never), consider defaulting to a single never line.
-export function formatMultilineUnion(items: string[], indent: number): string { - return items.map((s) => `| ${s}`).join(`\n${pad(indent)}`) -} +export function formatMultilineUnion(items: string[], indent: number): string { + const list = items.length ? items : ['never'] + return list.map((s) => `| ${s}`).join(`\n${pad(indent)}`) +}Can you confirm callers never pass an empty array today (e.g., for views/routes unions)? If they can, the above change would align output with the intended “always explicit never” style.
src/codegen/generateDTS.ts (1)
13-13: Indentation via pad keeps behavior and intent intactSwitching from string concatenation to pad(2, line) preserves two-space indent and reads clearer.
If we want to improve readability of the generated DTS (given the FIXME), we could stop stripping empty lines here and let codegen control whitespace. Not blocking this PR.
src/codegen/generateRouteMap.ts (1)
31-47: Minor simplification of child route names computationLogic is correct. You can simplify by deriving childRouteNames once without a mutable default.
- let childRouteNames = ['never'] - - if (node.children.size > 0) { - // TODO: remove Array.from() once Node 20 support is dropped - const deepNamedChildren = Array.from(node.getChildrenDeep()) - // skip routes that are not added to the types - .filter( - (childRoute) => childRoute.value.components.size > 0 && childRoute.name - ) - .map((childRoute) => childRoute.name) - .sort() - - if (deepNamedChildren.length > 0) { - childRouteNames = deepNamedChildren.map( - (childRouteName) => `'${childRouteName}'` - ) - } - } + const deepChildrenNames = + node.children.size > 0 + ? Array.from(node.getChildrenDeep()) + .filter( + (r) => r.value.components.size > 0 && r.name + ) + .map((r) => r.name!) + .sort() + : [] + const childRouteNames = + deepChildrenNames.length > 0 + ? deepChildrenNames.map((n) => `'${n}'`) + : ['never']Optional: once Node ≥20 (or TS lib with ES2023) is baseline, you can switch to toSorted() and drop Array.from() if getChildrenDeep() already returns an array-like iterator.
src/codegen/generateRouteMap.spec.ts (1)
701-703: replaceAll with a global regex is fine; confirm CI Node versionThis normalization is concise and readable. Ensure the test runner uses a Node version that supports String.prototype.replaceAll (Node ≥15).
If Node <15 needs support, replace with .replace and a loop or split/join:
- expect(result1.replaceAll(/\n\s+\|/g, ' |')).toContain( + expect(result1.replace(/\n\s+\|/g, ' |')).toContain( "| '/parent/alpha' | '/parent/beta' | '/parent/zebra'" )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
docs/.vitepress/twoslash-files.ts(1 hunks)docs/.vitepress/twoslash/code/typed-router.ts(2 hunks)docs/guide/typescript.md(1 hunks)docs/introduction.md(1 hunks)playground/src/router.ts(2 hunks)playground/typed-router.d.ts(2 hunks)src/codegen/generateDTS.ts(2 hunks)src/codegen/generateRouteFileInfoMap.spec.ts(4 hunks)src/codegen/generateRouteFileInfoMap.ts(2 hunks)src/codegen/generateRouteMap.spec.ts(20 hunks)src/codegen/generateRouteMap.ts(3 hunks)src/utils/index.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- playground/src/router.ts
- docs/introduction.md
🚧 Files skipped from review as they are similar to previous changes (4)
- src/codegen/generateRouteFileInfoMap.ts
- docs/.vitepress/twoslash-files.ts
- docs/guide/typescript.md
- src/codegen/generateRouteFileInfoMap.spec.ts
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/codegen/generateDTS.ts (1)
src/utils/index.ts (1)
pad(24-26)
docs/.vitepress/twoslash/code/typed-router.ts (1)
src/codegen/generateRouteParams.ts (1)
ParamValue(55-57)
playground/typed-router.d.ts (1)
src/codegen/generateRouteParams.ts (4)
ParamValue(55-57)ParamValueOneOrMore(30-33)ParamValueZeroOrMore(39-41)ParamValueZeroOrOne(47-49)
src/codegen/generateRouteMap.ts (1)
src/utils/index.ts (2)
pad(24-26)formatMultilineUnion(34-36)
🔇 Additional comments (10)
src/codegen/generateDTS.ts (1)
1-1: Importing pad centralizes indentation logic — LGTMKeeps formatting helpers in one place and avoids ad-hoc spacing.
src/codegen/generateRouteMap.ts (2)
15-16: Consistent indentation of entries — LGTMUsing pad(2, ...) for each RouteNamedMap entry keeps the root wrapper simple and the output consistent.
50-54: Multiline generics formatting achieves the “leading pipe” style — LGTMformatMultilineUnion + pad produces the intended output and stable diffs.
docs/.vitepress/twoslash/code/typed-router.ts (1)
18-19: Leading-pipe unions in docs match the generator — LGTMThis mirrors the codegen output and helps reduce diff churn in documentation examples.
Open question from the PR: should entries with no members use “views: never” or multiline “views: | never”? Please decide for consistency across codegen, tests, and docs; I can update references accordingly.
Also applies to: 25-26, 32-33, 39-40
src/codegen/generateRouteMap.spec.ts (1)
24-51: Snapshots reflect the multiline union style — LGTMThe updated expectations for RouteRecordInfo’s 5th parameter and leading pipes match the new generator behavior.
playground/typed-router.d.ts (5)
21-27: Fifth parameter added to RouteRecordInfo type.The new parameter represents child route relationships. The multiline union format with leading pipe improves git diff readability.
26-26: Consistent use of| neverfor routes without children.Most routes correctly use
| neverfor the fifth parameter when they have no child routes. This maintains consistency across the RouteNamedMap.Also applies to: 33-34, 40-40, 47-47, 54-54, 61-61, 68-68, 75-75, 82-82, 89-89, 96-96, 103-103, 110-110, 117-117, 124-124, 131-131, 138-138, 145-145, 152-152, 159-159, 166-166, 173-173, 180-180, 187-187, 194-194, 201-201, 208-208, 215-215, 222-222, 229-229, 236-236, 243-243, 250-250, 257-257, 264-264, 271-271, 278-278, 285-285, 299-299, 306-306, 313-313, 320-320, 327-327, 334-334, 341-341, 348-348, 355-355, 362-362, 369-369, 376-376, 383-383, 390-390, 397-397, 404-404, 411-411, 418-418, 425-425, 432-432, 439-439
292-292: Child route relationship correctly represented.Route '/named-views/parent' properly shows its child route '/named-views/parent/' in the fifth parameter union.
455-460: Multiline union formatting improves diff readability.The _RouteFileInfoMap now uses consistent multiline union formatting with leading pipes for both
routesandviews. Entries without views correctly useviews: | never.Also applies to: 462-466, 468-472, 474-479, 481-485, 487-492, 494-498, 500-504, 506-510, 512-516, 518-522, 524-528, 530-534, 536-542, 544-548, 550-554, 556-560, 562-566, 568-572, 574-579, 581-585, 587-591, 593-597, 599-603, 605-609, 611-615, 617-621, 623-627, 629-633, 635-639, 641-645, 647-651, 653-657, 659-663, 665-669, 671-675, 677-681, 683-687, 689-693, 695-702, 704-708, 710-714, 716-720, 722-726, 728-732, 734-738, 740-744, 746-750, 752-756, 758-762, 764-768, 770-774, 776-780, 782-786, 788-792, 794-798, 800-804, 806-810, 812-816, 818-822, 824-828, 830-834, 836-840
465-465: Consistentviews: | nevernotation.All entries without views use the consistent
views: | neverformat rather thanviews: never, maintaining uniformity with the multiline union style.Also applies to: 471-471, 484-484, 491-491, 497-497, 503-503, 509-509, 515-515, 521-521, 527-527, 533-533, 547-547, 553-553, 559-559, 565-565, 571-571, 578-578, 584-584, 590-590, 596-596, 602-602, 608-608, 614-614, 620-620, 626-626, 632-632, 638-638, 644-644, 650-650, 656-656, 662-662, 668-668, 674-674, 680-680, 686-686, 692-692, 707-707, 713-713, 719-719, 725-725, 731-731, 737-737, 743-743, 749-749, 755-755, 761-761, 767-767, 773-773, 779-779, 785-785, 791-791, 797-797, 803-803, 809-809, 815-815, 821-821, 827-827, 833-833, 839-839
…d add `stringToStringType` utility
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/codegen/generateDTS.ts(2 hunks)src/codegen/generateRouteFileInfoMap.ts(2 hunks)src/codegen/generateRouteMap.ts(3 hunks)src/utils/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/codegen/generateDTS.ts
- src/codegen/generateRouteFileInfoMap.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/codegen/generateRouteMap.ts (2)
src/utils/index.ts (3)
indent(24-26)formatMultilineUnion(34-38)stringToStringType(47-49)src/core/tree.ts (1)
TreeNode(15-304)
🔇 Additional comments (5)
src/utils/index.ts (2)
24-26: LGTM!Clean implementation of the indent function.
34-38: LGTM!Well-designed function that handles empty arrays correctly by defaulting to
['never'].src/codegen/generateRouteMap.ts (3)
31-42: LGTM!Good use of type guard to ensure type safety when filtering child routes.
44-46: LGTM!Clean composition of utility functions to format child routes.
48-50: LGTM!Consistent multiline formatting improves readability and reduces diff noise.
…` method, minor optimizations
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
src/codegen/generateRouteFileInfoMap.ts (1)
41-52: Deduplicate/sort unions and sort file keys for deterministic, minimal diffsPrevents duplicated union members and stabilizes order across runs/changes. Also sorts file keys to keep the interface deterministic.
- const code = Array.from(routesInfo.entries()) - .map( - ([file, { routes, views }]) => - ` - '${file}': { - routes: - ${formatMultilineUnion(routes.map(stringToStringType), 6)} - views: - ${formatMultilineUnion(views.map(stringToStringType), 6)} - }` - ) - .join('\n') + const code = Array.from(routesInfo.entries()) + .sort(([a], [b]) => a.localeCompare(b)) + .map(([file, { routes, views }]) => { + const routesUnion = [...new Set(routes)].sort().map(stringToStringType) + const viewsUnion = [...new Set(views)].sort().map(stringToStringType) + return ` + '${file}': { + routes: + ${formatMultilineUnion(routesUnion, 6)} + views: + ${formatMultilineUnion(viewsUnion, 6)} + }` + }) + .join('\n')
🧹 Nitpick comments (4)
src/codegen/generateRouteFileInfoMap.ts (1)
80-88: Avoid duplicate route names; use a Set during collectionEnsures uniqueness at the source and keeps output stable.
- const routeNames = [node, ...node.getChildrenDeepSorted()] - // an unnamed route cannot be accessed in types - .reduce<string[]>((acc, node) => { - if (node.isNamed()) { - acc.push(node.name) - } - return acc - }, []) + const routeNames = Array.from( + [node, ...node.getChildrenDeepSorted()].reduce<Set<string>>((set, node) => { + if (node.isNamed()) set.add(node.name) + return set + }, new Set<string>()) + )src/core/tree.ts (1)
297-300: isNamed() guard is correct; add a clarifying doc commentTruthiness captures empty-string and false overrides as “not named”. A short JSDoc will prevent confusion.
Apply this diff:
+ /** True when the computed name is a non-empty string. + * Empty string or `false` overrides are considered "not named". + */ isNamed(): this is TreeNodeNamed { return !!this.name }src/core/tree.spec.ts (1)
407-445: Consider adding a null-name override test to match the implementation commentThe code comment in TreeNode.get name() mentions null as a way to remove a name. Add a test to lock this in.
You could extend this test with:
// Set null name // If types allow null on overrides: node.value.setOverride('', { name: null as any }) expect(node.name).toBe(null) expect(node.isNamed()).toBe(false)If the route block type doesn’t currently allow null, either align the comment or widen the type accordingly and add this test.
src/codegen/generateRouteMap.ts (1)
34-47: Deduplicate child route names before building the unionUnlikely but possible to encounter duplicates; Set-based dedupe keeps unions minimal and deterministic.
Apply this diff:
- const childRouteNames: string[] = - node.children.size > 0 - ? // TODO: remove Array.from() once Node 20 support is dropped - Array.from(node.getChildrenDeep()) - // skip routes that are not added to the types - .reduce<string[]>((acc, childRoute) => { - if (childRoute.value.components.size && childRoute.isNamed()) { - acc.push(childRoute.name) - } - return acc - }, []) - .sort() - : [] + const childRouteNames: string[] = + node.children.size > 0 + ? Array.from( + new Set( + // TODO: remove Array.from() once Node 20 support is dropped + Array.from(node.getChildrenDeep()) + .filter((c) => c.value.components.size && c.isNamed()) + .map((c) => c.name) + ) + ).sort() + : []
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (7)
src/codegen/generateDTS.ts(2 hunks)src/codegen/generateRouteFileInfoMap.ts(3 hunks)src/codegen/generateRouteMap.ts(2 hunks)src/codegen/generateRouteRecords.ts(3 hunks)src/core/tree.spec.ts(2 hunks)src/core/tree.ts(2 hunks)src/utils/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/utils/index.ts
- src/codegen/generateDTS.ts
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/core/tree.ts (1)
src/types.ts (1)
TreeNode(7-7)
src/codegen/generateRouteRecords.ts (1)
src/utils/index.ts (2)
pad(24-26)stringToStringType(47-49)
src/codegen/generateRouteFileInfoMap.ts (2)
src/utils/index.ts (2)
formatMultilineUnion(34-38)stringToStringType(47-49)client.d.ts (1)
routes(7-7)
src/codegen/generateRouteMap.ts (3)
src/utils/index.ts (3)
pad(24-26)stringToStringType(47-49)formatMultilineUnion(34-38)src/core/tree.ts (1)
TreeNodeNamed(15-17)src/codegen/generateRouteParams.ts (1)
generateRouteParams(3-22)
🔇 Additional comments (9)
src/codegen/generateRouteFileInfoMap.ts (1)
3-3: Imports LGTMUtility usage aligns with the new multiline-union formatting approach.
src/core/tree.ts (1)
15-18: Type alias is precise and fits the intended narrowingTreeNodeNamed correctly narrows name to string for use with the isNamed() guard and downstream codegen.
src/core/tree.spec.ts (1)
390-405: Good coverage for custom names with the new isNamed() APIAsserting both node.name and node.isNamed() after overrides validates the guard behavior well.
src/codegen/generateRouteRecords.ts (2)
5-5: Importing pad/stringToStringType improves consistencyCentralizing spacing and string literal formatting is a good move toward cleaner diffs.
52-54: Indentation via pad() is cleaner and less error-proneThis reduces manual spacing logic and makes future tweaks trivial.
src/codegen/generateRouteMap.ts (4)
1-1: Type narrowing import looks goodUsing TreeNodeNamed pairs well with the isNamed() predicate.
3-3: Utility imports match the new multiline union formattingformatMultilineUnion and pad use is appropriate for leading-pipe unions and consistent spacing.
48-55: Multiline union and multiline type args look greatformatMultilineUnion with a leading pipe meets the “cleaner diffs” goal and reads well.
26-26: No external callers for generateRouteRecordInfoThe only invocation of
generateRouteRecordInfois ingenerateRouteNamedMap(line 11), immediately guarded bynode.isNamed(). There are no other usages in the repo, so updating the signature toTreeNodeNamedwon’t break external call sites.
Every
RouteRecordInfoentry is now multiline. The child routes are always set and written over multiple lines using pipes (|), and I now explicitly set| nevertoo to prevent annoying Git diffs when for example this......is changed to this in any commit
With
| neverexplicitly set as the fifth generic, only one line is changed in the Git diff instead of two.In
_RouteFileInfoMap, bothviewsandroutesare now multiline unions with pipes:Full example output
To do
Summary by CodeRabbit