test: restructure transition e2e tests #5800
Merged
Conversation
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
Contributor
WalkthroughRestructures transition routes across multiple e2e test applications. Replaces the flat Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 98e28a4
☁️ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
e2e/solid-router/basic-file-based/tests/transition.spec.ts (1)
23-66: Await everyclick()so Playwright actually performs the actions in order.Calling
page.getByTestId(...).click()withoutawaitfires an async action and then the test barrels ahead. That makes the assertions race with an unfinished click, and any rejection (element not ready, navigation timeout, etc.) turns into an unhandled promise rather than a surfaced failure. Please restore theawaiton all of these clicks to keep the test deterministic.- page.getByTestId('increase-button').click() + await page.getByTestId('increase-button').click()(Repeat for the other clicks in this file.)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
e2e/react-router/basic-react-query-file-based/src/routeTree.gen.ts(8 hunks)e2e/react-router/basic-react-query-file-based/src/routes/transition/count/query.tsx(2 hunks)e2e/react-router/basic-react-query-file-based/tests/transition.spec.ts(1 hunks)e2e/solid-router/basic-file-based/src/routeTree.gen.ts(11 hunks)e2e/solid-router/basic-file-based/src/routes/transition/count/create-resource.tsx(2 hunks)e2e/solid-router/basic-file-based/src/routes/transition/typing/create-resource.tsx(1 hunks)e2e/solid-router/basic-file-based/tests/transition.spec.ts(4 hunks)e2e/solid-router/basic-solid-query-file-based/src/routeTree.gen.ts(8 hunks)e2e/solid-router/basic-solid-query-file-based/src/routes/transition/count/query.tsx(3 hunks)e2e/solid-router/basic-solid-query-file-based/tests/transition.spec.ts(1 hunks)e2e/solid-start/basic/src/routeTree.gen.ts(11 hunks)e2e/solid-start/basic/src/routes/transition/count/create-resource.tsx(1 hunks)e2e/solid-start/basic/src/routes/transition/typing/create-resource.tsx(1 hunks)e2e/solid-start/basic/vite.config.ts(1 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:
e2e/solid-start/basic/src/routes/transition/count/create-resource.tsxe2e/solid-router/basic-file-based/src/routes/transition/typing/create-resource.tsxe2e/react-router/basic-react-query-file-based/src/routes/transition/count/query.tsxe2e/solid-router/basic-solid-query-file-based/tests/transition.spec.tse2e/solid-router/basic-file-based/tests/transition.spec.tse2e/solid-start/basic/src/routes/transition/typing/create-resource.tsxe2e/solid-router/basic-solid-query-file-based/src/routes/transition/count/query.tsxe2e/solid-start/basic/src/routeTree.gen.tse2e/solid-router/basic-file-based/src/routes/transition/count/create-resource.tsxe2e/solid-start/basic/vite.config.tse2e/react-router/basic-react-query-file-based/src/routeTree.gen.tse2e/solid-router/basic-solid-query-file-based/src/routeTree.gen.tse2e/react-router/basic-react-query-file-based/tests/transition.spec.tse2e/solid-router/basic-file-based/src/routeTree.gen.ts
**/src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Place file-based routes under src/routes/ directories
Files:
e2e/solid-start/basic/src/routes/transition/count/create-resource.tsxe2e/solid-router/basic-file-based/src/routes/transition/typing/create-resource.tsxe2e/react-router/basic-react-query-file-based/src/routes/transition/count/query.tsxe2e/solid-start/basic/src/routes/transition/typing/create-resource.tsxe2e/solid-router/basic-solid-query-file-based/src/routes/transition/count/query.tsxe2e/solid-router/basic-file-based/src/routes/transition/count/create-resource.tsx
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/solid-start/basic/src/routes/transition/count/create-resource.tsxe2e/solid-router/basic-file-based/src/routes/transition/typing/create-resource.tsxe2e/react-router/basic-react-query-file-based/src/routes/transition/count/query.tsxe2e/solid-router/basic-solid-query-file-based/tests/transition.spec.tse2e/solid-router/basic-file-based/tests/transition.spec.tse2e/solid-start/basic/src/routes/transition/typing/create-resource.tsxe2e/solid-router/basic-solid-query-file-based/src/routes/transition/count/query.tsxe2e/solid-start/basic/src/routeTree.gen.tse2e/solid-router/basic-file-based/src/routes/transition/count/create-resource.tsxe2e/solid-start/basic/vite.config.tse2e/react-router/basic-react-query-file-based/src/routeTree.gen.tse2e/solid-router/basic-solid-query-file-based/src/routeTree.gen.tse2e/react-router/basic-react-query-file-based/tests/transition.spec.tse2e/solid-router/basic-file-based/src/routeTree.gen.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: nlynzaad
Repo: TanStack/router PR: 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.
Learnt from: FatahChan
Repo: TanStack/router PR: 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.
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to e2e/** : Store end-to-end tests under the e2e/ directory
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/{router-cli,router-generator,router-plugin,virtual-file-routes}/** : Keep CLI, generators, bundler plugins, and virtual file routing utilities in their dedicated tooling package directories
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/{react-router,solid-router}/** : Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/
Applied to files:
e2e/solid-start/basic/src/routes/transition/count/create-resource.tsxe2e/solid-router/basic-file-based/src/routes/transition/typing/create-resource.tsxe2e/solid-start/basic/src/routes/transition/typing/create-resource.tsxe2e/solid-router/basic-solid-query-file-based/src/routes/transition/count/query.tsxe2e/solid-start/basic/src/routeTree.gen.tse2e/solid-start/basic/vite.config.tse2e/react-router/basic-react-query-file-based/src/routeTree.gen.tse2e/solid-router/basic-solid-query-file-based/src/routeTree.gen.tse2e/solid-router/basic-file-based/src/routeTree.gen.ts
📚 Learning: 2025-10-08T08:11:47.088Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 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:
e2e/solid-router/basic-file-based/src/routes/transition/typing/create-resource.tsxe2e/solid-router/basic-solid-query-file-based/tests/transition.spec.tse2e/solid-router/basic-file-based/tests/transition.spec.tse2e/solid-router/basic-solid-query-file-based/src/routes/transition/count/query.tsxe2e/solid-start/basic/src/routeTree.gen.tse2e/solid-router/basic-file-based/src/routes/transition/count/create-resource.tsxe2e/solid-start/basic/vite.config.tse2e/react-router/basic-react-query-file-based/src/routeTree.gen.tse2e/solid-router/basic-solid-query-file-based/src/routeTree.gen.tse2e/react-router/basic-react-query-file-based/tests/transition.spec.tse2e/solid-router/basic-file-based/src/routeTree.gen.ts
📚 Learning: 2025-10-01T18:31:35.420Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: e2e/react-start/custom-basepath/src/routeTree.gen.ts:58-61
Timestamp: 2025-10-01T18:31:35.420Z
Learning: Do not review files named `routeTree.gen.ts` in TanStack Router repositories, as these are autogenerated files that should not be manually modified.
Applied to files:
e2e/solid-start/basic/src/routeTree.gen.tse2e/solid-start/basic/vite.config.tse2e/solid-router/basic-solid-query-file-based/src/routeTree.gen.tse2e/solid-router/basic-file-based/src/routeTree.gen.ts
📚 Learning: 2025-10-01T18:30:26.591Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: packages/router-core/src/router.ts:2231-2245
Timestamp: 2025-10-01T18:30:26.591Z
Learning: In `packages/router-core/src/router.ts`, the `resolveRedirect` method intentionally strips the router's origin from redirect URLs when they match (e.g., `https://foo.com/bar` → `/bar` for same-origin redirects) while preserving the full URL for cross-origin redirects. This logic should not be removed or simplified to use `location.publicHref` directly.
Applied to files:
e2e/solid-router/basic-file-based/src/routes/transition/count/create-resource.tsxe2e/solid-start/basic/vite.config.ts
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to **/src/routes/** : Place file-based routes under src/routes/ directories
Applied to files:
e2e/react-router/basic-react-query-file-based/src/routeTree.gen.tse2e/solid-router/basic-solid-query-file-based/src/routeTree.gen.ts
📚 Learning: 2025-09-28T21:41:45.233Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 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:
e2e/solid-router/basic-file-based/src/routeTree.gen.ts
🧬 Code graph analysis (7)
e2e/solid-start/basic/src/routes/transition/count/create-resource.tsx (1)
e2e/solid-router/basic-file-based/src/routes/transition/count/create-resource.tsx (1)
Route(5-10)
e2e/solid-router/basic-file-based/src/routes/transition/typing/create-resource.tsx (2)
e2e/solid-router/basic-file-based/src/routes/transition/count/create-resource.tsx (1)
Route(5-10)e2e/solid-start/basic/src/routes/transition/count/create-resource.tsx (1)
Route(5-10)
e2e/solid-start/basic/src/routes/transition/typing/create-resource.tsx (2)
e2e/solid-router/basic-file-based/src/routes/transition/typing/create-resource.tsx (1)
Route(4-10)e2e/solid-start/basic/src/routes/transition/count/create-resource.tsx (1)
Route(5-10)
e2e/solid-router/basic-solid-query-file-based/src/routes/transition/count/query.tsx (1)
e2e/react-router/basic-react-query-file-based/src/routes/transition/count/query.tsx (1)
Route(20-27)
e2e/solid-router/basic-file-based/src/routes/transition/count/create-resource.tsx (1)
e2e/solid-start/basic/src/routes/transition/count/create-resource.tsx (1)
Route(5-10)
e2e/react-router/basic-react-query-file-based/src/routeTree.gen.ts (1)
e2e/solid-router/basic-solid-query-file-based/src/routeTree.gen.ts (3)
FileRoutesByTo(75-82)FileRoutesById(83-94)RootRouteChildren(126-131)
e2e/solid-router/basic-solid-query-file-based/src/routeTree.gen.ts (1)
e2e/react-router/basic-react-query-file-based/src/routeTree.gen.ts (3)
FileRoutesByTo(75-82)FileRoutesById(83-94)RootRouteChildren(126-131)
⏰ 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 (9)
e2e/solid-start/basic/src/routes/transition/count/create-resource.tsx (3)
1-3: LGTM!The imports are appropriate for a Solid Router route with async resource handling and search param validation.
5-10: LGTM!The route definition correctly validates the search parameter and provides a sensible default. The nested path structure aligns with the PR's restructuring objectives.
12-48: LGTM!The component implementation correctly follows Solid.js reactivity patterns:
Route.useSearch()returns a signal accessor properly tracked increateResource- The resource dependency
() => searchQuery().nensures refetching when the search param changes- Suspense boundary appropriately handles the async loading state
- Test IDs enable e2e test automation
The intentional 1-second delay effectively demonstrates transition behavior for testing purposes.
e2e/solid-router/basic-solid-query-file-based/src/routes/transition/count/query.tsx (1)
20-27: LGTM! Route path restructuring is consistent.The route path and Link navigation have been correctly updated from
/transition/to/transition/count/query, maintaining consistency between the route definition and component navigation.Also applies to: 40-44
e2e/solid-start/basic/vite.config.ts (1)
25-25: LGTM! Prerender exclusion appropriately updated.Adding
/transitionto the exclusion filter is appropriate given the new nested transition routes that likely require dynamic rendering.e2e/react-router/basic-react-query-file-based/tests/transition.spec.ts (1)
6-6: LGTM! Test correctly updated for new route structure.The test navigation path has been properly updated to match the restructured route at
/transition/count/query.e2e/solid-router/basic-solid-query-file-based/tests/transition.spec.ts (1)
6-6: LGTM! Test correctly updated for new route structure.The test navigation path has been properly updated to match the restructured route at
/transition/count/query.e2e/react-router/basic-react-query-file-based/src/routes/transition/count/query.tsx (1)
20-27: LGTM! Route path restructuring is consistent.The route path and Link navigation have been correctly updated from
/transition/to/transition/count/query, maintaining consistency between the route definition and component navigation.Also applies to: 40-44
e2e/solid-router/basic-file-based/src/routes/transition/count/create-resource.tsx (1)
5-10: LGTM! Route path restructuring is consistent.The route path and Link navigation have been correctly updated from
/transition/to/transition/count/create-resource, maintaining consistency between the route definition and component navigation.Also applies to: 18-22
Comment on lines
+27
to
+34
| <input | ||
| class="border bg-gray-50 text-black px-3 py-1" | ||
| value={''} | ||
| placeholder="Type to search..." | ||
| onInput={(e) => { | ||
| navigate({ search: { query: e.currentTarget.value } }) | ||
| }} | ||
| /> |
Contributor
There was a problem hiding this comment.
Fix the input value binding.
Same issue as in the solid-start variant: the input has value={''} which prevents the typed text from being visible in the input field.
Apply this diff:
<input
class="border bg-gray-50 text-black px-3 py-1"
- value={''}
+ value={searchQuery().query}
placeholder="Type to search..."
onInput={(e) => {
navigate({ search: { query: e.currentTarget.value } })
}}
/>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
e2e/solid-router/basic-file-based/src/routes/transition/typing/create-resource.tsx
around lines 27 to 34, the input is hard-coded with value={''} which prevents
typed text from appearing; replace the static empty value with the current query
state so the input is controlled correctly (for example bind value to the
search/query signal coming from useSearchParams or your component state, e.g.
value={search()?.query ?? ''} or value={query() ?? ''}), and ensure the
signal/function used is imported/defined so the onInput navigate call continues
to update the URL.
Comment on lines
+27
to
+34
| <input | ||
| class="border bg-gray-50 text-black px-3 py-1" | ||
| value={''} | ||
| placeholder="Type to search..." | ||
| onInput={(e) => { | ||
| navigate({ search: { query: e.currentTarget.value } }) | ||
| }} | ||
| /> |
Contributor
There was a problem hiding this comment.
Fix the input value binding.
The input has value={''} which means it will always display as empty, even as the user types. This creates a broken UX where the typed text is invisible in the input field, though it's reflected in the search state below.
Apply this diff to fix the controlled input:
<input
class="border bg-gray-50 text-black px-3 py-1"
- value={''}
+ value={searchQuery().query}
placeholder="Type to search..."
onInput={(e) => {
navigate({ search: { query: e.currentTarget.value } })
}}
/>Alternatively, remove the value prop entirely for an uncontrolled input (though controlled is preferred for this use case).
📝 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.
Suggested change
| <input | |
| class="border bg-gray-50 text-black px-3 py-1" | |
| value={''} | |
| placeholder="Type to search..." | |
| onInput={(e) => { | |
| navigate({ search: { query: e.currentTarget.value } }) | |
| }} | |
| /> | |
| <input | |
| class="border bg-gray-50 text-black px-3 py-1" | |
| value={searchQuery().query} | |
| placeholder="Type to search..." | |
| onInput={(e) => { | |
| navigate({ search: { query: e.currentTarget.value } }) | |
| }} | |
| /> |
🤖 Prompt for AI Agents
In e2e/solid-start/basic/src/routes/transition/typing/create-resource.tsx around
lines 27 to 34, the input currently has a hard-coded value of '' which prevents
user input from appearing; change the input to be a controlled input by binding
its value to the current search query state (the same state you update in
onInput) and ensure onInput updates that state (or derive the value from the
router search params you navigate to). If you prefer uncontrolled, remove the
value prop entirely, but preferred fix is to replace value={''} with the query
state variable so the input shows typed text.
This was referenced Nov 9, 2025
roduyemi
pushed a commit
to roduyemi/oss-router
that referenced
this pull request
Nov 19, 2025
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.
Summary by CodeRabbit
New Features
Refactor
Tests