fix(router-core): add support for handling any kind of empty _splat consistently #5510
Conversation
|
Warning Rate limit exceeded@nlynzaad has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 5 minutes and 7 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughTreats empty or falsy Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Link / Navigate / Router
participant Interpolator as interpolatePath()
participant Config as trailingSlashOptions
Note over Caller,Interpolator: Build path for a splat route (template contains `$`)
Caller->>Interpolator: interpolate(pathTemplate, params, leaveWildcards?)
alt params._splat is truthy
Interpolator->>Interpolator: encode & insert splat segment
Interpolator-->>Caller: path including encoded splat
else params._splat is missing or falsy
Interpolator->>Interpolator: treat splat as missing -> omit wildcard segment/prefix
Interpolator-->>Caller: base path without splat
end
Caller->>Config: read trailingSlash option
Config-->>Caller: append or omit trailing slash according to option
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
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 |
paul-sachs
changed the title
|
for which scenarios is this relevant? navigating an setting |
|
thanks for the unit tests on path and Link, as Manuel mentioned please add tests for navigate as well, both useNavigate and navigate, just to ensure consistency. Apart from that it's a go from me. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/react-router/tests/navigate.test.tsx (3)
1250-1250: KeepcreateRootRouteusage consistent.Elsewhere in this file you call
createRootRoute({}). Mirror that here to avoid accidental reliance on defaulted options.Apply:
- const rootRoute = createRootRoute() + const rootRoute = createRootRoute({})
1270-1287: Add a case that clears an existing non-empty splat.Also verify that navigating from "/splat/foo" (or deeper) to an empty/omitted
_splatlands on "/splat" (or "/splat/") pertrailingSlash. This guards interpolation when removing segments, not just building from root.Append inside this
describe('splat routes with empty splat', ...):+ it.each([{ trailingSlash: true }, { trailingSlash: false }])( + 'clears a non-empty _splat with trailingSlash: $trailingSlash', + async ({ trailingSlash }) => { + const tail = trailingSlash ? '/' : '' + const history = createMemoryHistory({ initialEntries: ['/splat/foo/bar'] }) + const rootRoute = createRootRoute({}) + const indexRoute = createRoute({ getParentRoute: () => rootRoute, path: '/' }) + const splatRoute = createRoute({ getParentRoute: () => rootRoute, path: 'splat/$' }) + const router = createRouter({ + routeTree: rootRoute.addChildren([indexRoute, splatRoute]), + history, + trailingSlash: trailingSlash ? 'always' : 'never', + }) + await router.load() + // Clear via '', undefined, and omission + for (const params of [{ _splat: '' }, { _splat: undefined }, {}]) { + await router.navigate({ to: '/splat/$', params }) + await router.invalidate() + expect(router.state.location.pathname).toBe(`/splat${tail}`) + } + }, + )
1242-1290: Add auseNavigateparity test.Reviewers asked for both programmatic
navigateand hook-based navigation. Please add a React-level test that callsuseNavigate()with{ to: '/splat/$', params: { _splat: undefined } }and asserts the same pathname outcomes for 'always'/'never'. I can help wire a minimal render harness if you don’t have one handy.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/react-router/tests/navigate.test.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/react-router/tests/navigate.test.tsx
packages/{react-router,solid-router}/**
📄 CodeRabbit inference engine (AGENTS.md)
Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/
Files:
packages/react-router/tests/navigate.test.tsx
🧠 Learnings (1)
📚 Learning: 2025-10-08T08:11:47.088Z
Learnt from: nlynzaad
PR: TanStack/router#5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.
Applied to files:
packages/react-router/tests/navigate.test.tsx
🧬 Code graph analysis (1)
packages/react-router/tests/navigate.test.tsx (2)
packages/history/src/index.ts (1)
createMemoryHistory(568-614)packages/react-router/src/router.ts (1)
createRouter(80-82)
🔇 Additional comments (1)
packages/react-router/tests/navigate.test.tsx (1)
1242-1290: Good coverage for empty/undefined/omitted _splat across trailingSlash policies.Scenario matrix and back-to-index reset make the assertions robust. This directly guards against "/undefined" regressions and enforces 'always' vs 'never'.
|
@schiller-manuel Added additional test coverage. I followed the suggestions of CodeRabbit as well, to better cover |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/react-router/tests/navigate.test.tsx (1)
1242-1290: Great coverage for empty/undefined/omitted_splatacross all trailingSlash modes.Behavior matches the spec and prevents
/undefinedregressions. Optional: factor the matrix into a typed constant to drop the casts and reuse across files:const trailingSlashCases = (['preserve','always','never'] as const).map((t) => ({ trailingSlash: t }))packages/react-router/tests/useNavigate.test.tsx (1)
2559-2680: Solid navigate-path coverage for empty_splatvia useNavigate.Asserts match expectations for 'preserve'/'always'/'never' and properly reset between runs. Consider extracting a shared typed matrix to remove repeated casts, mirroring the navigate and link tests.
packages/react-router/tests/link.test.tsx (1)
5552-5643: Href + active-state assertions for splat links are on point.This closes the loop on Link behavior for empty/undefined/omitted
_splat. If you want to DRY the three files, consider a small helper to supply the trailingSlash cases and expected tail.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/react-router/tests/link.test.tsx(2 hunks)packages/react-router/tests/navigate.test.tsx(2 hunks)packages/react-router/tests/useNavigate.test.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/react-router/tests/useNavigate.test.tsxpackages/react-router/tests/navigate.test.tsxpackages/react-router/tests/link.test.tsx
packages/{react-router,solid-router}/**
📄 CodeRabbit inference engine (AGENTS.md)
Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/
Files:
packages/react-router/tests/useNavigate.test.tsxpackages/react-router/tests/navigate.test.tsxpackages/react-router/tests/link.test.tsx
🧠 Learnings (2)
📚 Learning: 2025-09-28T21:41:45.233Z
Learnt from: nlynzaad
PR: TanStack/router#5284
File: e2e/react-start/basic/server.js:50-0
Timestamp: 2025-09-28T21:41:45.233Z
Learning: In Express v5, catch-all routes must use named wildcards. Use `/*splat` to match everything except root path, or `/{*splat}` (with braces) to match including root path. The old `*` syntax is not allowed and will cause "Missing parameter name" errors. This breaking change requires explicit naming of wildcard parameters.
Applied to files:
packages/react-router/tests/useNavigate.test.tsxpackages/react-router/tests/navigate.test.tsx
📚 Learning: 2025-10-08T08:11:47.088Z
Learnt from: nlynzaad
PR: TanStack/router#5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.
Applied to files:
packages/react-router/tests/navigate.test.tsxpackages/react-router/tests/link.test.tsx
🧬 Code graph analysis (3)
packages/react-router/tests/useNavigate.test.tsx (1)
packages/router-core/src/router.ts (1)
TrailingSlashOption(800-800)
packages/react-router/tests/navigate.test.tsx (3)
packages/router-core/src/router.ts (1)
TrailingSlashOption(800-800)packages/react-router/src/index.tsx (5)
TrailingSlashOption(80-80)createMemoryHistory(118-118)createRootRoute(258-258)createRoute(255-255)createRouter(273-273)packages/history/src/index.ts (1)
createMemoryHistory(568-614)
packages/react-router/tests/link.test.tsx (2)
packages/router-core/src/router.ts (1)
TrailingSlashOption(800-800)packages/react-router/src/router.ts (1)
createRouter(80-82)
🔇 Additional comments (3)
packages/react-router/tests/navigate.test.tsx (1)
9-9: Type import looks good.Importing TrailingSlashOption from the public surface is correct and keeps tests strongly typed.
packages/react-router/tests/useNavigate.test.tsx (1)
26-26: Type import is appropriate.Using TrailingSlashOption here keeps the parameterized tests precise.
packages/react-router/tests/link.test.tsx (1)
44-44: Type import is fine.Keeps tests aligned with the public API surface.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/react-router/tests/link.test.tsx (1)
5553-5644: LGTM: Thorough Link component test for empty splat handling.This test validates that the Link component generates consistent hrefs for empty,
undefined, and omitted_splatparameters across alltrailingSlashconfigurations. The activation check (lines 5637-5639) confirms the router correctly recognizes all three link variants as pointing to the same route.Potential duplication concern: The AI summary indicates this test suite may be duplicated in the file. If so, please remove the duplicate to avoid redundant test execution and maintenance overhead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/react-router/tests/link.test.tsx(2 hunks)packages/react-router/tests/navigate.test.tsx(2 hunks)packages/react-router/tests/useNavigate.test.tsx(3 hunks)packages/react-router/tests/utils.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/react-router/tests/useNavigate.test.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/react-router/tests/utils.tspackages/react-router/tests/navigate.test.tsxpackages/react-router/tests/link.test.tsx
packages/{react-router,solid-router}/**
📄 CodeRabbit inference engine (AGENTS.md)
Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/
Files:
packages/react-router/tests/utils.tspackages/react-router/tests/navigate.test.tsxpackages/react-router/tests/link.test.tsx
🧠 Learnings (1)
📓 Common learnings
Learnt from: nlynzaad
PR: TanStack/router#5284
File: e2e/react-start/basic/server.js:50-0
Timestamp: 2025-09-28T21:41:45.233Z
Learning: In Express v5, catch-all routes must use named wildcards. Use `/*splat` to match everything except root path, or `/{*splat}` (with braces) to match including root path. The old `*` syntax is not allowed and will cause "Missing parameter name" errors. This breaking change requires explicit naming of wildcard parameters.
🧬 Code graph analysis (2)
packages/react-router/tests/navigate.test.tsx (2)
packages/react-router/tests/utils.ts (1)
trailingSlashCases(58-58)packages/history/src/index.ts (1)
createMemoryHistory(568-614)
packages/react-router/tests/link.test.tsx (1)
packages/react-router/tests/utils.ts (1)
trailingSlashCases(58-58)
🔇 Additional comments (2)
packages/react-router/tests/utils.ts (1)
57-58: LGTM: Clean test utility for trailing slash parameterization.This provides a single source of truth for testing all three trailing slash modes across the test suites. The const assertion ensures proper type inference.
packages/react-router/tests/navigate.test.tsx (1)
1243-1291: LGTM: Comprehensive test coverage for empty splat navigation.This test validates that empty string,
undefined, and omitted_splatparameters all produce consistent navigation behavior across alltrailingSlashconfigurations. The iteration through param sets with cleanup (navigating back to index) ensures test isolation.Note on
preservemode (line 1247): The test treats'preserve'the same as'never'(no trailing slash). This is likely correct for programmatic navigation where there's no "user-typed" URL to preserve, but worth confirming this aligns with the router's intended behavior for thepreservemode.
|
@paul-sachs thanks a lot for this. I made some minor updates:
|
|
View your CI Pipeline Execution ↗ for commit d55f44c
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/solid-router/tests/useNavigate.test.tsx (1)
2528-2647: Stabilize event timing; add defensive assertion
- Don’t wrap fireEvent.click/history.push inside waitFor. Click/push first, then waitFor on resulting state to reduce flakes.
- Also assert pathname never contains "undefined" to lock the regression.
Apply this representative diff to the three interactions:
- await waitFor(() => { - fireEvent.click(splatBtnWithEmptySplat) - }) + fireEvent.click(splatBtnWithEmptySplat) + await waitFor(() => expect(window.location.pathname).toBe(`/splat${tail}`)) + expect(window.location.pathname).not.toContain('undefined') - await waitFor(() => { - history.push('/') - }) + history.push('/') + await waitFor(() => expect(window.location.pathname).toBe('/'))packages/solid-router/tests/navigate.test.tsx (1)
572-624: Harden expectations for the splat permutationsCurrent checks are solid. Consider adding:
- A negative assertion against “undefined” in the path.
- Optional: type paramSets to make intent explicit.
Example tweak:
- const paramSets = [ + const paramSets: Array<Partial<{ _splat: string }>> = [ { _splat: '' }, { _splat: undefined }, {}, ] ... expect(router.state.location.pathname).toBe(`/splat${tail}`) +expect(router.state.location.pathname).not.toContain('undefined')packages/react-router/tests/link.test.tsx (1)
5553-5641: Strengthen assertions; minor readability
- Add a negative assertion to ensure no “undefined” leaks into href/path.
- Optional: factor expectedHref =
/splat${tail}to avoid repetition.Example additions:
expect(splatLinkWithEmptySplat.getAttribute('href')).toBe(`/splat${tail}`) +expect(splatLinkWithEmptySplat.getAttribute('href')).not.toMatch(/undefined/) ... expect(window.location.pathname).toBe(`/splat${tail}`) +expect(window.location.pathname).not.toMatch(/undefined/)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/react-router/tests/link.test.tsx(2 hunks)packages/react-router/tests/navigate.test.tsx(2 hunks)packages/react-router/tests/useNavigate.test.tsx(2 hunks)packages/router-core/src/index.ts(1 hunks)packages/router-core/src/router.ts(1 hunks)packages/solid-router/tests/link.test.tsx(2 hunks)packages/solid-router/tests/navigate.test.tsx(2 hunks)packages/solid-router/tests/useNavigate.test.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/router-core/src/index.tspackages/solid-router/tests/useNavigate.test.tsxpackages/react-router/tests/navigate.test.tsxpackages/router-core/src/router.tspackages/solid-router/tests/navigate.test.tsxpackages/react-router/tests/useNavigate.test.tsxpackages/solid-router/tests/link.test.tsxpackages/react-router/tests/link.test.tsx
packages/router-core/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep framework-agnostic core router logic in packages/router-core/
Files:
packages/router-core/src/index.tspackages/router-core/src/router.ts
packages/{react-router,solid-router}/**
📄 CodeRabbit inference engine (AGENTS.md)
Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/
Files:
packages/solid-router/tests/useNavigate.test.tsxpackages/react-router/tests/navigate.test.tsxpackages/solid-router/tests/navigate.test.tsxpackages/react-router/tests/useNavigate.test.tsxpackages/solid-router/tests/link.test.tsxpackages/react-router/tests/link.test.tsx
🧠 Learnings (1)
📚 Learning: 2025-10-08T08:11:47.088Z
Learnt from: nlynzaad
PR: TanStack/router#5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.
Applied to files:
packages/solid-router/tests/useNavigate.test.tsxpackages/react-router/tests/navigate.test.tsxpackages/solid-router/tests/navigate.test.tsxpackages/solid-router/tests/link.test.tsx
🧬 Code graph analysis (7)
packages/solid-router/tests/useNavigate.test.tsx (2)
packages/router-core/src/router.ts (1)
trailingSlashOptions(800-804)packages/solid-router/src/useNavigate.tsx (1)
useNavigate(11-25)
packages/react-router/tests/navigate.test.tsx (3)
packages/router-core/src/router.ts (1)
trailingSlashOptions(800-804)packages/history/src/index.ts (1)
createMemoryHistory(568-614)packages/react-router/src/router.ts (1)
createRouter(91-93)
packages/router-core/src/router.ts (3)
packages/router-core/src/index.ts (2)
trailingSlashOptions(207-207)TrailingSlashOption(212-212)packages/react-router/src/index.tsx (1)
TrailingSlashOption(80-80)packages/solid-router/src/index.tsx (1)
TrailingSlashOption(79-79)
packages/solid-router/tests/navigate.test.tsx (1)
packages/solid-router/src/router.ts (1)
createRouter(76-78)
packages/react-router/tests/useNavigate.test.tsx (1)
packages/router-core/src/router.ts (1)
trailingSlashOptions(800-804)
packages/solid-router/tests/link.test.tsx (1)
packages/router-core/src/router.ts (1)
trailingSlashOptions(800-804)
packages/react-router/tests/link.test.tsx (2)
packages/router-core/src/router.ts (1)
trailingSlashOptions(800-804)packages/react-router/src/router.ts (1)
createRouter(91-93)
🔇 Additional comments (9)
packages/router-core/src/index.ts (1)
199-209: Expose trailingSlashOptions in public API — good additionMatches router.ts const and unblocks cross-package tests/imports. No issues.
packages/solid-router/tests/useNavigate.test.tsx (1)
13-13: Import source for trailingSlashOptions — OKPublic export is available; import looks correct.
packages/solid-router/tests/navigate.test.tsx (1)
3-3: Import of trailingSlashOptions — OKConsistent with router-core export.
packages/react-router/tests/link.test.tsx (1)
15-15: Import of trailingSlashOptions — OKMatches new public export.
packages/react-router/tests/navigate.test.tsx (1)
1243-1294: Great coverage for splat edge casesExercising empty/undefined/missing
_splatacross everytrailingSlashmode (viaObject.values(trailingSlashOptions)) is a nice way to lock the regression fix and guard future modes automatically.packages/solid-router/tests/link.test.tsx (1)
5089-5177: Solid link parity looks solidThanks for mirroring the React coverage here—asserting both the generated hrefs and active-state behaviour across the three
_splatpermutations gives us end-to-end confidence that the Solid bindings respect the core fix.packages/router-core/src/router.ts (1)
800-808: Nice single source for trailingSlash valuesDeriving
TrailingSlashOptionfrom thetrailingSlashOptionsconst keeps the runtime export and type union in lockstep, which makes the downstreamObject.values(...)usage in tests both ergonomic and type-safe.packages/react-router/tests/useNavigate.test.tsx (2)
13-14: LGTM!The import of
trailingSlashOptionsis correct and enables parameterized testing across all trailing slash configurations.
2565-2679: LGTM!The test implementation comprehensively covers the empty splat scenarios. The logic correctly handles all three edge cases (empty string, undefined, and missing
_splat), verifies the pathname respects thetrailingSlashoption, and properly resets state between test cases for isolation.
More templates
@tanstack/arktype-adapter
@tanstack/directive-functions-plugin
@tanstack/eslint-plugin-router
@tanstack/history
@tanstack/nitro-v2-vite-plugin
@tanstack/react-router
@tanstack/react-router-devtools
@tanstack/react-router-ssr-query
@tanstack/react-start
@tanstack/react-start-client
@tanstack/react-start-server
@tanstack/router-cli
@tanstack/router-core
@tanstack/router-devtools
@tanstack/router-devtools-core
@tanstack/router-generator
@tanstack/router-plugin
@tanstack/router-ssr-query-core
@tanstack/router-utils
@tanstack/router-vite-plugin
@tanstack/server-functions-plugin
@tanstack/solid-router
@tanstack/solid-router-devtools
@tanstack/solid-router-ssr-query
@tanstack/solid-start
@tanstack/solid-start-client
@tanstack/solid-start-server
@tanstack/start-client-core
@tanstack/start-plugin-core
@tanstack/start-server-core
@tanstack/start-static-server-functions
@tanstack/start-storage-context
@tanstack/valibot-adapter
@tanstack/virtual-file-routes
@tanstack/zod-adapter
commit: |
… fork/paul-sachs/trailing-slash-splat-2
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/solid-router/tests/useNavigate.test.tsx (1)
2528-2643: Good end‑to‑end coverage; consider simplifying waitFor usage.Current pattern awaits
findByTextinside an asyncwaitForcallback. Prefer either:
- waitFor pathname, then a standalone
findByText, or- use synchronous
getByTextinsidewaitFor.Example:
await waitFor(() => expect(window.location.pathname).toBe(`/splat${tail}`)) expect(await screen.findByText('Splat Route')).toBeInTheDocument()packages/solid-router/tests/link.test.tsx (1)
5089-5181: Thorough Link-level validation for empty/undefined/missing_splat.Href generation and active state checks look correct across all trailingSlash modes; route render assertion completes the flow.
Minor: the inline comment “When _splat has a value...” could say “When navigating with empty/undefined/missing _splat...” for precision.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/solid-router/tests/link.test.tsx(2 hunks)packages/solid-router/tests/navigate.test.tsx(2 hunks)packages/solid-router/tests/useNavigate.test.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/solid-router/tests/link.test.tsxpackages/solid-router/tests/useNavigate.test.tsxpackages/solid-router/tests/navigate.test.tsx
packages/{react-router,solid-router}/**
📄 CodeRabbit inference engine (AGENTS.md)
Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/
Files:
packages/solid-router/tests/link.test.tsxpackages/solid-router/tests/useNavigate.test.tsxpackages/solid-router/tests/navigate.test.tsx
🧠 Learnings (1)
📚 Learning: 2025-10-08T08:11:47.088Z
Learnt from: nlynzaad
PR: TanStack/router#5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.
Applied to files:
packages/solid-router/tests/link.test.tsxpackages/solid-router/tests/useNavigate.test.tsxpackages/solid-router/tests/navigate.test.tsx
🧬 Code graph analysis (3)
packages/solid-router/tests/link.test.tsx (1)
packages/router-core/src/router.ts (1)
trailingSlashOptions(800-804)
packages/solid-router/tests/useNavigate.test.tsx (2)
packages/router-core/src/router.ts (1)
trailingSlashOptions(800-804)packages/solid-router/src/useNavigate.tsx (1)
useNavigate(11-25)
packages/solid-router/tests/navigate.test.tsx (1)
packages/solid-router/src/router.ts (1)
createRouter(76-78)
🪛 ESLint
packages/solid-router/tests/navigate.test.tsx
[error] 11-11: @solidjs/testing-library import should occur before import of ../src
(import/order)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Preview
- GitHub Check: Test
🔇 Additional comments (3)
packages/solid-router/tests/navigate.test.tsx (1)
573-628: Solid coverage for empty/undefined/missing_splatacross trailingSlash modes.Exercises navigate() + history reset and asserts path normalization. Matches the intended behavior.
packages/solid-router/tests/useNavigate.test.tsx (1)
13-13: LGTM import.Public
trailingSlashOptionsusage is appropriate here.packages/solid-router/tests/link.test.tsx (1)
13-13: LGTM import.Using the exported
trailingSlashOptionskeeps tests in sync with core.
3 participants
When expanding a splat,
undefinedshould be handled exactly like if the_splatparam didn't exist at all. This allows consistent handling of _splat routes to ensure that trailing slashes are kept or dropped according to the spec.Fixes #5385
Summary by CodeRabbit
Bug Fixes
New Features
Tests