test(solid): add params.spec.ts to start/router #5617
Conversation
WalkthroughAdds E2E Playwright tests for named and wildcard route parameters and a Unicode route, plus new route files and generated route-tree entries registering the Unicode path "/대한민국" in two e2e fixtures. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant T as Playwright Test
participant B as Browser
participant A as App/Router
rect rgb(230,247,255)
T->>B: navigate to /params-ps or /대한민국
end
B->>A: request route
A->>A: resolve route (named/splat params or Unicode path)
alt params tests
A->>B: render listing, links, and `params-output`
T->>B: click link or visit href
B->>A: route transition -> update URL and render destination
A->>B: update DOM with params JSON and heading
T->>B: assert pathname, heading visible, params JSON
else unicode test
A->>B: render Hello "/대한민국"!
T->>B: assert body text and percent-encoded URL
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
|
View your CI Pipeline Execution ↗ for commit 3459ec0
☁️ Nx Cloud last updated this comment at |
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: |
![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 (1)
e2e/solid-start/basic/tests/params.spec.ts (1)
5-7: Consider removing the redundant beforeEach navigation.Since the test immediately navigates to
/대한민국, the initial navigation to/in beforeEach adds no value and slightly increases test execution time.Apply this diff to streamline the test:
-test.beforeEach(async ({ page }) => { - await page.goto('/') -}) - test.use({
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
e2e/solid-router/basic/tests/params.spec.ts(1 hunks)e2e/solid-start/basic/tests/params.spec.ts(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:
e2e/solid-start/basic/tests/params.spec.tse2e/solid-router/basic/tests/params.spec.ts
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/solid-start/basic/tests/params.spec.tse2e/solid-router/basic/tests/params.spec.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: FatahChan
PR: TanStack/router#5475
File: e2e/react-start/basic-prerendering/src/routes/redirect/$target/via-beforeLoad.tsx:8-0
Timestamp: 2025-10-14T18:59:33.990Z
Learning: In TanStack Router e2e test files, when a route parameter is validated at the route level (e.g., using zod in validateSearch or param validation), switch statements on that parameter do not require a default case, as the validation ensures only expected values will reach the switch.
🧬 Code graph analysis (1)
e2e/solid-router/basic/tests/params.spec.ts (1)
e2e/solid-router/basic-file-based/tests/params.spec.ts (3)
test(134-195)page(59-132)page(139-194)
⏰ 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 (8)
e2e/solid-start/basic/tests/params.spec.ts (2)
1-3: LGTM!The imports are appropriate for e2e testing with Playwright and TanStack Router utilities.
14-22: LGTM!The test properly validates both Unicode route rendering and URL percent-encoding. The assertions correctly verify the page content displays the Unicode characters while the URL uses the percent-encoded form.
e2e/solid-router/basic/tests/params.spec.ts (6)
1-2: Verify the test import is correct.The solid-start test file imports
testfrom@tanstack/router-e2e-utils, while this file imports directly from@playwright/test. Ensure this difference is intentional, as the custom test utility may provide additional features like error whitelisting or baseURL fixtures that could be beneficial here.If the custom test utility should be used, apply this diff:
-import { expect, test } from '@playwright/test' +import { expect } from '@playwright/test' + +import { test } from '@tanstack/router-e2e-utils'
3-6: LGTM!The test suite setup is appropriate. The beforeEach navigation to
/params-psprovides a consistent starting point for all nested tests.
8-34: LGTM!Excellent use of TypeScript's
satisfiesoperator for type-safe test data. The parameterized test cases provide good coverage for named parameter scenarios including prefix and suffix variations.
36-76: LGTM!The named params tests are well-structured with proper parameterization. The tests comprehensively cover link interpolation, navigation, and initial load scenarios with appropriate assertions.
79-105: LGTM!The wildcard parameter test data follows the same well-structured pattern as the named params. The inclusion of both
'*'and'_splat'keys correctly models the expected wildcard parameter behavior.
107-148: LGTM!The wildcard parameter tests maintain excellent structural consistency with the named params tests, making the test suite easy to maintain and understand.
There was a problem hiding this comment.
Actionable comments posted: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
e2e/solid-router/basic/src/main.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:
e2e/solid-router/basic/src/main.tsx
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/solid-router/basic/src/main.tsx
🧬 Code graph analysis (1)
e2e/solid-router/basic/src/main.tsx (1)
packages/solid-router/src/index.tsx (3)
createRoute(263-263)Link(229-229)redirect(257-257)
⏰ 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). (1)
- GitHub Check: Test
🔇 Additional comments (8)
e2e/solid-router/basic/src/main.tsx (8)
11-12: Importing redirect is correct and necessary for beforeLoad redirects.
211-214: Good: params-ps parent route anchor.
Path '/params-ps' as the parent is fine. Children must be relative to nest correctly.
216-286: Params index looks good and matches the test intents.
Links and testids are clear; JSON output target is consistent.
293-299: Redirect-on-index is fine.
Throwing redirect to '/params-ps' is correct here.
348-354: Redirect-on-index for wildcard is correct.
356-368: Wildcard splat route is correct and relative.
Path '$' and params usage via useParams() look good.
370-382: Wildcard prefix variant is correct.
Relative path and rendering look good.
384-396: Wildcard suffix variant is correct.
Relative path and testid output are consistent.
| const paramsPsNamedRoute = createRoute({ | ||
| getParentRoute: () => paramsPsRoute, | ||
| path: '/named', | ||
| }) | ||
|
|
There was a problem hiding this comment.
Bug: Absolute child path breaks nesting under /params-ps.
Use a relative path so final URL is /params-ps/named, not /named.
Apply:
- path: '/named',
+ path: 'named',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const paramsPsNamedRoute = createRoute({ | |
| getParentRoute: () => paramsPsRoute, | |
| path: '/named', | |
| }) | |
| const paramsPsNamedRoute = createRoute({ | |
| getParentRoute: () => paramsPsRoute, | |
| path: 'named', | |
| }) |
🤖 Prompt for AI Agents
In e2e/solid-router/basic/src/main.tsx around lines 288 to 292, the child route
paramsPsNamedRoute incorrectly uses an absolute path '/named' which breaks
nesting under /params-ps; change the path to a relative one (remove the leading
slash, e.g. 'named') so the final URL resolves to /params-ps/named and ensure
getParentRoute remains paramsPsRoute.
| const paramsPsNamedFooRoute = createRoute({ | ||
| getParentRoute: () => paramsPsNamedRoute, | ||
| path: '/$foo', | ||
| component: function ParamsNamedFoo() { | ||
| const p = paramsPsNamedFooRoute.useParams() | ||
| return ( | ||
| <div> | ||
| <h3>ParamsNamedFoo</h3> | ||
| <div data-testid="params-output">{JSON.stringify(p())}</div> | ||
| </div> | ||
| ) | ||
| }, | ||
| }) |
There was a problem hiding this comment.
Bug: Absolute path makes this route root-level (/ $foo) and conflicts.
Should be relative to /params-ps/named.
- path: '/$foo',
+ path: '$foo',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const paramsPsNamedFooRoute = createRoute({ | |
| getParentRoute: () => paramsPsNamedRoute, | |
| path: '/$foo', | |
| component: function ParamsNamedFoo() { | |
| const p = paramsPsNamedFooRoute.useParams() | |
| return ( | |
| <div> | |
| <h3>ParamsNamedFoo</h3> | |
| <div data-testid="params-output">{JSON.stringify(p())}</div> | |
| </div> | |
| ) | |
| }, | |
| }) | |
| const paramsPsNamedFooRoute = createRoute({ | |
| getParentRoute: () => paramsPsNamedRoute, | |
| path: '$foo', | |
| component: function ParamsNamedFoo() { | |
| const p = paramsPsNamedFooRoute.useParams() | |
| return ( | |
| <div> | |
| <h3>ParamsNamedFoo</h3> | |
| <div data-testid="params-output">{JSON.stringify(p())}</div> | |
| </div> | |
| ) | |
| }, | |
| }) |
🤖 Prompt for AI Agents
In e2e/solid-router/basic/src/main.tsx around lines 301-313, the route path is
defined as '/$foo' which makes it absolute and conflicts at the app root; change
the path to be relative by removing the leading slash (use '$foo') so this child
route resolves under its parent (/params-ps/named). Keep getParentRoute
unchanged and run the tests to verify nested routing works.
| const paramsPsNamedFooPrefixRoute = createRoute({ | ||
| getParentRoute: () => paramsPsNamedRoute, | ||
| path: '/prefix{$foo}', | ||
| component: function ParamsNamedFooMarkdown() { | ||
| const p = paramsPsNamedFooPrefixRoute.useParams() | ||
| return ( | ||
| <div> | ||
| <h3>ParamsNamedFooPrefix</h3> | ||
| <div data-testid="params-output">{JSON.stringify(p())}</div> | ||
| </div> | ||
| ) | ||
| }, | ||
| }) |
There was a problem hiding this comment.
Two fixes: absolute path and component name typo.
- Make path relative so it nests under /params-ps/named.
- Rename component to match heading.
- path: '/prefix{$foo}',
- component: function ParamsNamedFooMarkdown() {
+ path: 'prefix{$foo}',
+ component: function ParamsNamedFooPrefix() {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const paramsPsNamedFooPrefixRoute = createRoute({ | |
| getParentRoute: () => paramsPsNamedRoute, | |
| path: '/prefix{$foo}', | |
| component: function ParamsNamedFooMarkdown() { | |
| const p = paramsPsNamedFooPrefixRoute.useParams() | |
| return ( | |
| <div> | |
| <h3>ParamsNamedFooPrefix</h3> | |
| <div data-testid="params-output">{JSON.stringify(p())}</div> | |
| </div> | |
| ) | |
| }, | |
| }) | |
| const paramsPsNamedFooPrefixRoute = createRoute({ | |
| getParentRoute: () => paramsPsNamedRoute, | |
| path: 'prefix{$foo}', | |
| component: function ParamsNamedFooPrefix() { | |
| const p = paramsPsNamedFooPrefixRoute.useParams() | |
| return ( | |
| <div> | |
| <h3>ParamsNamedFooPrefix</h3> | |
| <div data-testid="params-output">{JSON.stringify(p())}</div> | |
| </div> | |
| ) | |
| }, | |
| }) |
🤖 Prompt for AI Agents
In e2e/solid-router/basic/src/main.tsx around lines 315 to 327, the route is
defined with an absolute path and the component function name doesn't match the
heading; change the path from '/prefix{$foo}' to a relative 'prefix{$foo}' so it
nests under /params-ps/named, and rename the component function from
ParamsNamedFooMarkdown to ParamsNamedFooPrefix (and update any references) so
the function name matches the rendered heading.
| const paramsPsNamedFooSuffixRoute = createRoute({ | ||
| getParentRoute: () => paramsPsNamedRoute, | ||
| path: '/{$foo}suffix', | ||
| component: function ParamsNamedFooSuffix() { | ||
| const p = paramsPsNamedFooSuffixRoute.useParams() | ||
| return ( | ||
| <div> | ||
| <h3>ParamsNamedFooSuffix</h3> | ||
| <div data-testid="params-output">{JSON.stringify(p())}</div> | ||
| </div> | ||
| ) | ||
| }, | ||
| }) |
There was a problem hiding this comment.
Bug: Absolute path; should be relative to /params-ps/named.
- path: '/{$foo}suffix',
+ path: '{$foo}suffix',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const paramsPsNamedFooSuffixRoute = createRoute({ | |
| getParentRoute: () => paramsPsNamedRoute, | |
| path: '/{$foo}suffix', | |
| component: function ParamsNamedFooSuffix() { | |
| const p = paramsPsNamedFooSuffixRoute.useParams() | |
| return ( | |
| <div> | |
| <h3>ParamsNamedFooSuffix</h3> | |
| <div data-testid="params-output">{JSON.stringify(p())}</div> | |
| </div> | |
| ) | |
| }, | |
| }) | |
| const paramsPsNamedFooSuffixRoute = createRoute({ | |
| getParentRoute: () => paramsPsNamedRoute, | |
| path: '{$foo}suffix', | |
| component: function ParamsNamedFooSuffix() { | |
| const p = paramsPsNamedFooSuffixRoute.useParams() | |
| return ( | |
| <div> | |
| <h3>ParamsNamedFooSuffix</h3> | |
| <div data-testid="params-output">{JSON.stringify(p())}</div> | |
| </div> | |
| ) | |
| }, | |
| }) |
🤖 Prompt for AI Agents
In e2e/solid-router/basic/src/main.tsx around lines 329 to 341, the route path
is defined as an absolute path '/{$foo}suffix' but it should be relative to its
parent (/params-ps/named); change the path to a relative value (remove the
leading slash) so it becomes '{$foo}suffix' (or the router's relative-path form)
so the route resolves under paramsPsNamedRoute; leave getParentRoute as-is.
| const paramsPsWildcardRoute = createRoute({ | ||
| getParentRoute: () => paramsPsRoute, | ||
| path: '/wildcard', | ||
| }) |
There was a problem hiding this comment.
Bug: Absolute child path prevents /params-ps/wildcard nesting.
- path: '/wildcard',
+ path: 'wildcard',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const paramsPsWildcardRoute = createRoute({ | |
| getParentRoute: () => paramsPsRoute, | |
| path: '/wildcard', | |
| }) | |
| const paramsPsWildcardRoute = createRoute({ | |
| getParentRoute: () => paramsPsRoute, | |
| path: 'wildcard', | |
| }) |
🤖 Prompt for AI Agents
In e2e/solid-router/basic/src/main.tsx around lines 343 to 346, the child route
is defined with an absolute path '/wildcard' which prevents it from nesting
under the parent paramsPsRoute; change the path to a relative segment (remove
the leading slash, e.g. 'wildcard') so the router composes it as
/params-ps/wildcard and remains nested under the parent route.
| paramsPsRoute.addChildren([ | ||
| paramsPsNamedRoute.addChildren([ | ||
| paramsPsNamedFooPrefixRoute, | ||
| paramsPsNamedFooSuffixRoute, | ||
| paramsPsNamedFooRoute, | ||
| paramsPsNamedIndexRoute, | ||
| ]), | ||
| paramsPsWildcardRoute.addChildren([ | ||
| paramsPsWildcardSplatRoute, | ||
| paramsPsWildcardSplatPrefixRoute, | ||
| paramsPsWildcardSplatSuffixRoute, | ||
| paramsPsWildcardIndexRoute, | ||
| ]), | ||
| paramsPsIndexRoute, | ||
| ]), |
There was a problem hiding this comment.
🧩 Analysis chain
Route tree wiring looks right; re-verify after path fixes.
Once child paths are made relative, this tree will reflect URLs under /params-ps as intended.
Run to confirm no stray leading slashes remain in params-ps children:
🏁 Script executed:
#!/usr/bin/env bash
# Check for accidental absolute child paths under params-ps subtree
rg -nP 'const\s+paramsPs(Named|Wildcard).*?path:\s*\"\/' -C2
rg -nP 'const\s+paramsPs(Named|Wildcard).*\n.*path:\s*\"\/' -C2Length of output: 0
---
🏁 Script executed:
```shell
#!/bin/bash
# Examine the file and find route definitions for paramsPsNamed and paramsPsWildcard
rg -n 'const\s+(paramsPsNamed|paramsPsWildcard)' e2e/solid-router/basic/src/main.tsx -A 5
Length of output: 2200
🏁 Script executed:
#!/bin/bash
# Also search for the full route tree structure with path definitions
rg -n 'path:' e2e/solid-router/basic/src/main.tsx | grep -i 'params.*ps'Length of output: 25
🏁 Script executed:
#!/bin/bash
# Get a broader view of the params-ps route section
rg -n 'const\s+paramsPsRoute' e2e/solid-router/basic/src/main.tsx -A 50 | head -100Length of output: 1632
Remove leading slashes from paramsPsNamedRoute and paramsPsWildcardRoute child paths.
The route tree structure still contains absolute child paths that must be made relative:
paramsPsNamedRoute: change/named→namedparamsPsNamedFooRoute: change/$foo→$fooparamsPsNamedFooPrefixRoute: change/prefix{$foo}→prefix{$foo}paramsPsNamedFooSuffixRoute: change/{$foo}suffix→{$foo}suffixparamsPsNamedIndexRoute: change/→ `` (omit or use appropriate index marker per router spec)paramsPsWildcardRoute: change/wildcard→wildcardparamsPsWildcardIndexRoute: change/→ `` (omit or use appropriate index marker per router spec)
The wildcard splat routes (paramsPsWildcardSplatRoute, etc.) are already correct with relative paths.
🤖 Prompt for AI Agents
In e2e/solid-router/basic/src/main.tsx around lines 403 to 417, several child
route paths are incorrectly specified as absolute (leading slash) and must be
made relative: remove the leading '/' from paramsPsNamedRoute (`/named` →
`named`), paramsPsNamedFooRoute (`/$foo` → `$foo`), paramsPsNamedFooPrefixRoute
(`/prefix{$foo}` → `prefix{$foo}`), paramsPsNamedFooSuffixRoute (`/{$foo}suffix`
→ `{$foo}suffix`), and change paramsPsNamedIndexRoute from '/' to the router's
index representation (empty path or index marker); likewise change
paramsPsWildcardRoute (`/wildcard` → `wildcard`) and paramsPsWildcardIndexRoute
from '/' to the index representation; leave the wildcard splat child routes
as-is. Ensure all child routes use relative paths per the router spec so the
tree composes correctly.
Summary by CodeRabbit