-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Solid hydration #5518
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Solid hydration #5518
Conversation
WalkthroughMigrates many root routes to emit full HTML documents with HydrationScript/HeadContent/Scripts, introduces async client bootstrap via hydrateStart and StartClient(router), simplifies StartServer, adds client/server hydration guards in router internals, and adjusts renderRouterToString to include a DOCTYPE. Changes
Sequence Diagram(s)sequenceDiagram
participant Entry as Client Entry
participant HS as hydrateStart()
participant Router as Router Instance
participant SC as StartClient
participant RP as RouterProvider
Entry->>HS: call hydrateStart()
HS-->>Entry: returns router (async)
Entry->>SC: render StartClient(router)
SC->>RP: mount RouterProvider with router
RP-->>Entry: app rendered / hydration continues
Note right of Entry: HTML includes HydrationScript in <head> and HeadContent/Scripts in <body>
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/solid-router/package.json (1)
100-111
: Move Vite to devDependencies—it's a build tool not used at runtime.Vite has no runtime imports in the source and requires Node >=18, conflicting with the declared engines >=12. Leaving it in dependencies bloats installs and breaks consumers. The suggested diff correctly moves it to devDependencies where build tools belong.
"dependencies": { "@solid-devtools/logger": "^0.9.4", "@solid-primitives/refs": "^1.0.8", "@solidjs/meta": "^0.29.4", "@tanstack/history": "workspace:*", "@tanstack/router-core": "workspace:*", "@tanstack/solid-store": "0.7.0", "isbot": "^5.1.22", "tiny-invariant": "^1.3.3", - "tiny-warning": "^1.0.3", - "vite": "^7.1.7" + "tiny-warning": "^1.0.3" }, "devDependencies": { + "vite": "^7.1.7", "@solidjs/testing-library": "^0.8.10",packages/solid-start-server/package.json (1)
60-68
: Move Vite from dependencies to devDependencies.Vite is not used at runtime in this package (no imports found in source code) and is strictly a build tool. Keeping it in dependencies unnecessarily increases consumer install size.
The Node engine requirement (≥22.12.0) is compatible with Vite's requirement (^20.19.0 || ≥22.12.0), and vite-plugin-solid 2.11.9 supports Vite ^7.0.0, so no compatibility concerns.
Apply the suggested diff to move vite to devDependencies.
🧹 Nitpick comments (2)
packages/solid-router/src/Matches.tsx (1)
28-29
: Fix import order to satisfy ESLint.ESLint reports that these imports should be ordered differently (non-type imports before type imports, and external packages before internal ones).
Apply this diff to fix the import order:
+import { HydrationScript } from 'solid-js/web' import * as Solid from 'solid-js' import warning from 'tiny-warning' import { CatchBoundary, ErrorComponent } from './CatchBoundary' import { useRouterState } from './useRouterState' import { useRouter } from './useRouter' import { Transitioner } from './Transitioner' import { matchContext } from './matchContext' import { Match } from './Match' import { SafeFragment } from './SafeFragment' +import { Scripts } from './Scripts' import type { AnyRouter, DeepPartial, Expand, MakeOptionalPathParams, MakeOptionalSearchParams, MakeRouteMatchUnion, MaskOptions, MatchRouteOptions, NoInfer, RegisteredRouter, ResolveRelativePath, ResolveRoute, RouteByPath, RouterState, ToSubOptionsProps, } from '@tanstack/router-core' -import { Scripts } from './Scripts' -import { HydrationScript } from 'solid-js/web'examples/solid/start-bare/src/routes/__root.tsx (1)
2-6
: Clean imports and use type-only for Solid’s JSX.Remove unused imports and switch to import type for JSX. Also fix import order per lint.
Apply:
-import { createRootRoute, HeadContent, Link, Outlet, Scripts } from '@tanstack/solid-router' +import { createRootRoute, HeadContent, Scripts } from '@tanstack/solid-router' -import * as Solid from 'solid-js' +import type { JSX } from 'solid-js' import { Hydration, HydrationScript, NoHydration } from 'solid-js/web' -import appCss from '~/styles/app.css?url' +import appCss from '~/styles/app.css?url' -function RootDocument({ children }: { children: Solid.JSX.Element }) { +function RootDocument({ children }: { children: JSX.Element }) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
examples/solid/start-bare/src/routes/__root.tsx
(2 hunks)packages/solid-router/package.json
(1 hunks)packages/solid-router/src/Matches.tsx
(8 hunks)packages/solid-start-client/src/StartClient.tsx
(1 hunks)packages/solid-start-server/package.json
(1 hunks)packages/solid-start-server/src/StartServer.tsx
(1 hunks)packages/solid-start/src/default-entry/client.tsx
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
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/package.json
packages/solid-router/src/Matches.tsx
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use workspace:* protocol for internal dependencies in package.json files
Files:
packages/solid-router/package.json
packages/solid-start-server/package.json
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/solid-start/src/default-entry/client.tsx
packages/solid-start-client/src/StartClient.tsx
packages/solid-start-server/src/StartServer.tsx
examples/solid/start-bare/src/routes/__root.tsx
packages/solid-router/src/Matches.tsx
packages/{*-start,start-*}/**
📄 CodeRabbit inference engine (AGENTS.md)
Name and place Start framework packages under packages/-start/ or packages/start-/
Files:
packages/solid-start/src/default-entry/client.tsx
**/src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Place file-based routes under src/routes/ directories
Files:
examples/solid/start-bare/src/routes/__root.tsx
examples/{react,solid}/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep example applications under examples/react/ and examples/solid/
Files:
examples/solid/start-bare/src/routes/__root.tsx
🧠 Learnings (1)
📓 Common learnings
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/
🧬 Code graph analysis (5)
packages/solid-start/src/default-entry/client.tsx (1)
packages/solid-start-client/src/StartClient.tsx (1)
StartClient
(6-15)
packages/solid-start-client/src/StartClient.tsx (1)
packages/react-start-client/src/StartClient.tsx (1)
StartClient
(8-19)
packages/solid-start-server/src/StartServer.tsx (1)
packages/react-start-server/src/StartServer.tsx (1)
StartServer
(5-9)
examples/solid/start-bare/src/routes/__root.tsx (1)
packages/solid-router/src/index.tsx (2)
HeadContent
(353-353)Scripts
(354-354)
packages/solid-router/src/Matches.tsx (4)
packages/solid-router/src/Transitioner.tsx (1)
Transitioner
(11-140)packages/solid-router/src/useRouterState.tsx (1)
useRouterState
(20-36)packages/router-core/src/routeInfo.ts (1)
RouteByPath
(99-102)packages/router-core/src/link.ts (1)
ResolveRelativePath
(189-205)
🪛 ESLint
packages/solid-start-client/src/StartClient.tsx
[error] 1-1: Member 'Suspense' of the import declaration should be sorted alphabetically.
(sort-imports)
[error] 2-2: 'Await' is defined but never used.
(unused-imports/no-unused-imports)
packages/solid-start-server/src/StartServer.tsx
[error] 2-2: Member 'Show' of the import declaration should be sorted alphabetically.
(sort-imports)
[error] 2-2: 'Show' is defined but never used.
(unused-imports/no-unused-imports)
examples/solid/start-bare/src/routes/__root.tsx
[error] 2-2: Member 'HeadContent' of the import declaration should be sorted alphabetically.
(sort-imports)
[error] 2-2: 'Link' is defined but never used.
(unused-imports/no-unused-imports)
[error] 2-2: 'Outlet' is defined but never used.
(unused-imports/no-unused-imports)
[error] 3-3: ~/styles/app.css?url
import should occur after import of solid-js/web
(import/order)
[error] 4-4: All imports in the declaration are only used as types. Use import type
.
(@typescript-eslint/consistent-type-imports)
packages/solid-router/src/Matches.tsx
[error] 1-1: Do not use "@ts-nocheck" because it alters compilation errors.
(@typescript-eslint/ban-ts-comment)
[error] 28-28: ./Scripts
import should occur before type import of @tanstack/router-core
(import/order)
[error] 29-29: solid-js/web
import should occur before import of ./CatchBoundary
(import/order)
🔇 Additional comments (4)
packages/solid-router/src/Matches.tsx (3)
179-185
: TypeScript 5.0+ is configured and const type parameters are supported.The
const
modifier on generic type parameters is a TypeScript 5.0+ feature that only affects compile-time type inference and does not depend on the runtime compilation target. The project's TypeScript dependencies range from 5.6.2 to 5.9.2, well above the 5.0 requirement. No changes are needed.
56-56
: Remove generic approval language; change is verified as correct.The Transitioner component does have proper internal SSR/hydration handling. Both Solid and React implementations check
(typeof window !== 'undefined' && router.ssr)
before attempting to load the router during SSR hydration, correctly deferring tossr-client
for initial loading. Moving the SSR gate from the parent (Matches.tsx) to Transitioner.tsx is appropriate and architecturally cleaner.
44-44
: Verify SSR/CSR compatibility with unconditional root Suspense.The hardcoded
Solid.Suspense
at the root level (line 44) lacks SSR conditionals, creating an asymmetry with the childMatch.tsx
component which still conditionally chooses betweenSolid.Suspense
andSafeFragment
. WhileHydrationScript
presence indicates hydration awareness, the unconditional approach differs from other router components (e.g.,ClientOnly.tsx
,Asset.tsx
,Match.tsx
) that checkisServer
. Confirm this doesn't cause SSR serialization issues or hydration mismatches, particularly in edge cases where root-level Suspense interaction with hydration markers matters.packages/solid-start/src/default-entry/client.tsx (1)
5-7
: Consider whether blocking hydration until router is ready aligns with the intended UX.The current implementation uses top-level
await
to block hydration until the router is initialized. This delays Time-to-Interactive (TTI), as users see the SSR'd page but cannot interact with it untilgetRouter()
resolves.Since
StartClient
already usesSuspense
internally (per the relevant snippet), an alternative approach would be to hydrate immediately and let Suspense handle the async router loading:hydrate(() => <StartClient router={getRouter()} />, document)Then modify
StartClient
to handle the promise or use a resource pattern. This would allow faster hydration while still handling async router initialization gracefully.Please verify whether the blocking behavior is intentional for this hydration architecture or whether leveraging Suspense for non-blocking hydration would better serve the user experience.
<NoHydration> | ||
<html> | ||
<head> | ||
<HeadContent /> | ||
<HydrationScript /> | ||
</head> | ||
<body> | ||
<Hydration> | ||
{children} | ||
</Hydration> | ||
<Scripts /> | ||
</body> | ||
</html> | ||
</NoHydration> | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify NoHydration wrapping the entire document; nested Hydration may not re-enable hydration.
Placing at the root can disable hydration for descendants. Confirm that inside re-enables hydration for Solid 1.9+. You may need to move to targeted islands or remove it.
Option A (remove NoHydration):
- <NoHydration>
<html>
<head>
<HeadContent />
<HydrationScript />
</head>
<body>
- <Hydration>
- {children}
- </Hydration>
+ <Hydration>{children}</Hydration>
<Scripts />
</body>
- </html>
- </NoHydration>
+ </html>
Web check:
🌐 Web query:
In Solid 1.9+, does <Hydration> nested inside <NoHydration> re-enable hydration for its subtree, or does <NoHydration> hard-disable hydration regardless of nesting? Cite official docs/examples.
💡 Result:
No — hard-disables hydration for its subtree. The docs state that placing a inside has no effect and will not override 's behavior. [1] See also the release note describing the API (for starting/stopping hydratable sections) for context. [2]
Sources:
[1] Solid docs — .
[2] Solid release notes (Hydration / Islands support).
Remove wrapper; nested cannot override it.
The web search confirms that hard-disables hydration for its entire subtree— nested inside it has no effect and cannot re-enable hydration. Placing a inside will not override 's behavior. The current code structure prevents hydration from working.
Apply the suggested fix (Option A): remove the outer <NoHydration>
wrapper and keep only <Hydration>{children}</Hydration>
in the body.
- <NoHydration>
<html>
<head>
<HeadContent />
<HydrationScript />
</head>
<body>
- <Hydration>
- {children}
- </Hydration>
+ <Hydration>{children}</Hydration>
<Scripts />
</body>
- </html>
- </NoHydration>
+ </html>
🤖 Prompt for AI Agents
In examples/solid/start-bare/src/routes/__root.tsx around lines 16 to 30, the
root layout wraps the entire document in <NoHydration> which disables hydration
for its subtree and prevents the nested <Hydration> from having any effect;
remove the outer <NoHydration> wrapper and instead wrap only the page children
with <Hydration> in the <body>, leaving <HeadContent /> and <HydrationScript />
in the <head> and keeping <Scripts /> after the hydrated children so hydration
is actually enabled.
`The following error wasn't caught by any route! At the very leas | ||
t, consider setting an 'errorComponent' in your RootRoute!`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix malformed error message string.
The error message string is incorrectly split across lines, breaking the word "least" and introducing unwanted whitespace.
Apply this diff to fix the formatting:
warning(
false,
- `The following error wasn't caught by any route! At the very leas
- t, consider setting an 'errorComponent' in your RootRoute!`,
+ `The following error wasn't caught by any route! At the very least, consider setting an 'errorComponent' in your RootRoute!`,
)
📝 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.
`The following error wasn't caught by any route! At the very leas | |
t, consider setting an 'errorComponent' in your RootRoute!`, | |
warning( | |
false, | |
`The following error wasn't caught by any route! At the very least, consider setting an 'errorComponent' in your RootRoute!`, | |
) |
🤖 Prompt for AI Agents
In packages/solid-router/src/Matches.tsx around lines 102 to 103, the error
message string is split across lines causing the word "least" to be broken and
adding unwanted whitespace; fix it by joining the split string into a single
continuous string (remove the line break and any extra spaces) so the message
reads correctly, e.g. "The following error wasn't caught by any route! At the
very least, consider setting an 'errorComponent' in your RootRoute!".
import { getRouter } from '#tanstack-router-entry' | ||
|
||
hydrate(() => <StartClient />, document.body) | ||
const router = await getRouter(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add error handling for router initialization.
The top-level await
has no try-catch block. If getRouter()
rejects, the entire module will fail to execute and hydration will never occur, leaving users with a non-interactive page and no error feedback.
Apply this diff to add error handling:
-const router = await getRouter();
+let router;
+try {
+ router = await getRouter();
+} catch (error) {
+ console.error('Failed to initialize router:', error);
+ throw error; // Re-throw or provide fallback behavior
+}
Alternatively, consider whether router initialization failures should gracefully degrade or display an error boundary to the user.
📝 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 router = await getRouter(); | |
let router; | |
try { | |
router = await getRouter(); | |
} catch (error) { | |
console.error('Failed to initialize router:', error); | |
throw error; // Re-throw or provide fallback behavior | |
} |
🤖 Prompt for AI Agents
In packages/solid-start/src/default-entry/client.tsx around line 5, the
top-level await calling getRouter() has no error handling; wrap the router
initialization in a try-catch so that if getRouter() rejects we catch the error,
log it (or report to your monitoring), and provide a safe fallback (e.g., set
router to null/undefined and proceed with a no-router hydration path or mount an
error boundary/UI message) rather than letting the module throw and block
hydration; ensure any caught error is surfaced appropriately (console/error
logger) and that downstream code checks for a missing router before using it.
|
Command | Status | Duration | Result |
---|---|---|---|
nx affected --targets=test:eslint,test:unit,tes... |
❌ Failed | 6m 20s | View ↗ |
nx run-many --target=build --exclude=examples/*... |
✅ Succeeded | 3s | View ↗ |
☁️ Nx Cloud last updated this comment at 2025-10-20 04:00:53
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-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: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/solid-router/src/Matches.tsx (1)
98-99
: Fix the malformed error message string.The error message is incorrectly split across lines, breaking the word "least" into "leas" and "t", which will introduce unwanted whitespace in the output.
Apply this diff:
`The following error wasn't caught by any route! At the very leas - t, consider setting an 'errorComponent' in your RootRoute!`, + `The following error wasn't caught by any route! At the very least, consider setting an 'errorComponent' in your RootRoute!`,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
examples/solid/start-bare/src/routes/__root.tsx
(2 hunks)examples/solid/start-basic-static/src/routes/__root.tsx
(2 hunks)examples/solid/start-basic/src/routes/__root.tsx
(2 hunks)packages/solid-router/package.json
(1 hunks)packages/solid-router/src/Matches.tsx
(4 hunks)packages/solid-router/src/RouterProvider.tsx
(1 hunks)packages/solid-start-client/src/StartClient.tsx
(1 hunks)packages/solid-start-server/package.json
(1 hunks)packages/solid-start-server/src/StartServer.tsx
(1 hunks)packages/solid-start/src/default-entry/client.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/solid-router/package.json
- packages/solid-start/src/default-entry/client.tsx
- packages/solid-start-server/package.json
🧰 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:
examples/solid/start-basic-static/src/routes/__root.tsx
packages/solid-start-client/src/StartClient.tsx
packages/solid-start-server/src/StartServer.tsx
examples/solid/start-basic/src/routes/__root.tsx
examples/solid/start-bare/src/routes/__root.tsx
packages/solid-router/src/RouterProvider.tsx
packages/solid-router/src/Matches.tsx
**/src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Place file-based routes under src/routes/ directories
Files:
examples/solid/start-basic-static/src/routes/__root.tsx
examples/solid/start-basic/src/routes/__root.tsx
examples/solid/start-bare/src/routes/__root.tsx
examples/{react,solid}/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep example applications under examples/react/ and examples/solid/
Files:
examples/solid/start-basic-static/src/routes/__root.tsx
examples/solid/start-basic/src/routes/__root.tsx
examples/solid/start-bare/src/routes/__root.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/src/RouterProvider.tsx
packages/solid-router/src/Matches.tsx
🧠 Learnings (2)
📚 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:
examples/solid/start-bare/src/routes/__root.tsx
packages/solid-router/src/RouterProvider.tsx
📚 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:
packages/solid-router/src/Matches.tsx
🧬 Code graph analysis (4)
packages/solid-start-client/src/StartClient.tsx (2)
packages/react-start-client/src/StartClient.tsx (1)
StartClient
(8-19)packages/solid-router/src/RouterProvider.tsx (1)
RouterProvider
(44-53)
packages/solid-start-server/src/StartServer.tsx (2)
packages/react-start-server/src/StartServer.tsx (1)
StartServer
(5-9)packages/solid-router/src/RouterProvider.tsx (1)
RouterProvider
(44-53)
examples/solid/start-bare/src/routes/__root.tsx (1)
packages/solid-router/src/index.tsx (3)
HeadContent
(353-353)Link
(229-229)Scripts
(354-354)
packages/solid-router/src/Matches.tsx (2)
packages/solid-router/src/Transitioner.tsx (1)
Transitioner
(11-140)packages/solid-router/src/Match.tsx (1)
Match
(22-155)
⏰ 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 (6)
examples/solid/start-basic/src/routes/__root.tsx (1)
9-9
: LGTM! Proper hydration-aware document structure.The root document now correctly renders a full HTML structure with
HydrationScript
in the head andScripts
at the end of the body, following the standard Solid Start hydration pattern. The structure aligns with the broader PR changes.Also applies to: 62-126
examples/solid/start-basic-static/src/routes/__root.tsx (1)
10-10
: LGTM! Consistent hydration pattern across examples.This follows the same hydration-aware HTML document structure as the other examples, with
HydrationScript
in the head and proper placement ofHeadContent
andScripts
.Also applies to: 77-141
packages/solid-start-server/src/StartServer.tsx (1)
1-8
: LGTM! Server-side rendering simplified correctly.The component now directly renders
RouterProvider
without unnecessary Suspense or resource gating, matching the React implementation pattern and addressing the concerns from the previous review.packages/solid-router/src/RouterProvider.tsx (1)
3-3
: LGTM! Import reordering with no behavioral change.These changes only adjust import statement ordering without affecting runtime behavior.
Also applies to: 9-9
packages/solid-router/src/Matches.tsx (1)
10-10
: LGTM! Routing logic and Transitioner rendering corrected.The import reordering, unconditional
Transitioner
rendering, and properMatch
component rendering withSolid.Show
all look good. The previous critical issues with debug code and type checking have been resolved.Also applies to: 60-60, 79-85
examples/solid/start-bare/src/routes/__root.tsx (1)
2-11
: LGTM! Hydration wrapper issue resolved with proper document structure.The component now renders a clean HTML document structure without the problematic
NoHydration
wrapper that was flagged in the previous review. The hydration-aware pattern withHydrationScript
in the head andScripts
in the body is correct and consistent with the other examples.Also applies to: 20-37
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
e2e/solid-start/server-functions/src/routes/__root.tsx (1)
17-18
: Normalize meta charset key for Solid.Use
charset
(lowercase) for Solid’s meta tag generation to ensure correct output.- charSet: 'utf-8', + charset: 'utf-8',e2e/solid-start/scroll-restoration/src/routes/__root.tsx (1)
50-52
: Fix or remove invalid color on manifest link.
rel: 'manifest'
doesn’t usecolor
, and#fffff
is invalid hex. Remove the property (or movecolor
to amask-icon
link if intended).- { rel: 'manifest', href: '/site.webmanifest', color: '#fffff' }, + { rel: 'manifest', href: '/site.webmanifest' },e2e/solid-start/basic/src/routes/__root.tsx (1)
48-50
: Remove invalid color on manifest link.- { rel: 'manifest', href: '/site.webmanifest', color: '#fffff' }, + { rel: 'manifest', href: '/site.webmanifest' },e2e/solid-start/website/src/routes/__root.tsx (1)
48-50
: Remove invalid color on manifest link.- { rel: 'manifest', href: '/site.webmanifest', color: '#fffff' }, + { rel: 'manifest', href: '/site.webmanifest' },e2e/solid-start/spa-mode/src/routes/__root.tsx (1)
20-21
: Normalize to charset for Solid meta.- charSet: 'utf-8', + charset: 'utf-8',e2e/solid-start/server-routes/src/routes/__root.tsx (1)
17-18
: Usecharset
instead ofcharSet
.- charSet: 'utf-8', + charset: 'utf-8',
🧹 Nitpick comments (11)
e2e/solid-start/server-functions/src/routes/__root.tsx (1)
35-45
: Add html lang attribute for a11y/SEO.Set the document language.
- <html> + <html lang="en">e2e/solid-start/scroll-restoration/src/routes/__root.tsx (1)
63-77
: Add html lang attribute.- <html> + <html lang="en">e2e/solid-start/basic/src/routes/__root.tsx (2)
18-28
: Add charset meta first in head.Ensure UTF‑8 is declared early.
head: () => ({ - meta: [ + meta: [ + { charset: 'utf-8' }, { name: 'viewport', content: 'width=device-width, initial-scale=1', },
70-158
: Add html lang attribute.- <html> + <html lang="en">e2e/solid-start/website/src/routes/__root.tsx (1)
61-71
: Add html lang attribute.- <html> + <html lang="en">e2e/solid-start/spa-mode/src/routes/__root.tsx (1)
72-108
: Add html lang attribute.- <html> + <html lang="en">e2e/solid-start/server-routes/src/routes/__root.tsx (1)
35-45
: Add html lang attribute.- <html> + <html lang="en">e2e/solid-start/custom-basepath/src/routes/__root.tsx (2)
58-112
: Add lang + semantic landmarks (nav/main) for a11y
- html needs a lang.
- Use nav for the menu and wrap routed content in main; add aria-current to active links.
Apply:
- <html> + <html lang="en"> <head> <HeadContent /> <HydrationScript /> </head> - <body> - <div class="p-2 flex gap-2 text-lg"> + <body> + <nav class="p-2 flex gap-2 text-lg" aria-label="Primary"> <Link to="/" activeProps={{ - class: 'font-bold', + class: 'font-bold', + 'aria-current': 'page', }} activeOptions={{ exact: true }} > Home </Link>{' '} <Link to="/posts" activeProps={{ - class: 'font-bold', + class: 'font-bold', + 'aria-current': 'page', }} > Posts </Link>{' '} <Link to="/users" activeProps={{ - class: 'font-bold', + class: 'font-bold', + 'aria-current': 'page', }} > Users </Link>{' '} <Link to="/deferred" activeProps={{ - class: 'font-bold', + class: 'font-bold', + 'aria-current': 'page', }} > Deferred </Link>{' '} <Link // @ts-expect-error to="/this-route-does-not-exist" activeProps={{ - class: 'font-bold', + class: 'font-bold', + 'aria-current': 'page', }} > This Route Does Not Exist - </Link> - </div> - <Outlet /> + </Link> + </nav> + <main id="main" role="main"> + <Outlet /> + </main> <TanStackRouterDevtoolsInProd /> <Scripts /> </body> </html>
109-110
: Unify devtools import/usage for consistencyOther files import/use TanStackRouterDevtools. Consider switching to that alias here for consistency (same runtime behavior).
e2e/solid-start/selective-ssr/src/routes/__root.tsx (1)
126-160
: Add lang + semantic landmarks (nav/main)Improve accessibility and document semantics.
Apply:
- return ( - <html> + return ( + <html lang="en"> <head> <HeadContent /> <HydrationScript /> </head> - <body> - <div class="p-2 flex gap-2 text-lg"> + <body> + <nav class="p-2 flex gap-2 text-lg" aria-label="Primary"> <h1>Selective SSR E2E Test</h1> <Link to="/" activeProps={{ - class: 'font-bold', + class: 'font-bold', + 'aria-current': 'page', }} > Home </Link> - </div> + </nav> <hr /> <ClientOnly> <div> router isLoading:{' '} <b data-testid="router-isLoading"> {routerState().isLoading ? 'true' : 'false'} </b> </div> <div> router status:{' '} <b data-testid="router-status">{routerState().status}</b> </div> </ClientOnly> <hr /> - {children} + <main id="main" role="main">{children}</main> <Scripts /> </body> </html>e2e/solid-start/basic-tsr-config/src/app/routes/__root.tsx (1)
30-41
: Add lang + main landmarkMatch other examples and improve a11y.
Apply:
- <html> + <html lang="en"> <head> <HeadContent /> <HydrationScript /> </head> <body> - <Outlet /> + <main id="main" role="main"> + <Outlet /> + </main> <TanStackRouterDevtools position="bottom-right" /> <Scripts /> </body> </html>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
e2e/solid-start/basic-tsr-config/src/app/routes/__root.tsx
(2 hunks)e2e/solid-start/basic/src/client.tsx
(1 hunks)e2e/solid-start/basic/src/routes/__root.tsx
(2 hunks)e2e/solid-start/custom-basepath/src/routes/__root.tsx
(2 hunks)e2e/solid-start/scroll-restoration/src/routes/__root.tsx
(2 hunks)e2e/solid-start/selective-ssr/src/routes/__root.tsx
(2 hunks)e2e/solid-start/server-functions/src/routes/__root.tsx
(2 hunks)e2e/solid-start/server-routes/src/routes/__root.tsx
(2 hunks)e2e/solid-start/spa-mode/src/routes/__root.tsx
(2 hunks)e2e/solid-start/website/src/routes/__root.tsx
(2 hunks)packages/solid-start-client/src/index.tsx
(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:
packages/solid-start-client/src/index.tsx
e2e/solid-start/custom-basepath/src/routes/__root.tsx
e2e/solid-start/website/src/routes/__root.tsx
e2e/solid-start/basic/src/client.tsx
e2e/solid-start/basic/src/routes/__root.tsx
e2e/solid-start/server-functions/src/routes/__root.tsx
e2e/solid-start/server-routes/src/routes/__root.tsx
e2e/solid-start/spa-mode/src/routes/__root.tsx
e2e/solid-start/basic-tsr-config/src/app/routes/__root.tsx
e2e/solid-start/scroll-restoration/src/routes/__root.tsx
e2e/solid-start/selective-ssr/src/routes/__root.tsx
**/src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Place file-based routes under src/routes/ directories
Files:
e2e/solid-start/custom-basepath/src/routes/__root.tsx
e2e/solid-start/website/src/routes/__root.tsx
e2e/solid-start/basic/src/routes/__root.tsx
e2e/solid-start/server-functions/src/routes/__root.tsx
e2e/solid-start/server-routes/src/routes/__root.tsx
e2e/solid-start/spa-mode/src/routes/__root.tsx
e2e/solid-start/scroll-restoration/src/routes/__root.tsx
e2e/solid-start/selective-ssr/src/routes/__root.tsx
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/solid-start/custom-basepath/src/routes/__root.tsx
e2e/solid-start/website/src/routes/__root.tsx
e2e/solid-start/basic/src/client.tsx
e2e/solid-start/basic/src/routes/__root.tsx
e2e/solid-start/server-functions/src/routes/__root.tsx
e2e/solid-start/server-routes/src/routes/__root.tsx
e2e/solid-start/spa-mode/src/routes/__root.tsx
e2e/solid-start/basic-tsr-config/src/app/routes/__root.tsx
e2e/solid-start/scroll-restoration/src/routes/__root.tsx
e2e/solid-start/selective-ssr/src/routes/__root.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: TanStack/router#0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to docs/{router,start}/** : Place router docs under docs/router/ and start framework docs under docs/start/
📚 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-start/custom-basepath/src/routes/__root.tsx
e2e/solid-start/basic-tsr-config/src/app/routes/__root.tsx
🧬 Code graph analysis (4)
e2e/solid-start/custom-basepath/src/routes/__root.tsx (1)
packages/router-devtools/src/index.tsx (1)
TanStackRouterDevtoolsInProd
(5-5)
e2e/solid-start/basic/src/routes/__root.tsx (1)
packages/solid-router/src/HeadContent.tsx (1)
HeadContent
(187-197)
e2e/solid-start/spa-mode/src/routes/__root.tsx (1)
packages/solid-router/src/ClientOnly.tsx (1)
ClientOnly
(32-38)
e2e/solid-start/selective-ssr/src/routes/__root.tsx (1)
packages/solid-router/src/ClientOnly.tsx (1)
ClientOnly
(32-38)
⏰ 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 (3)
packages/solid-start-client/src/index.tsx (1)
2-2
: Dependency verified and export path confirmed valid.The re-export is correctly configured. Verification confirms:
@tanstack/start-client-core
is properly declared inpackages/solid-start-client/package.json
as a workspace dependency (workspace:*
)@tanstack/start-client-core/client
is a valid export path with proper subpath configuration inpackages/start-client-core/package.json
hydrateStart
is correctly exported from that path- The pattern matches usage in other packages (react-start-client, solid-start)
No concerns remain.
e2e/solid-start/basic-tsr-config/src/app/routes/__root.tsx (1)
10-24
: Verify meta charset keyUpstream examples use
charSet: 'utf-8'
. Here it'scharset
. Confirm the correct casing for the head manager so the tag renders as expected.e2e/solid-start/basic/src/client.tsx (1)
8-8
: The review comment is incorrect. TLA is intentional and already supported.The production default entry file (packages/solid-start/src/default-entry/client.tsx) already uses TLA with
const router = await hydrateStart()
, which the e2e test correctly mirrors. The root tsconfig specifies"target": "ES2020"
and"module": "ES2022"
, both of which support top-level await. The e2e test file is consistent with the established production pattern and requires no changes.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/solid-router/src/Transitioner.tsx (1)
18-20
: LGTM! Server guard correctly prevents client-only transition logic.The early return when
router.isServer
is true appropriately prevents client-side signals, effects, and mounting behavior from executing during SSR.Optional refinement: Consider moving the guard before the
useRouterState()
call on lines 14-16 to avoid creating an unusedisLoading
signal on the server:export function Transitioner() { const router = useRouter() + + if (router.isServer) { + return null + } + let mountLoadForRouter = { router, mounted: false } const isLoading = useRouterState({ select: ({ isLoading }) => isLoading, }) - - if (router.isServer) { - return null - }This would avoid any unnecessary reactive subscription setup on the server.
packages/solid-router/src/ClientOnly.tsx (1)
39-65
: Solid implementation with room for optimization.The
useHydrated
hook is correctly implemented and well-documented. Consider these optional improvements:
Performance optimization: Each
ClientOnly
instance creates a new signal. If many instances exist, consider using a global hydration signal or context to avoid redundant state.Potential export: The JSDoc example (disabling buttons) suggests this hook could be useful beyond
ClientOnly
. Consider exporting it as a public utility if there are valid use cases elsewhere in the codebase.Example of a global approach:
// Create once, share across all components const [globalHydrated, setGlobalHydrated] = createSignal(false) let hydrationInitialized = false function useHydrated(): Solid.Accessor<boolean> { if (!hydrationInitialized) { hydrationInitialized = true Solid.onMount(() => setGlobalHydrated(true)) } return globalHydrated }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
e2e/solid-start/basic/src/client.tsx
(1 hunks)packages/solid-router/src/ClientOnly.tsx
(2 hunks)packages/solid-router/src/Transitioner.tsx
(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:
packages/solid-router/src/Transitioner.tsx
packages/solid-router/src/ClientOnly.tsx
e2e/solid-start/basic/src/client.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/src/Transitioner.tsx
packages/solid-router/src/ClientOnly.tsx
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/solid-start/basic/src/client.tsx
⏰ 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 (4)
packages/solid-router/src/ClientOnly.tsx (2)
5-5
: LGTM: Documentation wording improved.The change from "if" to "when" is more accurate and clearer.
32-34
: Excellent fix for hydration mismatches!The shift from
!isServer
tohydrated()
eliminates a hydration mismatch issue. Previously, the server would render the fallback while the client's initial render would immediately show children, causing a mismatch. Now both server and client render the fallback initially, then transition to children afteronMount
fires on the client.e2e/solid-start/basic/src/client.tsx (2)
4-4
: LGTM!The import additions support the new two-step hydration flow.
10-10
: No issues found — code is correct as-is.The
hydrate()
function fromsolid-js/web
does acceptdocument
as a valid target for full-document hydration. This pattern is already used in the default entry point (packages/solid-start/src/default-entry/client.tsx
) and is documented in Solid.js as the standard way to hydrate entire documents in SSR contexts, including head elements.
e2e/solid-start/basic/src/client.tsx
Outdated
console.log("[client-entry]: using custom client entry in 'src/client.tsx'") | ||
|
||
hydrate(() => <StartClient />, document.body) | ||
const router = await hydrateStart() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add error handling for router initialization.
The top-level await has no error handling. If hydrateStart()
rejects, it will cause an unhandled promise rejection that crashes the entire client bootstrap without proper error reporting.
Apply this diff to add error handling:
-const router = await hydrateStart()
+const router = await hydrateStart().catch((error) => {
+ console.error('[client-entry]: Failed to initialize router:', error)
+ throw error
+})
Or wrap in try-catch for more comprehensive error handling:
-const router = await hydrateStart()
+let router
+try {
+ router = await hydrateStart()
+} catch (error) {
+ console.error('[client-entry]: Failed to initialize router:', error)
+ // Optionally render an error UI or retry logic here
+ throw error
+}
🤖 Prompt for AI Agents
In e2e/solid-start/basic/src/client.tsx around line 8, the top-level await
calling hydrateStart() lacks error handling; wrap the call in a try/catch
(declare router in an outer scope if needed), await hydrateStart() inside the
try, and in the catch log the error (console.error or a structured logger),
surface a minimal fallback (e.g. render a simple error UI or stop further
bootstrap), and avoid leaving the promise unhandled so the client bootstrap does
not crash silently.
b1c0610
to
7c2df63
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (5)
e2e/solid-start/scroll-restoration/src/routes/__root.tsx (1)
63-77
: Move HeadContent to and add (avoid FOUC; improve a11y).Render head assets/meta before body; keep Scripts as last child.
- <html> - <head> - <HydrationScript /> - </head> - <body> - <HeadContent /> + <html lang="en"> + <head> + <HeadContent /> + <HydrationScript /> + </head> + <body> <Nav type="header" /> <hr /> <Outlet /> <hr /> <Nav type="footer" /> <TanStackRouterDevtools position="bottom-right" /> <Scripts /> </body> </html>Please confirm whether HeadContent is internally hoisted to ; if not, this change is required to prevent style/metadata lateness.
e2e/solid-start/website/src/routes/__root.tsx (1)
61-71
: Place HeadContent in and set .Ensures meta/links/styles load before body; improves accessibility.
- <html> - <head> - <HydrationScript /> - </head> - <body> - <HeadContent /> + <html lang="en"> + <head> + <HeadContent /> + <HydrationScript /> + </head> + <body> <Outlet /> <TanStackRouterDevtools position="bottom-right" /> <Scripts /> </body> </html>Confirm whether HeadContent is automatically hoisted; if not, this prevents potential FOUC.
e2e/solid-start/server-routes/src/routes/__root.tsx (1)
35-46
: Hoist HeadContent into and add .Prevents late-loading head assets; improves a11y.
- <html> - <head> - <HydrationScript /> - </head> - <body> - <HeadContent /> + <html lang="en"> + <head> + <HeadContent /> + <HydrationScript /> + </head> + <body> <Outlet /> <TanStackRouterDevtools position="bottom-right" /> <Scripts /> </body> </html>Please verify whether HeadContent is auto-hoisted to during SSR; if not, this change is needed.
e2e/solid-start/spa-mode/src/routes/__root.tsx (1)
72-107
: Hoist HeadContent, make Scripts last, and add .Ensures head assets load early; consistent body end script ordering; better a11y.
- <html> - <head> - <HydrationScript /> - </head> - <body> - <HeadContent /> + <html lang="en"> + <head> + <HeadContent /> + <HydrationScript /> + </head> + <body> <div class="p-2 flex gap-2 text-lg"> <h1>SPA Mode E2E Test</h1> <Link to="/" activeProps={{ class: 'font-bold', }} > Home </Link> </div> <hr /> <ClientOnly> <div> router isLoading:{' '} <b data-testid="router-isLoading"> {routerState().isLoading ? 'true' : 'false'} </b> </div> <div> router status:{' '} <b data-testid="router-status">{routerState().status}</b> </div> </ClientOnly> <hr /> {children} - <Scripts /> - <TanStackRouterDevtools position="bottom-right" /> + <TanStackRouterDevtools position="bottom-right" /> + <Scripts /> </body> </html>Please confirm that HeadContent isn’t relying on body placement for any behavior.
e2e/solid-start/basic/src/routes/__root.tsx (1)
70-157
: Move HeadContent into and add .Aligns with typical Solid/SSR patterns; avoids late CSS/meta application.
- <html> - <head> - <HydrationScript /> - </head> - <body> - <HeadContent /> + <html lang="en"> + <head> + <HeadContent /> + <HydrationScript /> + </head> + <body> <div class="p-2 flex gap-2 text-lg"> <Link to="/" activeProps={{ class: 'font-bold', }} activeOptions={{ exact: true }} > Home </Link>{' '} <Link to="/posts" activeProps={{ class: 'font-bold', }} > Posts </Link>{' '} <Link to="/users" activeProps={{ class: 'font-bold', }} > Users </Link>{' '} <Link to="/layout-a" activeProps={{ class: 'font-bold', }} > Layout </Link>{' '} <Link to="/scripts" activeProps={{ class: 'font-bold', }} > Scripts </Link>{' '} <Link to="/inline-scripts" activeProps={{ class: 'font-bold', }} > Inline Scripts </Link>{' '} <Link // @ts-expect-error to="/this-route-does-not-exist" activeProps={{ class: 'font-bold', }} > This Route Does Not Exist </Link> </div> <Outlet /> <div class="inline-div">This is an inline styled div</div> <TanStackRouterDevtoolsInProd /> <Scripts /> </body> </html>Please verify if HeadContent is auto-hoisted to at render time; if not, this change is necessary.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
e2e/solid-start/basic-tsr-config/src/app/routes/__root.tsx
(2 hunks)e2e/solid-start/basic/src/routes/__root.tsx
(2 hunks)e2e/solid-start/custom-basepath/src/routes/__root.tsx
(2 hunks)e2e/solid-start/scroll-restoration/src/routes/__root.tsx
(2 hunks)e2e/solid-start/selective-ssr/src/routes/__root.tsx
(2 hunks)e2e/solid-start/server-functions/src/routes/__root.tsx
(2 hunks)e2e/solid-start/server-routes/src/routes/__root.tsx
(2 hunks)e2e/solid-start/spa-mode/src/routes/__root.tsx
(2 hunks)e2e/solid-start/website/src/routes/__root.tsx
(2 hunks)packages/solid-router/src/HeadContent.tsx
(1 hunks)packages/solid-router/src/ssr/renderRouterToString.tsx
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/solid-router/src/HeadContent.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- e2e/solid-start/server-functions/src/routes/__root.tsx
- e2e/solid-start/custom-basepath/src/routes/__root.tsx
- e2e/solid-start/basic-tsr-config/src/app/routes/__root.tsx
🧰 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-start/basic/src/routes/__root.tsx
e2e/solid-start/scroll-restoration/src/routes/__root.tsx
packages/solid-router/src/ssr/renderRouterToString.tsx
e2e/solid-start/server-routes/src/routes/__root.tsx
e2e/solid-start/selective-ssr/src/routes/__root.tsx
e2e/solid-start/spa-mode/src/routes/__root.tsx
e2e/solid-start/website/src/routes/__root.tsx
**/src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Place file-based routes under src/routes/ directories
Files:
e2e/solid-start/basic/src/routes/__root.tsx
e2e/solid-start/scroll-restoration/src/routes/__root.tsx
e2e/solid-start/server-routes/src/routes/__root.tsx
e2e/solid-start/selective-ssr/src/routes/__root.tsx
e2e/solid-start/spa-mode/src/routes/__root.tsx
e2e/solid-start/website/src/routes/__root.tsx
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/solid-start/basic/src/routes/__root.tsx
e2e/solid-start/scroll-restoration/src/routes/__root.tsx
e2e/solid-start/server-routes/src/routes/__root.tsx
e2e/solid-start/selective-ssr/src/routes/__root.tsx
e2e/solid-start/spa-mode/src/routes/__root.tsx
e2e/solid-start/website/src/routes/__root.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/src/ssr/renderRouterToString.tsx
🧬 Code graph analysis (6)
e2e/solid-start/basic/src/routes/__root.tsx (1)
packages/solid-router/src/HeadContent.tsx (1)
HeadContent
(189-199)
e2e/solid-start/scroll-restoration/src/routes/__root.tsx (1)
packages/solid-router/src/HeadContent.tsx (1)
HeadContent
(189-199)
e2e/solid-start/server-routes/src/routes/__root.tsx (1)
packages/solid-router/src/HeadContent.tsx (1)
HeadContent
(189-199)
e2e/solid-start/selective-ssr/src/routes/__root.tsx (2)
packages/solid-router/src/HeadContent.tsx (1)
HeadContent
(189-199)packages/solid-router/src/ClientOnly.tsx (1)
ClientOnly
(31-38)
e2e/solid-start/spa-mode/src/routes/__root.tsx (2)
packages/solid-router/src/HeadContent.tsx (1)
HeadContent
(189-199)packages/solid-router/src/ClientOnly.tsx (1)
ClientOnly
(31-38)
e2e/solid-start/website/src/routes/__root.tsx (1)
packages/solid-router/src/HeadContent.tsx (1)
HeadContent
(189-199)
⏰ 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 (1)
e2e/solid-start/selective-ssr/src/routes/__root.tsx (1)
4-4
: LGTM! Imports aligned with hydration model.The added imports (
HeadContent
,Scripts
,HydrationScript
) are correct for the hydration-aware document structure.Also applies to: 7-7, 12-12
<html> | ||
<head> | ||
<HydrationScript /> | ||
</head> | ||
<body> | ||
<HeadContent /> | ||
<div class="p-2 flex gap-2 text-lg"> | ||
<h1>Selective SSR E2E Test</h1> | ||
<Link | ||
to="/" | ||
activeProps={{ | ||
class: 'font-bold', | ||
}} | ||
> | ||
Home | ||
</Link> | ||
</div> | ||
<div> | ||
router status:{' '} | ||
<b data-testid="router-status">{routerState().status}</b> | ||
</div> | ||
</ClientOnly> | ||
<hr /> | ||
{children} | ||
</> | ||
<hr /> | ||
<ClientOnly> | ||
<div> | ||
router isLoading:{' '} | ||
<b data-testid="router-isLoading"> | ||
{routerState().isLoading ? 'true' : 'false'} | ||
</b> | ||
</div> | ||
<div> | ||
router status:{' '} | ||
<b data-testid="router-status">{routerState().status}</b> | ||
</div> | ||
</ClientOnly> | ||
<hr /> | ||
{children} | ||
<Scripts /> | ||
</body> | ||
</html> | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify HeadContent
placement and consider adding lang
attribute.
The document structure looks appropriate for hydration, but two items need attention:
-
HeadContent in body (line 131): The
HeadContent
component is rendered inside<body>
, but its name and typical behavior suggest it should be in<head>
. Verify this is correct for@tanstack/solid-router
's implementation—some frameworks use portals/teleportation from body to head, but this should be confirmed. -
Missing
lang
attribute (line 126): The<html>
tag should include alang
attribute for accessibility (WCAG 3.1.1 Level A).
Suggested improvement for accessibility:
- <html>
+ <html lang="en">
Run the following to check if other root routes in the codebase follow a similar pattern for HeadContent
placement:
🏁 Script executed:
#!/bin/bash
# Check HeadContent placement in other root routes
rg -n -A2 -B2 '<HeadContent' --type=tsx --type=ts -g 'e2e/**/routes/__root.tsx'
Length of output: 27
I'll correct the search command and also verify the HeadContent behavior through web documentation.
🏁 Script executed:
#!/bin/bash
# Search for HeadContent placement without strict type specification
rg -n -A2 -B2 '<HeadContent' e2e --type=js --type=ts
Length of output: 43
🌐 Web query:
@tanstack/solid-router HeadContent placement where to use
💡 Result:
Short answer
- Render in your root layout's (Start / full‑stack apps). For SPA apps, include at the top-level/root route component (ideally where you can affect the document ). [1][2][3]
Details / notes
- For Start/full‑stack: put inside the of your root route HTML so title/meta/link/head scripts are managed server‑side. [1][2]
- For single‑page apps: include in the root route component (or root shell) so the router can mount/unmount route head tags as navigation occurs. Also render in the root body where appropriate. [2][3]
Examples
- Full‑stack root: [2]
- SPA/root route:
<>
</> [3]
Sources
[1] TanStack Start — Routing (HeadContent description).
[2] TanStack Router — Head management / docs (usage for Start and SPA).
[3] TanStack Start — Learn the basics (root route example with HeadContent & Scripts).
---
🏁 Script executed:
```shell
#!/bin/bash
# Check if this is a Solid Start project by looking at the package.json in the e2e test directory
if [ -f "e2e/solid-start/selective-ssr/package.json" ]; then
echo "=== package.json found ==="
cat "e2e/solid-start/selective-ssr/package.json"
else
echo "Checking parent level..."
find e2e/solid-start -name "package.json" -type f | head -5
fi
Length of output: 846
Move HeadContent
from <body>
to <head>
and add lang
attribute.
This is a Solid Start full-stack project, and per TanStack documentation, HeadContent
must be rendered in the <head>
, not <body>
. Currently at line 131, it's incorrectly placed in <body>
. Additionally, the <html>
tag should include a lang
attribute for accessibility (WCAG 3.1.1 Level A).
Required fixes:
- <html>
+ <html lang="en">
<head>
<HydrationScript />
+ <HeadContent />
</head>
<body>
- <HeadContent />
<div class="p-2 flex gap-2 text-lg">
📝 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.
<html> | |
<head> | |
<HydrationScript /> | |
</head> | |
<body> | |
<HeadContent /> | |
<div class="p-2 flex gap-2 text-lg"> | |
<h1>Selective SSR E2E Test</h1> | |
<Link | |
to="/" | |
activeProps={{ | |
class: 'font-bold', | |
}} | |
> | |
Home | |
</Link> | |
</div> | |
<div> | |
router status:{' '} | |
<b data-testid="router-status">{routerState().status}</b> | |
</div> | |
</ClientOnly> | |
<hr /> | |
{children} | |
</> | |
<hr /> | |
<ClientOnly> | |
<div> | |
router isLoading:{' '} | |
<b data-testid="router-isLoading"> | |
{routerState().isLoading ? 'true' : 'false'} | |
</b> | |
</div> | |
<div> | |
router status:{' '} | |
<b data-testid="router-status">{routerState().status}</b> | |
</div> | |
</ClientOnly> | |
<hr /> | |
{children} | |
<Scripts /> | |
</body> | |
</html> | |
) | |
<html lang="en"> | |
<head> | |
<HydrationScript /> | |
<HeadContent /> | |
</head> | |
<body> | |
<div class="p-2 flex gap-2 text-lg"> | |
<h1>Selective SSR E2E Test</h1> | |
<Link | |
to="/" | |
activeProps={{ | |
class: 'font-bold', | |
}} | |
> | |
Home | |
</Link> | |
</div> | |
<hr /> | |
<ClientOnly> | |
<div> | |
router isLoading:{' '} | |
<b data-testid="router-isLoading"> | |
{routerState().isLoading ? 'true' : 'false'} | |
</b> | |
</div> | |
<div> | |
router status:{' '} | |
<b data-testid="router-status">{routerState().status}</b> | |
</div> | |
</ClientOnly> | |
<hr /> | |
{children} | |
<Scripts /> | |
</body> | |
</html> | |
) |
🤖 Prompt for AI Agents
In e2e/solid-start/selective-ssr/src/routes/__root.tsx around lines 126 to 161,
HeadContent is currently rendered inside the <body> and the <html> tag lacks a
lang attribute; move the <HeadContent /> element from the body into the <head>
(e.g., after <HydrationScript />) and add a lang attribute to the <html> tag
(for example lang="en"); ensure you remove the HeadContent instance from the
body so it only renders in the head.
) | ||
html = html.replace(`</body>`, `${injectedHtml}</body>`) | ||
return new Response(html, { | ||
return new Response(`<!DOCTYPE html>${html}`, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify no duplicate DOCTYPE declarations in rendered components.
The DOCTYPE addition aligns well with the PR's goal of emitting full HTML documents. However, ensure that the children()
function doesn't already render a DOCTYPE declaration, which would result in duplicate declarations in the final output.
Run the following script to check for potential duplicate DOCTYPE declarations:
🏁 Script executed:
#!/bin/bash
# Description: Search for DOCTYPE declarations in Solid components that might be rendered via children()
# Search for DOCTYPE in TSX/JSX files that might render full documents
rg -n -t tsx -t ts --type-add 'tsx:*.tsx' -C 3 '<!DOCTYPE|doctype' packages/solid-router/ packages/solid-start-client/ packages/solid-start-server/
# Search for components that might render <html> tags (indicating full document rendering)
ast-grep --pattern 'return <html $$$>$$$</html>'
Length of output: 1128
Now I need to examine how the docType
variable from RouterServer.tsx is used to determine if there's a duplicate DOCTYPE risk:
Now I need to check how the docType
variable in RouterServer.tsx is being used:
Verify and prevent duplicate DOCTYPE declarations in the rendered output.
The docType
variable defined in RouterServer.tsx as const docType = ssr("<!DOCTYPE html>")
is rendered within the component tree alongside <Root />
. This means when renderToString()
renders the component hierarchy, the DOCTYPE is already included in the resulting HTML string.
Line 23 prepends an additional <!DOCTYPE html>
to this output, creating duplicate DOCTYPE declarations. This is invalid HTML and will cause parsing issues.
Fix required: Either remove the DOCTYPE from the component tree (RouterServer.tsx) or remove the prepended DOCTYPE from line 23, not both.
🤖 Prompt for AI Agents
In packages/solid-router/src/ssr/renderRouterToString.tsx around line 23, the
function unconditionally prepends "<!DOCTYPE html>" to the string returned by
renderToString, which duplicates the docType already injected into the component
tree from RouterServer.tsx; remove the hardcoded prepend at line 23 so the
function returns new Response(html) (i.e., rely on the docType included by the
component tree), and ensure RouterServer.tsx still provides const docType =
ssr("<!DOCTYPE html>") so there is a single DOCTYPE in the final output.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
examples/solid/start-bare/src/routes/__root.tsx
(2 hunks)examples/solid/start-basic-static/src/routes/__root.tsx
(2 hunks)examples/solid/start-basic/src/routes/__root.tsx
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/solid/start-basic-static/src/routes/__root.tsx
🧰 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:
examples/solid/start-basic/src/routes/__root.tsx
examples/solid/start-bare/src/routes/__root.tsx
**/src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Place file-based routes under src/routes/ directories
Files:
examples/solid/start-basic/src/routes/__root.tsx
examples/solid/start-bare/src/routes/__root.tsx
examples/{react,solid}/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep example applications under examples/react/ and examples/solid/
Files:
examples/solid/start-basic/src/routes/__root.tsx
examples/solid/start-bare/src/routes/__root.tsx
🧠 Learnings (1)
📚 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:
examples/solid/start-bare/src/routes/__root.tsx
🧬 Code graph analysis (2)
examples/solid/start-basic/src/routes/__root.tsx (1)
packages/react-router/src/index.tsx (3)
HeadContent
(349-349)Link
(148-148)Scripts
(350-350)
examples/solid/start-bare/src/routes/__root.tsx (1)
packages/solid-router/src/index.tsx (3)
HeadContent
(353-353)Link
(229-229)Scripts
(354-354)
⏰ 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 (4)
examples/solid/start-basic/src/routes/__root.tsx (2)
9-9
: LGTM: Correct import for Solid hydration.The
HydrationScript
import from 'solid-js/web' is appropriate for enabling hydration in the Solid Start application.
68-124
: LGTM: Body structure and script placement are correct.The body content is well-structured with:
- Navigation links with proper active state handling
- Properly documented TypeScript error suppression for the intentional invalid route (line 111)
- Scripts component correctly placed at the end of the body for optimal loading
examples/solid/start-bare/src/routes/__root.tsx (2)
2-18
: LGTM! Imports and route configuration are properly structured.The hydration-aware imports (
HydrationScript
,HeadContent
,Scripts
) are correctly included, and the route configuration withshellComponent
follows the expected pattern for the new document model.
26-36
: The review comment is incorrect—<HeadContent />
placement in the body is the correct pattern for Solid Start server-rendered examples.The script results confirm that
<HeadContent />
in the<body>
(not in<head>
) is the consistent pattern across all server-rendered Solid Start examples:start-bare
,start-basic
, andstart-basic-static
all follow this same structure. The<HeadContent />
component in Solid Start uses portals to inject head content into the actual<head>
element during hydration, even though it's placed in the JSX body. The code is correct as-is and requires no changes.Likely an incorrect or invalid review comment.
<html> | ||
<head> | ||
<HydrationScript /> | ||
</head> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Add lang
attribute to <html>
for accessibility; verify essential meta tags are injected.
The <html>
element is missing a lang
attribute, which is important for accessibility and SEO. Additionally, there are no visible <meta charset>
or <meta name="viewport">
tags. While these might be injected by the framework or the head()
function, please verify they are being added properly.
Apply this diff to add the lang attribute:
- <html>
+ <html lang="en">
<head>
<HydrationScript />
</head>
Run the following script to check how other root routes in the codebase handle HTML attributes and meta tags:
🏁 Script executed:
#!/bin/bash
# Check how other root routes define html and head elements
rg -A 5 '<html' --type tsx examples/
Length of output: 27
I need to fix the script approach. Let me search the codebase with corrected commands and also check the specific file directly.Now let me inspect the specific file and check for other examples:
🏁 Script executed:
#!/bin/bash
# Directly inspect the target file to see current state
echo "=== Content of examples/solid/start-bare/src/routes/__root.tsx ==="
cat examples/solid/start-bare/src/routes/__root.tsx
echo -e "\n=== Checking other root route files in examples ==="
find examples -name "__root.tsx" -type f
Length of output: 3932
Let me check a few other root route examples to see if they use the lang
attribute:
🏁 Script executed:
#!/bin/bash
# Check lang attribute usage in several other root routes
echo "=== React start-bare example ==="
sed -n '1,50p' examples/react/start-bare/src/routes/__root.tsx | grep -A 5 '<html'
echo -e "\n=== React basic-file-based example ==="
sed -n '1,50p' examples/react/basic-file-based/src/routes/__root.tsx | grep -A 5 '<html'
echo -e "\n=== Solid start-basic example ==="
sed -n '1,50p' examples/solid/start-basic/src/routes/__root.tsx | grep -A 5 '<html'
echo -e "\n=== Checking for meta tag injection patterns ==="
grep -r "charset\|viewport" examples/solid/start-bare/ --include="*.tsx" --include="*.ts" --include="*.html" 2>/dev/null | head -20
Length of output: 246
Let me do broader searches to understand the pattern across the codebase:
🏁 Script executed:
#!/bin/bash
# Check if ANY examples use lang attribute
echo "=== Checking for lang attribute across ALL examples ==="
grep -r 'lang="' examples/ --include="*.tsx" --include="*.html" 2>/dev/null | head -10
echo -e "\n=== Checking for meta tags in server or config files ==="
find examples/solid/start-bare -type f \( -name "*.ts" -o -name "*.tsx" -o -name "*.config.*" -o -name "*.html" \) | head -20
echo -e "\n=== Checking for HeadContent or meta tag handling ==="
grep -r "HeadContent\|<meta\|charset\|viewport" examples/solid/start-bare/src/ --include="*.tsx" --include="*.ts" 2>/dev/null
Length of output: 1329
Add lang="en"
attribute to <html>
element for accessibility and consistency.
The <html>
element in examples/solid/start-bare/src/routes/__root.tsx is missing the lang
attribute. Other examples in the codebase—particularly React SSR examples like basic-ssr-file-based and basic-ssr-streaming-file-based—include <html lang="en">
, confirming this is the expected pattern. The lang
attribute is important for accessibility and SEO. The HeadContent
component handles dynamic head elements, so visible meta tags are not strictly necessary, but the lang
attribute must be explicit on the HTML element.
Apply this diff:
- <html>
+ <html lang="en">
<head>
<HydrationScript />
</head>
📝 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.
<html> | |
<head> | |
<HydrationScript /> | |
</head> | |
<html lang="en"> | |
<head> | |
<HydrationScript /> | |
</head> |
🤖 Prompt for AI Agents
In examples/solid/start-bare/src/routes/__root.tsx around lines 22 to 25, the
<html> element is missing the lang attribute; update the opening <html> tag to
include lang="en" (i.e., change <html> to <html lang="en">) to match the
project's SSR examples and satisfy accessibility/SEO expectations.
<html> | ||
<head> | ||
<HydrationScript /> | ||
</head> | ||
<body> | ||
<HeadContent /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify HeadContent
placement – should it be in <head>
?
The HeadContent
component is currently placed inside the <body>
tag (line 67), but it's responsible for rendering the route's head configuration (meta tags, stylesheets, favicons defined in lines 17-54). Standard HTML requires these elements to be in the <head>
section, not the body.
The AI summary states "Moves HeadContent into the head", which contradicts the current code placement. This may prevent proper rendering of critical head elements like the stylesheet (appCss
), SEO meta tags, and viewport configuration.
Suggested fix: Move HeadContent
into the <head>
section:
<html>
<head>
<HydrationScript />
+ <HeadContent />
</head>
<body>
- <HeadContent />
<div class="p-2 flex gap-2 text-lg">
If this body placement is intentional for Solid-specific hydration reasons, please verify that head elements are rendering correctly in the final HTML output.
I'll verify the HeadContent
placement and Solid.js framework behavior to validate this review comment.
---
🏁 Script executed:
```shell
#!/bin/bash
# Examine the __root.tsx file to see current HeadContent placement
cat examples/solid/start-basic/src/routes/__root.tsx | sed -n '50,80p'
Length of output: 758
🏁 Script executed:
#!/bin/bash
# Search for HeadContent import in the file
grep -n "import.*HeadContent\|from.*HeadContent" examples/solid/start-basic/src/routes/__root.tsx
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Show the entire __root.tsx file structure to understand context
cat examples/solid/start-basic/src/routes/__root.tsx
Length of output: 3120
🏁 Script executed:
#!/bin/bash
# Check if there are other __root.tsx files using HeadContent for comparison
find . -name "__root.tsx" -type f | head -5
Length of output: 327
🏁 Script executed:
#!/bin/bash
# Search for HeadContent usage patterns in the repository
rg "HeadContent" --type tsx -A 2 -B 2 | head -40
Length of output: 27
🌐 Web query:
TanStack Solid Router HeadContent component placement head body
💡 Result:
Short answer — put HeadContent in the document head of your root layout. For Start / full‑stack apps render inside of the root route’s layout; for SPA apps (when you can’t control a real ) render it as high in the root component tree as possible (e.g. at the top of the root component before ). Scripts that belong in the body should use and be placed in . [1][2]
Sources:
- TanStack Start (Solid) — Routing / HeadContent guidance. [1]
- TanStack Router — Document Head Management (HeadContent + Scripts). [2]
Move HeadContent
from <body>
to <head>
tag
For Start / full-stack apps, render <HeadContent />
inside <head>
of the root route's layout. The current code incorrectly places it in the <body>
tag (line 68), preventing proper rendering of the head configuration (meta tags, stylesheets, favicons) defined in the route's head()
function.
Required fix:
<html>
<head>
<HydrationScript />
+ <HeadContent />
</head>
<body>
- <HeadContent />
<div class="p-2 flex gap-2 text-lg">
📝 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.
<html> | |
<head> | |
<HydrationScript /> | |
</head> | |
<body> | |
<HeadContent /> | |
<html> | |
<head> | |
<HydrationScript /> | |
<HeadContent /> | |
</head> | |
<body> |
🤖 Prompt for AI Agents
In examples/solid/start-basic/src/routes/__root.tsx around lines 62 to 67,
HeadContent is currently rendered inside the <body> which prevents the route
head() metadata from being applied; move the <HeadContent /> component from
inside the <body> to inside the <head> element (so the head contains
HydrationScript and HeadContent), ensuring head-related meta tags, stylesheets
and favicons are rendered correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
e2e/solid-start/basic/src/routes/stream.tsx (2)
2-2
: Fix import member sorting.ESLint flags that import members should be sorted alphabetically (uppercase before lowercase).
Apply this diff:
-import { createEffect, createSignal, Suspense } from 'solid-js' +import { Suspense, createEffect, createSignal } from 'solid-js'
48-62
: LGTM! Suspense boundary supports hydration workflow.The Suspense boundary correctly wraps the async Await component, which aligns with the PR's hydration and streaming objectives.
Consider adding a fallback to improve the user experience during the suspense phase:
- <Suspense> + <Suspense fallback={<div class="p-2">Loading...</div>}>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
e2e/solid-start/basic/src/routes/stream.tsx
(3 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/stream.tsx
**/src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Place file-based routes under src/routes/ directories
Files:
e2e/solid-start/basic/src/routes/stream.tsx
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/solid-start/basic/src/routes/stream.tsx
🪛 ESLint
e2e/solid-start/basic/src/routes/stream.tsx
[error] 2-2: Member 'Suspense' 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: Preview
- GitHub Check: Test
Summary by CodeRabbit
New Features
Bug Fixes
Chores