tests: e2e test for active links with special char encoding #5681
Conversation
WalkthroughThis PR adds a new feature for encoding-aware link active states with parameter validation. It introduces two route files under Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
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)
Comment |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx affected --targets=test:eslint,test:unit,tes... |
❌ Failed | 4m 40s | View ↗ |
nx run-many --target=build --exclude=examples/*... |
✅ Succeeded | 3s | View ↗ |
☁️ Nx Cloud last updated this comment at 2025-10-29 01:28:10 UTC
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: 2
🧹 Nitpick comments (2)
e2e/react-start/basic/tests/encoding.spec.ts (1)
42-44: UsetoHaveClassmatcher for better test reliability.The
toHaveAttribute('class', 'font-bold')assertion expects the class attribute to be exactly'font-bold', which will fail if additional classes are present. ThetoHaveClassmatcher is more robust and checks for the presence of a specific class.Apply this diff:
const link = page.getByTestId('self-link') await expect(link).toBeInViewport() - await expect(link).toHaveAttribute('class', 'font-bold') + await expect(link).toHaveClass('font-bold') await expect(link).toHaveAttribute('data-status', 'active')e2e/react-start/basic/src/routes/encoding/link-active/$target.tsx (1)
10-24: Refactor to extract hook calls from JSX.
Route.useParams()andRoute.useSearch()are called multiple times directly in the JSX (lines 17, 18, 20, 21). This is inefficient as hooks are called on every render, and it makes the code less readable. Extract these calls to the top of the component.Apply this diff:
function RouteComponent() { + const { target } = Route.useParams() + const search = Route.useSearch() + return ( <Link data-testid="self-link" activeProps={{ className: 'font-bold' }} activeOptions={{ includeSearch: true }} to="/encoding/link-active/$target" - params={{ target: Route.useParams().target }} - search={Route.useSearch()} + params={{ target }} + search={search} > - link to self with $target={Route.useParams().target}, search.foo= - {Route.useSearch().foo} + link to self with $target={target}, search.foo={search.foo} </Link> ) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
e2e/react-start/basic/src/routes/encoding/link-active/$target.tsx(1 hunks)e2e/react-start/basic/src/routes/encoding/link-active/index.tsx(1 hunks)e2e/react-start/basic/tests/encoding.spec.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/react-start/basic/src/routes/encoding/link-active/$target.tsxe2e/react-start/basic/src/routes/encoding/link-active/index.tsxe2e/react-start/basic/tests/encoding.spec.ts
**/src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Place file-based routes under src/routes/ directories
Files:
e2e/react-start/basic/src/routes/encoding/link-active/$target.tsxe2e/react-start/basic/src/routes/encoding/link-active/index.tsx
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/react-start/basic/src/routes/encoding/link-active/$target.tsxe2e/react-start/basic/src/routes/encoding/link-active/index.tsxe2e/react-start/basic/tests/encoding.spec.ts
🧬 Code graph analysis (2)
e2e/react-start/basic/src/routes/encoding/link-active/$target.tsx (1)
e2e/react-start/basic/src/routes/encoding/link-active/index.tsx (1)
Route(3-5)
e2e/react-start/basic/src/routes/encoding/link-active/index.tsx (1)
e2e/react-start/basic/src/routes/encoding/link-active/$target.tsx (1)
Route(4-8)
🪛 ESLint
e2e/react-start/basic/src/routes/encoding/link-active/$target.tsx
[error] 1-1: Member 'Link' of the import declaration should be sorted alphabetically.
(sort-imports)
⏰ 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: Test
- GitHub Check: Preview
🔇 Additional comments (1)
e2e/react-start/basic/src/routes/encoding/link-active/index.tsx (1)
1-19: LGTM!The route definition and links are correctly implemented, properly exercising special character encoding in route parameters.
| import { createFileRoute, Link } from '@tanstack/react-router' | ||
| import z from 'zod' |
There was a problem hiding this comment.
🧩 Analysis chain
Fix import sorting and verify zod import.
The static analysis tool flagged that Link should be sorted before createFileRoute alphabetically. Additionally, the zod import uses a default import pattern, but zod typically exports z as a named export.
Apply this diff to fix the import sorting:
-import { createFileRoute, Link } from '@tanstack/react-router'
+import { Link, createFileRoute } from '@tanstack/react-router'
import z from 'zod'Verify the zod import pattern with a web search:
As per coding guidelines.
🌐 Web query:
Does zod export z as a default export or named export?
💡 Result:
Zod does not use a default export. Import it as a namespace (recommended):
import * as z from "zod". [1][2]
Sources: Zod docs and repository. [1] https://zod.dev/intro (recommendation) [2] https://github.com/colinhacks/zod (README).
Fix import sorting and correct zod import pattern.
The zod import is incorrect. Zod does not use a default export and should be imported as a namespace: import * as z from "zod". Additionally, apply the import sorting fix to align with static analysis conventions.
-import { createFileRoute, Link } from '@tanstack/react-router'
-import z from 'zod'
+import { Link, createFileRoute } from '@tanstack/react-router'
+import * as z from 'zod'📝 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.
| import { createFileRoute, Link } from '@tanstack/react-router' | |
| import z from 'zod' | |
| import { Link, createFileRoute } from '@tanstack/react-router' | |
| import * as z from 'zod' |
🧰 Tools
🪛 ESLint
[error] 1-1: Member 'Link' of the import declaration should be sorted alphabetically.
(sort-imports)
🤖 Prompt for AI Agents
In e2e/react-start/basic/src/routes/encoding/link-active/$target.tsx lines 1-2,
the zod import is incorrect and imports should be sorted: change the zod import
to a namespace import (import * as z from "zod") and reorder the import
statements according to your project's import-sorting rules (typically
alphabetically by module specifier or as enforced by your linter) so imports
follow the static analysis conventions.
| combinate | ||
| test.describe('link active', () => { |
There was a problem hiding this comment.
Remove the stray combinate identifier.
Line 19 contains a standalone combinate identifier that has no effect. This appears to be an accidental leftover from development.
Apply this diff to remove it:
-combinate
test.describe('link active', () => {📝 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.
| combinate | |
| test.describe('link active', () => { | |
| test.describe('link active', () => { |
🤖 Prompt for AI Agents
In e2e/react-start/basic/tests/encoding.spec.ts around lines 19 to 20, there is
a stray standalone identifier "combinate" on line 19 that should be removed;
delete that token so the file begins directly with the test.describe('link
active', () => { block, ensuring no other code or whitespace is accidentally
removed and the test file still compiles.
Summary by CodeRabbit
New Features
Tests