test(solid-router): js-only-file-based e2e suite #5626
Conversation
WalkthroughAdds a new e2e Solid.js file-based routing project at Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant Vite as Vite Dev Server
participant Router as TanStack Router
participant API as Posts API
participant Dummy as Dummy E2E Server
User->>Browser: Open http://localhost:PORT
Browser->>Vite: GET index.html & /src/main.jsx
Vite->>Browser: Serve app
Browser->>Router: Initialize RouterProvider with routeTree
User->>Browser: Clicks "Posts"
Browser->>Router: Navigate to /posts
Router->>API: fetchPosts()
alt test mode
API->>Dummy: GET /posts (localhost:EXTERNAL_PORT)
Dummy->>API: posts[]
else prod
API->>API: GET https://jsonplaceholder.typicode.com/posts
end
API->>Browser: Render posts list
User->>Browser: Click post link
Browser->>Router: Navigate to /posts/$postId
Router->>API: fetchPost(postId)
API->>Dummy: GET /posts/{id}
Dummy->>API: post or 404
alt 404
Browser->>Router: Render notFoundComponent
else success
Browser->>Router: Render PostComponent
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 86c4d72
☁️ 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: 3
🧹 Nitpick comments (5)
e2e/solid-router/js-only-file-based/src/routes/__root.jsx (1)
46-47: The@ts-expect-errorcomment has no effect in a JSX file.Since this is a
.jsxfile, TypeScript checking isn't applied, making the@ts-expect-errordirective inert. Consider either:
- Removing the comment if TypeScript checking isn't needed
- Converting to
.tsxif TypeScript validation is desired for route type safetye2e/solid-router/js-only-file-based/src/routes/posts.route.jsx (1)
36-36: Consider styling the divider or using a flex divider.The
<hr />element lacks styling and may not render as intended between flex items. Consider using Tailwind utilities for proper spacing.Apply this diff to use a styled divider:
- <hr /> + <div class="border-r border-gray-300" />Or add proper styling to the hr:
- <hr /> + <hr class="border-t border-gray-300 my-2" />e2e/solid-router/js-only-file-based/src/routes/posts.$postId.jsx (1)
15-17: Consider using ErrorComponent directly in the route config.The
PostErrorComponentwrapper adds no additional logic and simply delegates toErrorComponent.Apply this diff to simplify:
+import { ErrorComponent } from '@tanstack/solid-router' + export const Route = createFileRoute('/posts/$postId')({ loader: async ({ params: { postId } }) => fetchPost(postId), - errorComponent: PostErrorComponent, + errorComponent: ErrorComponent, notFoundComponent: () => { return <p>Post not found</p> }, component: PostComponent, }) - -export function PostErrorComponent({ error }) { - return <ErrorComponent error={error} /> -}e2e/solid-router/js-only-file-based/src/posts.js (2)
3-3: Set the error name property for better debugging.Error subclasses should explicitly set their
nameproperty to improve stack traces and error identification.Apply this diff:
-export class NotFoundError extends Error {} +export class NotFoundError extends Error { + constructor(message) { + super(message) + this.name = 'NotFoundError' + } +}
11-14: Consider adding error handling for consistency.Unlike
fetchPost, this function doesn't include explicit error handling. While errors will propagate naturally, adding consistent error handling improves maintainability.Consider adding try-catch or error transformation:
export const fetchPosts = async () => { console.info('Fetching posts...') try { return await axios.get(`${queryURL}/posts`).then((r) => r.data.slice(0, 10)) } catch (error) { console.error('Failed to fetch posts:', error) throw error } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (24)
e2e/solid-router/basepath-file-based/package.json(1 hunks)e2e/solid-router/js-only-file-based/.gitignore(1 hunks)e2e/solid-router/js-only-file-based/index.html(1 hunks)e2e/solid-router/js-only-file-based/package.json(1 hunks)e2e/solid-router/js-only-file-based/playwright.config.ts(1 hunks)e2e/solid-router/js-only-file-based/postcss.config.mjs(1 hunks)e2e/solid-router/js-only-file-based/src/main.jsx(1 hunks)e2e/solid-router/js-only-file-based/src/posts.js(1 hunks)e2e/solid-router/js-only-file-based/src/routeTree.gen.js(1 hunks)e2e/solid-router/js-only-file-based/src/routes/__root.jsx(1 hunks)e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout.jsx(1 hunks)e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout.jsx(1 hunks)e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout/route-a.jsx(1 hunks)e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout/route-b.jsx(1 hunks)e2e/solid-router/js-only-file-based/src/routes/index.jsx(1 hunks)e2e/solid-router/js-only-file-based/src/routes/posts.$postId.jsx(1 hunks)e2e/solid-router/js-only-file-based/src/routes/posts.index.jsx(1 hunks)e2e/solid-router/js-only-file-based/src/routes/posts.route.jsx(1 hunks)e2e/solid-router/js-only-file-based/src/styles.css(1 hunks)e2e/solid-router/js-only-file-based/tests/app.spec.ts(1 hunks)e2e/solid-router/js-only-file-based/tests/setup/global.setup.ts(1 hunks)e2e/solid-router/js-only-file-based/tests/setup/global.teardown.ts(1 hunks)e2e/solid-router/js-only-file-based/tsconfig.json(1 hunks)e2e/solid-router/js-only-file-based/vite.config.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
e2e/solid-router/js-only-file-based/tests/setup/global.teardown.tse2e/solid-router/js-only-file-based/tests/setup/global.setup.tse2e/solid-router/js-only-file-based/tests/app.spec.tse2e/solid-router/js-only-file-based/playwright.config.ts
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/solid-router/js-only-file-based/tests/setup/global.teardown.tse2e/solid-router/js-only-file-based/src/routes/index.jsxe2e/solid-router/basepath-file-based/package.jsone2e/solid-router/js-only-file-based/vite.config.jse2e/solid-router/js-only-file-based/tests/setup/global.setup.tse2e/solid-router/js-only-file-based/src/styles.csse2e/solid-router/js-only-file-based/tests/app.spec.tse2e/solid-router/js-only-file-based/tsconfig.jsone2e/solid-router/js-only-file-based/src/routes/posts.index.jsxe2e/solid-router/js-only-file-based/postcss.config.mjse2e/solid-router/js-only-file-based/src/posts.jse2e/solid-router/js-only-file-based/playwright.config.tse2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout/route-b.jsxe2e/solid-router/js-only-file-based/package.jsone2e/solid-router/js-only-file-based/src/routes/posts.route.jsxe2e/solid-router/js-only-file-based/index.htmle2e/solid-router/js-only-file-based/src/routeTree.gen.jse2e/solid-router/js-only-file-based/src/routes/_pathlessLayout.jsxe2e/solid-router/js-only-file-based/src/main.jsxe2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout.jsxe2e/solid-router/js-only-file-based/src/routes/__root.jsxe2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout/route-a.jsxe2e/solid-router/js-only-file-based/src/routes/posts.$postId.jsx
**/src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Place file-based routes under src/routes/ directories
Files:
e2e/solid-router/js-only-file-based/src/routes/index.jsxe2e/solid-router/js-only-file-based/src/routes/posts.index.jsxe2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout/route-b.jsxe2e/solid-router/js-only-file-based/src/routes/posts.route.jsxe2e/solid-router/js-only-file-based/src/routes/_pathlessLayout.jsxe2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout.jsxe2e/solid-router/js-only-file-based/src/routes/__root.jsxe2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout/route-a.jsxe2e/solid-router/js-only-file-based/src/routes/posts.$postId.jsx
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use workspace:* protocol for internal dependencies in package.json files
Files:
e2e/solid-router/basepath-file-based/package.jsone2e/solid-router/js-only-file-based/package.json
🧠 Learnings (4)
📚 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:
e2e/solid-router/js-only-file-based/tests/app.spec.tse2e/solid-router/js-only-file-based/src/routeTree.gen.js
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
PR: TanStack/router#0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to **/*.{ts,tsx} : Use TypeScript in strict mode with extensive type safety across the codebase
Applied to files:
e2e/solid-router/js-only-file-based/tsconfig.json
📚 Learning: 2025-10-01T18:31:35.420Z
Learnt from: schiller-manuel
PR: TanStack/router#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-router/js-only-file-based/src/routeTree.gen.js
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
PR: TanStack/router#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-router/js-only-file-based/src/main.jsx
🧬 Code graph analysis (11)
e2e/solid-router/js-only-file-based/src/routes/index.jsx (1)
e2e/solid-router/js-only-file-based/src/routes/__root.jsx (2)
Route(4-14)Route(4-14)
e2e/solid-router/js-only-file-based/tests/setup/global.setup.ts (1)
scripts/set-ts-version.js (1)
packageJson(33-33)
e2e/solid-router/js-only-file-based/src/routes/posts.index.jsx (2)
e2e/solid-router/js-only-file-based/src/routes/posts.$postId.jsx (2)
Route(6-13)Route(6-13)e2e/solid-router/js-only-file-based/src/routes/posts.route.jsx (2)
Route(6-9)Route(6-9)
e2e/solid-router/js-only-file-based/src/posts.js (1)
e2e/solid-router/js-only-file-based/src/routes/posts.$postId.jsx (1)
post(20-20)
e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout/route-b.jsx (3)
e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout.jsx (2)
Route(4-6)Route(4-6)e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout.jsx (2)
Route(4-6)Route(4-6)e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout/route-a.jsx (2)
Route(2-6)Route(2-6)
e2e/solid-router/js-only-file-based/src/routes/posts.route.jsx (2)
e2e/solid-router/js-only-file-based/src/routes/posts.$postId.jsx (3)
Route(6-13)Route(6-13)post(20-20)e2e/solid-router/js-only-file-based/src/posts.js (3)
fetchPosts(11-14)fetchPosts(11-14)post(18-20)
e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout.jsx (3)
e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout.jsx (2)
Route(4-6)Route(4-6)e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout/route-a.jsx (2)
Route(2-6)Route(2-6)e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout/route-b.jsx (2)
Route(2-6)Route(2-6)
e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout.jsx (3)
e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout.jsx (2)
Route(4-6)Route(4-6)e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout/route-a.jsx (2)
Route(2-6)Route(2-6)e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout/route-b.jsx (2)
Route(2-6)Route(2-6)
e2e/solid-router/js-only-file-based/src/routes/__root.jsx (8)
e2e/solid-router/js-only-file-based/src/routes/index.jsx (2)
Route(3-5)Route(3-5)e2e/solid-router/js-only-file-based/src/routes/posts.index.jsx (2)
Route(3-5)Route(3-5)e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout.jsx (2)
Route(4-6)Route(4-6)e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout.jsx (2)
Route(4-6)Route(4-6)e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout/route-a.jsx (2)
Route(2-6)Route(2-6)e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout/route-b.jsx (2)
Route(2-6)Route(2-6)e2e/solid-router/js-only-file-based/src/routes/posts.$postId.jsx (2)
Route(6-13)Route(6-13)e2e/solid-router/js-only-file-based/src/routes/posts.route.jsx (2)
Route(6-9)Route(6-9)
e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout/route-a.jsx (5)
e2e/solid-router/js-only-file-based/src/routes/__root.jsx (2)
Route(4-14)Route(4-14)e2e/solid-router/js-only-file-based/src/routes/index.jsx (2)
Route(3-5)Route(3-5)e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout.jsx (2)
Route(4-6)Route(4-6)e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout.jsx (2)
Route(4-6)Route(4-6)e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout/route-b.jsx (2)
Route(2-6)Route(2-6)
e2e/solid-router/js-only-file-based/src/routes/posts.$postId.jsx (2)
e2e/solid-router/js-only-file-based/src/routes/posts.route.jsx (2)
Route(6-9)Route(6-9)e2e/solid-router/js-only-file-based/src/posts.js (3)
fetchPost(16-27)fetchPost(16-27)post(18-20)
⏰ 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 (23)
e2e/solid-router/js-only-file-based/.gitignore (1)
1-11: Well-structured.gitignorefor the e2e test project.The patterns comprehensively cover all common development artifacts including Node.js packages, macOS metadata, Vite build outputs, local overrides, and Playwright test artifacts. No issues to flag.
e2e/solid-router/basepath-file-based/package.json (1)
2-2: Good catch on the package name fix.The rename corrects a naming inconsistency—the old name incorrectly included "react" for a Solid Router e2e project. The new name follows a clearer, consistent pattern that aligns with the broader e2e project naming established in this PR.
e2e/solid-router/js-only-file-based/tsconfig.json (1)
1-15: Solid TypeScript configuration that enables strict mode and aligns with Solid Router conventions.The configuration correctly enables strict type checking (Line 3), properly configures JSX for Solid (Lines 5-6), uses modern module resolution (Line 8), and includes sensible exclusions. This setup aligns with the requirement to use strict TypeScript with extensive type safety across the codebase.
The
skipLibCheck: true(Line 12) is appropriate for e2e test projects to improve build performance while still maintaining type safety for authored code.e2e/solid-router/js-only-file-based/tests/setup/global.teardown.ts (1)
1-6: LGTM!The teardown implementation is clean and correctly mirrors the setup pattern for symmetric lifecycle management.
e2e/solid-router/js-only-file-based/src/routes/index.jsx (1)
1-13: LGTM!The index route follows the correct Solid Router file-based routing pattern and provides a clean entry point for e2e testing.
e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout.jsx (1)
1-17: LGTM!The pathless layout correctly uses the underscore prefix convention and properly implements the Outlet for nested route rendering.
e2e/solid-router/js-only-file-based/tests/setup/global.setup.ts (1)
1-6: LGTM!The setup function correctly initializes the dummy server for e2e testing with proper async handling.
e2e/solid-router/js-only-file-based/postcss.config.mjs (1)
1-5: LGTM!The PostCSS configuration correctly uses the Tailwind CSS v4 plugin syntax with
@tailwindcss/postcss.e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout.jsx (1)
1-35: LGTM!The nested pathless layout correctly demonstrates navigation with active link styling using
activePropsand properly renders child routes through theOutlet.e2e/solid-router/js-only-file-based/package.json (1)
3-29: LGTM!The package configuration correctly uses the
workspace:^protocol for internal dependencies and includes appropriate scripts and dependencies for the Solid Router e2e test suite.e2e/solid-router/js-only-file-based/vite.config.js (1)
1-14: LGTM!The Vite configuration correctly sets up the TanStack Router plugin for Solid with
disableTypes: true, which aligns with the JS-only nature of this e2e test suite.e2e/solid-router/js-only-file-based/playwright.config.ts (1)
1-42: LGTM! Well-structured Playwright configuration.The configuration correctly sets up port management, global setup/teardown, a single worker for test stability, and chains build/serve commands with proper environment variable passing.
e2e/solid-router/js-only-file-based/src/routes/__root.jsx (1)
1-61: LGTM! Root route configuration is well-structured.The root component provides clear navigation, proper outlet placement, and a custom not-found handler. The intentional link to a non-existent route aligns with the e2e test coverage for not-found scenarios.
e2e/solid-router/js-only-file-based/src/routes/posts.index.jsx (1)
1-9: LGTM! Clean index route implementation.The route definition follows the file-based routing pattern correctly, and the simple placeholder component is appropriate for an index route.
e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout/route-a.jsx (1)
1-10: LGTM! Consistent route definition for nested pathless layout.The route correctly integrates with the pathless layout hierarchy and matches the pattern used in sibling route files.
e2e/solid-router/js-only-file-based/index.html (1)
1-12: LGTM! Standard Vite entrypoint.The HTML structure correctly sets up the Solid.js app mount point and module loading. The generic "Vite App" title is acceptable for an e2e test project.
e2e/solid-router/js-only-file-based/src/styles.css (1)
1-21: LGTM! Tailwind v4 compatible stylesheet.The stylesheet correctly uses the v4 import syntax and applies base styles with proper dark mode support. The @layer directive and @apply usage are compatible with Tailwind v4.
e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout/route-b.jsx (1)
1-10: LGTM! Consistent with sibling route definition.The route mirrors the structure of route-a.jsx and correctly implements the nested pathless layout pattern.
e2e/solid-router/js-only-file-based/tests/app.spec.ts (1)
1-35: LGTM! Well-structured e2e tests with excellent accessibility practices.The tests effectively cover key routing scenarios (posts navigation, nested pathless layouts, not-found handling) using role-based selectors for better accessibility alignment and test resilience.
e2e/solid-router/js-only-file-based/src/routes/posts.route.jsx (1)
6-9: LGTM!The route definition correctly wires the loader and component following Solid Router file-based routing conventions.
e2e/solid-router/js-only-file-based/src/routeTree.gen.js (1)
1-95: Autogenerated file - no review needed.This file is automatically generated by TanStack Router and should not be manually modified or reviewed.
Based on learnings
e2e/solid-router/js-only-file-based/src/routes/posts.$postId.jsx (1)
6-13: LGTM!The route configuration properly handles loading, errors, and not-found states with appropriate components.
e2e/solid-router/js-only-file-based/src/posts.js (1)
5-9: LGTM!The environment-based URL selection appropriately switches between production API and test server for e2e testing.
| const rootElement = document.getElementById('app') | ||
|
|
||
| if (!rootElement.innerHTML) { | ||
| render(() => <RouterProvider router={router} />, rootElement) | ||
| } |
There was a problem hiding this comment.
Add null safety check for rootElement.
Accessing .innerHTML on a potentially null element will throw a runtime error if the #app element doesn't exist in the DOM.
Apply this diff to add proper null safety:
const rootElement = document.getElementById('app')
-if (!rootElement.innerHTML) {
+if (rootElement && !rootElement.innerHTML) {
render(() => <RouterProvider router={router} />, rootElement)
}📝 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 rootElement = document.getElementById('app') | |
| if (!rootElement.innerHTML) { | |
| render(() => <RouterProvider router={router} />, rootElement) | |
| } | |
| const rootElement = document.getElementById('app') | |
| if (rootElement && !rootElement.innerHTML) { | |
| render(() => <RouterProvider router={router} />, rootElement) | |
| } |
🤖 Prompt for AI Agents
In e2e/solid-router/js-only-file-based/src/main.jsx around lines 14 to 18, the
code accesses rootElement.innerHTML without verifying rootElement is non-null;
update the logic to first check whether rootElement exists and only then access
innerHTML (e.g., if rootElement is null, return or throw a clear error), and
ensure the render call is executed only when rootElement is present and its
innerHTML is empty.
| if (!post) { | ||
| throw new NotFoundError(`Post with id "${postId}" not found!`) | ||
| } |
There was a problem hiding this comment.
Strengthen the post validation logic.
The check if (!post) may not catch all invalid responses. If the API returns an empty object {} or an object without expected properties, the validation will fail.
Apply this diff to add more robust validation:
- if (!post) {
+ if (!post || !post.id) {
throw new NotFoundError(`Post with id "${postId}" not found!`)
}Or use more comprehensive validation:
- if (!post) {
+ if (!post || typeof post !== 'object' || !post.id || !post.title) {
throw new NotFoundError(`Post with id "${postId}" not found!`)
}📝 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.
| if (!post) { | |
| throw new NotFoundError(`Post with id "${postId}" not found!`) | |
| } | |
| if (!post || !post.id) { | |
| throw new NotFoundError(`Post with id "${postId}" not found!`) | |
| } |
🤖 Prompt for AI Agents
In e2e/solid-router/js-only-file-based/src/posts.js around lines 22 to 24, the
current validation uses if (!post) which misses cases like an empty object or
objects missing expected fields; update the validation to verify the post has
the required properties and types (e.g., post.id and post.title or whichever
fields your app expects) and throw NotFoundError when any required property is
absent or invalid; alternatively, integrate a lightweight runtime schema check
(e.g., a small utility or a Zod/validator call) to assert shape and types before
returning the post.
birkskyum
force-pushed
the
test(solid-router)--js-only-file-based-e2e-suite
branch
from
6bae1b6 to
86c4d72
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout.jsx (2)
1-2: Combine duplicate imports from the same module.Minor cleanup.
-import { createFileRoute } from '@tanstack/solid-router' -import { Link, Outlet } from '@tanstack/solid-router' +import { createFileRoute, Link, Outlet } from '@tanstack/solid-router'
.
12-29: Optional: wrap nav links in a semanticImproves a11y without behavior change.
- <div class="flex gap-2 border-b"> + <nav class="flex gap-2 border-b" aria-label="Nested routes"> <Link to="/route-a" activeProps={{ class: 'font-bold', }} > Go to route A </Link> <Link to="/route-b" activeProps={{ class: 'font-bold', }} > Go to route B </Link> - </div> + </nav>e2e/solid-router/js-only-file-based/playwright.config.ts (2)
29-34: Cross‑platform env vars for webServer command.On Windows, VAR=... prefix won’t work. Consider cross-env.
- webServer: { - command: `VITE_NODE_ENV="test" VITE_EXTERNAL_PORT=${EXTERNAL_PORT} VITE_SERVER_PORT=${PORT} pnpm build && pnpm serve --port ${PORT}`, + webServer: { + command: `pnpm exec cross-env VITE_NODE_ENV=test VITE_EXTERNAL_PORT=${EXTERNAL_PORT} VITE_SERVER_PORT=${PORT} pnpm build && pnpm exec cross-env VITE_EXTERNAL_PORT=${EXTERNAL_PORT} VITE_SERVER_PORT=${PORT} pnpm serve --port ${PORT}`,You’ll need cross-env as a devDependency in this e2e package.
19-19: Simplify reporter declaration.No options used; string form is cleaner.
- reporter: [['line']], + reporter: 'line',e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout.jsx (1)
1-2: Combine duplicate imports from the same module.Minor cleanup.
-import { createFileRoute } from '@tanstack/solid-router' -import { Outlet } from '@tanstack/solid-router' +import { createFileRoute, Outlet } from '@tanstack/solid-router'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (25)
e2e/solid-router/basepath-file-based/package.json(1 hunks)e2e/solid-router/generator-cli-only/package.json(1 hunks)e2e/solid-router/js-only-file-based/.gitignore(1 hunks)e2e/solid-router/js-only-file-based/index.html(1 hunks)e2e/solid-router/js-only-file-based/package.json(1 hunks)e2e/solid-router/js-only-file-based/playwright.config.ts(1 hunks)e2e/solid-router/js-only-file-based/postcss.config.mjs(1 hunks)e2e/solid-router/js-only-file-based/src/main.jsx(1 hunks)e2e/solid-router/js-only-file-based/src/posts.js(1 hunks)e2e/solid-router/js-only-file-based/src/routeTree.gen.js(1 hunks)e2e/solid-router/js-only-file-based/src/routes/__root.jsx(1 hunks)e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout.jsx(1 hunks)e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout.jsx(1 hunks)e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout/route-a.jsx(1 hunks)e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout/route-b.jsx(1 hunks)e2e/solid-router/js-only-file-based/src/routes/index.jsx(1 hunks)e2e/solid-router/js-only-file-based/src/routes/posts.$postId.jsx(1 hunks)e2e/solid-router/js-only-file-based/src/routes/posts.index.jsx(1 hunks)e2e/solid-router/js-only-file-based/src/routes/posts.route.jsx(1 hunks)e2e/solid-router/js-only-file-based/src/styles.css(1 hunks)e2e/solid-router/js-only-file-based/tests/app.spec.ts(1 hunks)e2e/solid-router/js-only-file-based/tests/setup/global.setup.ts(1 hunks)e2e/solid-router/js-only-file-based/tests/setup/global.teardown.ts(1 hunks)e2e/solid-router/js-only-file-based/tsconfig.json(1 hunks)e2e/solid-router/js-only-file-based/vite.config.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (15)
- e2e/solid-router/js-only-file-based/vite.config.js
- e2e/solid-router/js-only-file-based/index.html
- e2e/solid-router/js-only-file-based/postcss.config.mjs
- e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout/route-b.jsx
- e2e/solid-router/js-only-file-based/src/main.jsx
- e2e/solid-router/js-only-file-based/src/routes/posts.$postId.jsx
- e2e/solid-router/js-only-file-based/src/routes/__root.jsx
- e2e/solid-router/basepath-file-based/package.json
- e2e/solid-router/js-only-file-based/src/posts.js
- e2e/solid-router/js-only-file-based/package.json
- e2e/solid-router/js-only-file-based/src/routes/index.jsx
- e2e/solid-router/js-only-file-based/tests/setup/global.teardown.ts
- e2e/solid-router/js-only-file-based/src/routes/posts.route.jsx
- e2e/solid-router/js-only-file-based/.gitignore
- e2e/solid-router/js-only-file-based/src/styles.css
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
e2e/solid-router/js-only-file-based/tests/setup/global.setup.tse2e/solid-router/js-only-file-based/tests/app.spec.tse2e/solid-router/js-only-file-based/playwright.config.ts
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/solid-router/js-only-file-based/tests/setup/global.setup.tse2e/solid-router/js-only-file-based/tests/app.spec.tse2e/solid-router/generator-cli-only/package.jsone2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout/route-a.jsxe2e/solid-router/js-only-file-based/src/routes/_pathlessLayout.jsxe2e/solid-router/js-only-file-based/tsconfig.jsone2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout.jsxe2e/solid-router/js-only-file-based/src/routes/posts.index.jsxe2e/solid-router/js-only-file-based/src/routeTree.gen.jse2e/solid-router/js-only-file-based/playwright.config.ts
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use workspace:* protocol for internal dependencies in package.json files
Files:
e2e/solid-router/generator-cli-only/package.json
**/src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Place file-based routes under src/routes/ directories
Files:
e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout/route-a.jsxe2e/solid-router/js-only-file-based/src/routes/_pathlessLayout.jsxe2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout.jsxe2e/solid-router/js-only-file-based/src/routes/posts.index.jsx
🧠 Learnings (3)
📚 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:
e2e/solid-router/js-only-file-based/tests/app.spec.tse2e/solid-router/js-only-file-based/src/routeTree.gen.js
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
PR: TanStack/router#0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to **/*.{ts,tsx} : Use TypeScript in strict mode with extensive type safety across the codebase
Applied to files:
e2e/solid-router/js-only-file-based/tsconfig.json
📚 Learning: 2025-10-01T18:31:35.420Z
Learnt from: schiller-manuel
PR: TanStack/router#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-router/js-only-file-based/src/routeTree.gen.js
🧬 Code graph analysis (6)
e2e/solid-router/js-only-file-based/tests/setup/global.setup.ts (1)
scripts/set-ts-version.js (1)
packageJson(33-33)
e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout/route-a.jsx (3)
e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout.jsx (2)
Route(4-6)Route(4-6)e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout.jsx (2)
Route(4-6)Route(4-6)e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout/route-b.jsx (2)
Route(2-6)Route(2-6)
e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout.jsx (4)
e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout.jsx (2)
Route(4-6)Route(4-6)e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout/route-a.jsx (2)
Route(2-6)Route(2-6)e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout/route-b.jsx (2)
Route(2-6)Route(2-6)examples/solid/start-basic-netlify/src/routes/_pathlessLayout.tsx (1)
LayoutComponent(7-16)
e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout.jsx (3)
e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout.jsx (2)
Route(4-6)Route(4-6)e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout/route-a.jsx (2)
Route(2-6)Route(2-6)e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout/route-b.jsx (2)
Route(2-6)Route(2-6)
e2e/solid-router/js-only-file-based/src/routes/posts.index.jsx (2)
e2e/solid-router/js-only-file-based/src/routes/posts.$postId.jsx (2)
Route(6-13)Route(6-13)e2e/solid-router/js-only-file-based/src/routes/posts.route.jsx (2)
Route(6-9)Route(6-9)
e2e/solid-router/js-only-file-based/playwright.config.ts (1)
scripts/set-ts-version.js (1)
packageJson(33-33)
⏰ 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 (7)
e2e/solid-router/js-only-file-based/tsconfig.json (1)
1-15: Configuration is well-formed and follows best practices.The tsconfig properly enables strict type checking as required by the coding guidelines, includes all necessary SolidJS-specific settings (
jsx: "preserve"andjsxImportSource: "solid-js"), and uses modern module resolution appropriate for e2e tests with contemporary tooling.e2e/solid-router/generator-cli-only/package.json (1)
2-2: Package name corrected to reflect Solid.js context.The name has been updated from
tanstack-solid-router-e2e-react-generator-cli-only(which incorrectly referenced React) totanstack-router-e2e-solid-generator-cli-only, which is now consistent with the Solid.js package at this location. All internal dependencies properly use theworkspace:^protocol as per coding guidelines.Verify that the naming convention is applied consistently across the other updated e2e package.json files in this PR (specifically, check that the similar naming pattern has been applied to
basepath-file-basedand the newjs-only-file-basedpackages).e2e/solid-router/js-only-file-based/src/routes/posts.index.jsx (1)
3-9: LGTM for index route wiring.Index child under /posts is correctly declared; component is minimal and fine for e2e.
e2e/solid-router/js-only-file-based/src/routes/_pathlessLayout/_nested-layout/route-a.jsx (1)
2-10: LGTM.Route path and component are correct; integrates with the pathless parent as expected.
e2e/solid-router/js-only-file-based/tests/setup/global.setup.ts (1)
4-6: LGTM.Dummy server bootstrap is straightforward and correct.
e2e/solid-router/js-only-file-based/src/routeTree.gen.js (1)
1-95: Autogenerated file — no manual review.routeTree.gen.js is generated and should not be hand‑edited. Skipping review. Based on learnings.
e2e/solid-router/js-only-file-based/playwright.config.ts (1)
6-10: No TypeScript configuration issues found.The configuration correctly supports JSON import assertions and top-level await:
resolveJsonModule: trueenables JSON importsmodule: "ESNext"andtarget: "ESNext"enable top-level await- The code will execute as written
The review comment assumes
moduleResolution: "NodeNext"is required, but the actual config uses"Bundler", which is equally valid for bundler-based environments and properly supports both features.Likely an incorrect or invalid review comment.
| await expect(page.getByRole('paragraph')).toContainText( | ||
| 'This is the notFoundComponent configured on root route', | ||
| ) |
There was a problem hiding this comment.
Fix invalid ARIA role: ‘paragraph’ is not a valid role.
Use text or element selectors; current code will fail.
- await expect(page.getByRole('paragraph')).toContainText(
- 'This is the notFoundComponent configured on root route',
- )
+ await expect(
+ page.getByText('This is the notFoundComponent configured on root route')
+ ).toBeVisible()
+ // or:
+ // await expect(page.locator('p')).toContainText('This is the notFoundComponent configured on root route')📝 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.
| await expect(page.getByRole('paragraph')).toContainText( | |
| 'This is the notFoundComponent configured on root route', | |
| ) | |
| await expect( | |
| page.getByText('This is the notFoundComponent configured on root route') | |
| ).toBeVisible() | |
| // or: | |
| // await expect(page.locator('p')).toContainText('This is the notFoundComponent configured on root route') |
🤖 Prompt for AI Agents
In e2e/solid-router/js-only-file-based/tests/app.spec.ts around lines 30 to 32,
the test is using an invalid ARIA role "paragraph" so the selector will fail;
replace the getByRole('paragraph') call with a text or element selector — for
example use page.getByText('This is the notFoundComponent configured on root
route') or target the paragraph element with page.locator('p', { hasText: 'This
is the notFoundComponent configured on root route' }) so the assertion finds the
exact text correctly.
Summary by CodeRabbit
New Features
Tests
Chores