test(solid-start): serialization-adapters suite #5557
Conversation
WalkthroughAdds a new Solid Start e2e project for serialization adapters (routes, data models, adapters, tests, build tooling) and wires serialization-adapter plugins into Solid Router's SSR renderers; also changes Await to use deferred promise streams. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant Server
participant SSR
participant Router
participant Seroval as SerovalPlugins
Browser->>Router: HTTP request
Router->>SSR: render route tree
SSR->>Seroval: obtain serializationAdapters from router.options
Seroval->>SSR: provide seroval plugins
SSR->>Server: renderToStream(..., plugins)
Server-->>Browser: HTML stream + serialized payload
Note over Browser,Server: Hydration
Browser->>Seroval: deserialize payload via adapters
Browser->>Router: hydrate client router with reconstructed objects
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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 199c360
☁️ 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: |
birkskyum
force-pushed
the
solid-start-serialization-adapters-e2e-suite
branch
from
1fd8be6 to
46ce2aa
Compare
birkskyum
force-pushed
the
solid-start-serialization-adapters-e2e-suite
branch
2 times, most recently
from
47e5543 to
2972c13
Compare
| nonce: router.options.ssr?.nonce, | ||
| }, | ||
| plugins: serovalPlugins, | ||
| } as any, |
There was a problem hiding this comment.
we probably won't need the as any once the plugins api actually supported?
|
|
||
| const docType = Solid.ssr('<!DOCTYPE html>') | ||
|
|
||
| const serializationAdapters = (router.options as any)?.serializationAdapters || (router.options.ssr as any)?.serializationAdapters |
birkskyum
force-pushed
the
solid-start-serialization-adapters-e2e-suite
branch
from
01d0cc8 to
d5f1bd4
Compare
This reverts commit 96dca8b.
This reverts commit fbfe5f6.
birkskyum
force-pushed
the
solid-start-serialization-adapters-e2e-suite
branch
from
e847154 to
14dd994
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
packages/solid-router/src/ssr/renderRouterToStream.tsx (2)
39-40: Address theas anycast with proper type definitions.This cast violates TypeScript strict mode guidelines. A past review comment indicated this might be resolved once the plugins API is properly supported.
Ensure the type definitions for
Solid.renderToStreamoptions include thepluginsproperty, or update the Solid.js dependency if needed to support this properly typed API.
24-28: Strengthen type safety and simplify mapping logic.Multiple
as anycasts violate the TypeScript strict mode guideline. The redundant variable assignment on lines 26-27 can also be simplified.Consider applying this approach to improve type safety:
- Add proper type definitions to
router.optionsforserializationAdaptersso the cast is unnecessary- Simplify the mapping logic
-const serializationAdapters = (router.options as any)?.serializationAdapters || (router.options.ssr as any)?.serializationAdapters -const serovalPlugins = serializationAdapters?.map((adapter: any) => { - const plugin = makeSsrSerovalPlugin(adapter, { didRun: false }) - return plugin -}) +const serializationAdapters = (router.options as any)?.serializationAdapters || (router.options.ssr as any)?.serializationAdapters +const serovalPlugins = serializationAdapters?.map((adapter: any) => + makeSsrSerovalPlugin(adapter, { didRun: false }) +)However, the real fix requires updating the type definitions for
AnyRouterto includeserializationAdaptersin the options type, eliminating the need for any casts. As per coding guidelines.
🧹 Nitpick comments (12)
e2e/solid-start/serialization-adapters/src/routes/__root.tsx (1)
11-11: Inconsistent semicolon usage.This line uses a semicolon while other lines in the file don't. For consistency, consider removing the semicolon.
-import { HydrationScript } from 'solid-js/web'; +import { HydrationScript } from 'solid-js/web'e2e/solid-start/serialization-adapters/postcss.config.mjs (1)
1-5: Ensure Tailwind plugin matches your Tailwind version'@tailwindcss/postcss' targets Tailwind v4. If this e2e app uses v3, switch to 'tailwindcss' + 'autoprefixer'.
Example v3 config:
export default { plugins: { - '@tailwindcss/postcss': {}, + tailwindcss: {}, + autoprefixer: {}, }, }e2e/solid-start/serialization-adapters/src/start.tsx (1)
5-16: Export default Start for entrypoint compatibilitySome tooling expects a default export from start.tsx. Safe to also export default.
export const startInstance = createStart(() => { return { defaultSsr: true, serializationAdapters: [ fooAdapter, carAdapter, customErrorAdapter, // only register nestedOuterAdapter here, nestedInnerAdapter is registered as an "extends" of nestedOuterAdapter nestedOuterAdapter, ], } }) + +export default startInstancee2e/solid-start/serialization-adapters/src/routes/ssr/nested.tsx (1)
4-15: Add errorComponent for parity and clearer failuresOptional, but keeps routes consistent and aids debugging.
export const Route = createFileRoute('/ssr/nested')({ beforeLoad: () => { return { nested: makeNested() } }, loader: ({ context }) => { return context }, + errorComponent: (e) => <div>{e.error.message}</div>, component: () => { const loaderData = Route.useLoaderData() return <RenderNestedData nested={loaderData().nested} /> }, })e2e/solid-start/serialization-adapters/src/routes/ssr/data-only.tsx (1)
50-50: Unify pending textMinor: change "posts Loading..." to a neutral "Loading..." to avoid confusion.
- pendingComponent: () => <div>posts Loading...</div>, + pendingComponent: () => <div>Loading...</div>,e2e/solid-start/serialization-adapters/playwright.config.ts (1)
3-3: Increase webServer timeout for CI stabilityThe JSON import with
with { type: 'json' }is now a stable feature (reached stage 4 in October 2024) and is fully supported in Node 24 and TypeScript 5.7. The tsconfig is correctly configured, so no fs-based fallback is needed.However, the webServer timeout should be increased to handle longer build times in CI environments:
webServer: { command: `VITE_SERVER_PORT=${PORT} pnpm build && NODE_ENV=production PORT=${PORT} VITE_SERVER_PORT=${PORT} pnpm start`, url: baseURL, reuseExistingServer: !process.env.CI, stdout: 'pipe', + timeout: 120_000, },e2e/solid-start/serialization-adapters/src/CustomError.ts (1)
7-15: Optional: capture V8 stack and freeze prototype early.Small ergonomics improvement for debugging; safe no-op in non-V8.
constructor(message: string, options: { foo: string; bar: bigint }) { super(message) - - Object.setPrototypeOf(this, new.target.prototype) + Object.setPrototypeOf(this, new.target.prototype) + if (typeof (Error as any).captureStackTrace === 'function') { + (Error as any).captureStackTrace(this, CustomError) + }e2e/solid-start/serialization-adapters/src/routes/server-function/custom-error.tsx (1)
16-16: Tighten types and use bigint literal.Avoid
anyand prefer123nfor clarity.- throw new CustomError('Invalid input', { foo: 'bar', bar: BigInt(123) }) + throw new CustomError('Invalid input', { foo: 'bar', bar: 123n })- const [validResponse, setValidResponse] = createSignal<any>(null) + const [validResponse, setValidResponse] = createSignal<string | null>(null)Also applies to: 24-24
e2e/solid-start/serialization-adapters/tests/app.spec.ts (2)
8-10: Use exact text match to avoid false positives.
toContainText('false'|'idle')can pass when extra text is present; prefertoHaveTextwith anchors.- await expect(page.getByTestId('router-isLoading')).toContainText('false') - await expect(page.getByTestId('router-status')).toContainText('idle') + await expect(page.getByTestId('router-isLoading')).toHaveText(/^\s*false\s*$/) + await expect(page.getByTestId('router-status')).toHaveText(/^\s*idle\s*$/)
84-90: Assert exact initial “null” state.Containment can mask regressions (e.g., “not null”); assert exact value.
- await expect( - page.getByTestId('server-function-valid-response'), - ).toContainText('null') - await expect( - page.getByTestId('server-function-invalid-response'), - ).toContainText('null') + await expect( + page.getByTestId('server-function-valid-response'), + ).toHaveText(/^\s*null\s*$/) + await expect( + page.getByTestId('server-function-invalid-response'), + ).toHaveText(/^\s*null\s*$/)e2e/solid-start/serialization-adapters/src/data.tsx (2)
29-34: Minor typing: preferunknownfortestinputs and add generics for stronger types.Improves strict-mode safety without behavior change.
-export const fooAdapter = createSerializationAdapter({ +export const fooAdapter = createSerializationAdapter<Foo, string>({ key: 'foo', - test: (value: any) => value instanceof Foo, + test: (value: unknown): value is Foo => value instanceof Foo, toSerializable: (foo) => foo.value, fromSerializable: (value) => new Foo(value), })-export const nestedInnerAdapter = createSerializationAdapter({ +export const nestedInnerAdapter = createSerializationAdapter<NestedInner, string>({ key: 'nestedInner', - test: (value): value is NestedInner => value instanceof NestedInner, + test: (value: unknown): value is NestedInner => value instanceof NestedInner, toSerializable: (inner) => inner.value, fromSerializable: (value) => new NestedInner(value), })-export const nestedOuterAdapter = createSerializationAdapter({ +export const nestedOuterAdapter = createSerializationAdapter<NestedOuter, NestedInner>({ key: 'nestedOuter', extends: [nestedInnerAdapter], - test: (value) => value instanceof NestedOuter, + test: (value: unknown): value is NestedOuter => value instanceof NestedOuter, toSerializable: (outer) => outer.inner, fromSerializable: (value) => new NestedOuter(value), })Also applies to: 105-118
161-205: Remove redundant block in component body.The extra
{ ... }block is unnecessary; simplify for readability.-export function RenderNestedData({ nested }: { nested: NestedOuter }) { - { +export function RenderNestedData({ nested }: { nested: NestedOuter }) { const localData = makeNested() const expectedShoutState = localData.inner.shout() const expectedWhisperState = localData.whisper() const shoutState = nested.inner.shout() const whisperState = nested.whisper() return ( <div data-testid="data-only-container"> <h2 data-testid="data-only-heading">data-only</h2> <div data-testid="shout-container"> <h3>shout</h3> <div> expected:{' '} <div data-testid="shout-expected-state"> {JSON.stringify(expectedShoutState)} </div> </div> <div> actual:{' '} <div data-testid="shout-actual-state"> {JSON.stringify(shoutState)} </div> </div> </div> <div data-testid="whisper-container"> <h3>whisper</h3> <div> expected:{' '} <div data-testid="whisper-expected-state"> {JSON.stringify(expectedWhisperState)} </div> </div> <div> actual:{' '} <div data-testid="whisper-actual-state"> {JSON.stringify(whisperState)} </div> </div> </div> </div> ) - } }
📜 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 (23)
e2e/solid-start/serialization-adapters/.gitignore(1 hunks)e2e/solid-start/serialization-adapters/.prettierignore(1 hunks)e2e/solid-start/serialization-adapters/package.json(1 hunks)e2e/solid-start/serialization-adapters/playwright.config.ts(1 hunks)e2e/solid-start/serialization-adapters/postcss.config.mjs(1 hunks)e2e/solid-start/serialization-adapters/src/CustomError.ts(1 hunks)e2e/solid-start/serialization-adapters/src/data.tsx(1 hunks)e2e/solid-start/serialization-adapters/src/routeTree.gen.ts(1 hunks)e2e/solid-start/serialization-adapters/src/router.tsx(1 hunks)e2e/solid-start/serialization-adapters/src/routes/__root.tsx(1 hunks)e2e/solid-start/serialization-adapters/src/routes/index.tsx(1 hunks)e2e/solid-start/serialization-adapters/src/routes/server-function/custom-error.tsx(1 hunks)e2e/solid-start/serialization-adapters/src/routes/server-function/nested.tsx(1 hunks)e2e/solid-start/serialization-adapters/src/routes/ssr/data-only.tsx(1 hunks)e2e/solid-start/serialization-adapters/src/routes/ssr/nested.tsx(1 hunks)e2e/solid-start/serialization-adapters/src/routes/ssr/stream.tsx(1 hunks)e2e/solid-start/serialization-adapters/src/start.tsx(1 hunks)e2e/solid-start/serialization-adapters/src/styles/app.css(1 hunks)e2e/solid-start/serialization-adapters/tests/app.spec.ts(1 hunks)e2e/solid-start/serialization-adapters/tsconfig.json(1 hunks)e2e/solid-start/serialization-adapters/vite.config.ts(1 hunks)packages/solid-router/src/awaited.tsx(1 hunks)packages/solid-router/src/ssr/renderRouterToStream.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/solid-router/src/ssr/renderRouterToStream.tsxe2e/solid-start/serialization-adapters/src/router.tsxe2e/solid-start/serialization-adapters/src/routes/ssr/nested.tsxe2e/solid-start/serialization-adapters/tests/app.spec.tse2e/solid-start/serialization-adapters/src/routes/index.tsxe2e/solid-start/serialization-adapters/src/CustomError.tse2e/solid-start/serialization-adapters/src/routes/__root.tsxe2e/solid-start/serialization-adapters/src/routeTree.gen.tspackages/solid-router/src/awaited.tsxe2e/solid-start/serialization-adapters/src/routes/ssr/data-only.tsxe2e/solid-start/serialization-adapters/playwright.config.tse2e/solid-start/serialization-adapters/src/start.tsxe2e/solid-start/serialization-adapters/src/data.tsxe2e/solid-start/serialization-adapters/vite.config.tse2e/solid-start/serialization-adapters/src/routes/server-function/custom-error.tsxe2e/solid-start/serialization-adapters/src/routes/ssr/stream.tsxe2e/solid-start/serialization-adapters/src/routes/server-function/nested.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/renderRouterToStream.tsxpackages/solid-router/src/awaited.tsx
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/solid-start/serialization-adapters/src/router.tsxe2e/solid-start/serialization-adapters/postcss.config.mjse2e/solid-start/serialization-adapters/src/routes/ssr/nested.tsxe2e/solid-start/serialization-adapters/tests/app.spec.tse2e/solid-start/serialization-adapters/src/routes/index.tsxe2e/solid-start/serialization-adapters/src/CustomError.tse2e/solid-start/serialization-adapters/src/routes/__root.tsxe2e/solid-start/serialization-adapters/src/routeTree.gen.tse2e/solid-start/serialization-adapters/src/styles/app.csse2e/solid-start/serialization-adapters/tsconfig.jsone2e/solid-start/serialization-adapters/src/routes/ssr/data-only.tsxe2e/solid-start/serialization-adapters/playwright.config.tse2e/solid-start/serialization-adapters/src/start.tsxe2e/solid-start/serialization-adapters/src/data.tsxe2e/solid-start/serialization-adapters/vite.config.tse2e/solid-start/serialization-adapters/src/routes/server-function/custom-error.tsxe2e/solid-start/serialization-adapters/package.jsone2e/solid-start/serialization-adapters/src/routes/ssr/stream.tsxe2e/solid-start/serialization-adapters/src/routes/server-function/nested.tsx
**/src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Place file-based routes under src/routes/ directories
Files:
e2e/solid-start/serialization-adapters/src/routes/ssr/nested.tsxe2e/solid-start/serialization-adapters/src/routes/index.tsxe2e/solid-start/serialization-adapters/src/routes/__root.tsxe2e/solid-start/serialization-adapters/src/routes/ssr/data-only.tsxe2e/solid-start/serialization-adapters/src/routes/server-function/custom-error.tsxe2e/solid-start/serialization-adapters/src/routes/ssr/stream.tsxe2e/solid-start/serialization-adapters/src/routes/server-function/nested.tsx
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use workspace:* protocol for internal dependencies in package.json files
Files:
e2e/solid-start/serialization-adapters/package.json
🧠 Learnings (3)
📚 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-start/serialization-adapters/src/routeTree.gen.ts
📚 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-start/serialization-adapters/src/routeTree.gen.ts
📚 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-start/serialization-adapters/tsconfig.json
🧬 Code graph analysis (9)
e2e/solid-start/serialization-adapters/src/routes/ssr/nested.tsx (1)
e2e/solid-start/serialization-adapters/src/data.tsx (2)
makeNested(120-122)RenderNestedData(161-205)
e2e/solid-start/serialization-adapters/src/routes/index.tsx (6)
e2e/solid-start/serialization-adapters/src/routes/__root.tsx (1)
Route(15-33)e2e/solid-start/serialization-adapters/src/routes/ssr/stream.tsx (1)
Route(5-18)e2e/solid-start/serialization-adapters/src/routes/ssr/nested.tsx (1)
Route(4-15)e2e/solid-start/serialization-adapters/src/routes/server-function/custom-error.tsx (1)
Route(19-21)e2e/solid-start/serialization-adapters/src/routes/server-function/nested.tsx (1)
Route(11-13)e2e/solid-start/serialization-adapters/src/routes/ssr/data-only.tsx (1)
Route(4-51)
e2e/solid-start/serialization-adapters/src/routeTree.gen.ts (2)
e2e/solid-start/serialization-adapters/src/router.tsx (1)
getRouter(4-11)e2e/solid-start/serialization-adapters/src/start.tsx (1)
startInstance(5-16)
e2e/solid-start/serialization-adapters/src/routes/ssr/data-only.tsx (1)
e2e/solid-start/serialization-adapters/src/data.tsx (2)
makeData(49-90)RenderData(124-159)
e2e/solid-start/serialization-adapters/playwright.config.ts (1)
scripts/set-ts-version.js (1)
packageJson(33-33)
e2e/solid-start/serialization-adapters/src/start.tsx (2)
e2e/solid-start/serialization-adapters/src/data.tsx (3)
fooAdapter(29-34)carAdapter(36-47)nestedOuterAdapter(112-118)e2e/solid-start/serialization-adapters/src/CustomError.ts (1)
customErrorAdapter(18-31)
e2e/solid-start/serialization-adapters/src/routes/server-function/custom-error.tsx (2)
packages/start-server-core/src/request-response.ts (1)
setResponseStatus(196-204)e2e/solid-start/serialization-adapters/src/CustomError.ts (1)
CustomError(3-16)
e2e/solid-start/serialization-adapters/src/routes/ssr/stream.tsx (1)
e2e/solid-start/serialization-adapters/src/data.tsx (2)
makeData(49-90)RenderData(124-159)
e2e/solid-start/serialization-adapters/src/routes/server-function/nested.tsx (1)
e2e/solid-start/serialization-adapters/src/data.tsx (3)
makeNested(120-122)NestedOuter(91-96)RenderNestedData(161-205)
⏰ 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 (3)
e2e/solid-start/serialization-adapters/src/routes/index.tsx (1)
3-6: LGTM for E2E entry routeLinks and errorComponent are appropriate for SSR-focused tests.
e2e/solid-start/serialization-adapters/src/routes/ssr/stream.tsx (1)
5-18: LGTM for streaming SSRLoader shape, Suspense/Await usage, and error handling look correct.
Also applies to: 20-33
e2e/solid-start/serialization-adapters/src/routeTree.gen.ts (1)
1-10: Autogenerated route tree — skipping review.This file is generated and should not be edited or reviewed. Based on learnings.
| /build/ | ||
| /api/ | ||
| /server/build | ||
| /public/build# Sentry Config File |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Fix formatting: separate pattern and comment.
The pattern /public/build and the comment # Sentry Config File are on the same line. This makes the comment ineffective and could be confusing.
-/public/build# Sentry Config File
+/public/build
+# Sentry Config File📝 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.
| /public/build# Sentry Config File | |
| /public/build | |
| # Sentry Config File |
🤖 Prompt for AI Agents
In e2e/solid-start/serialization-adapters/.gitignore around line 13, the
gitignore entry combines the pattern "/public/build" and the comment "# Sentry
Config File" on the same line; split them so the ignore pattern stands alone on
its own line and the explanatory comment is on a separate line (e.g., move the
comment to the line above or below the pattern).
| "dev": "vite dev --port 3000", | ||
| "dev:e2e": "vite dev", | ||
| "build": "vite build && tsc --noEmit", | ||
| "start": "pnpx srvx --prod -s ../client dist/server/server.js", |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the client directory path in the start script.
The path -s ../client appears to reference a directory outside the current e2e test package. For an e2e test suite, the client build artifacts should typically be within the package's own build directory (e.g., dist/client or similar).
Run the following script to check if the referenced path exists and verify the intended directory structure:
🏁 Script executed:
#!/bin/bash
# Description: Verify client directory structure and start script paths
# Check if ../client exists relative to this package
if [ -d "e2e/solid-start/serialization-adapters/../client" ]; then
echo "Found: e2e/solid-start/serialization-adapters/../client"
ls -la e2e/solid-start/serialization-adapters/../client
else
echo "Not found: e2e/solid-start/serialization-adapters/../client"
fi
# Look for client build directories within this package
fd -t d "client" e2e/solid-start/serialization-adapters/
# Check for dist or build directories that might contain client artifacts
fd -t d "^(dist|build)$" e2e/solid-start/serialization-adapters/ --max-depth 2Length of output: 59
🏁 Script executed:
#!/bin/bash
# Description: Understand e2e/solid-start project structure and client reference
# List all directories in e2e/solid-start
echo "=== Directory structure of e2e/solid-start ==="
find e2e/solid-start -type d -not -path '*/node_modules/*' | head -20
# Find all package.json files to understand packages in e2e/solid-start
echo -e "\n=== Package.json files in e2e/solid-start ==="
fd -e "package.json" e2e/solid-start/ --max-depth 3
# Look for any 'client' directories anywhere in e2e or root
echo -e "\n=== Any 'client' directories in e2e/solid-start ==="
fd -t d "client" e2e/solid-start/ --max-depth 5
# Check if there's a client at root level
echo -e "\n=== Check root level directories ==="
ls -d */ 2>/dev/null | grep -E "(client|packages|apps)" || echo "No obvious client/packages/apps at root"
# Look at what the start script reference file contains and its context
echo -e "\n=== Full content of the package.json under review ==="
cat e2e/solid-start/serialization-adapters/package.jsonLength of output: 2402
🏁 Script executed:
#!/bin/bash
# Description: Find all package.json, check build outputs, and understand project structure
# Find all package.json files properly
echo "=== All package.json files in e2e/solid-start ==="
fd --full-path "package.json" e2e/solid-start/
# Check if there's a build/dist directory with client artifacts
echo -e "\n=== Check for dist directories in serialization-adapters ==="
ls -la e2e/solid-start/serialization-adapters/ 2>/dev/null | grep -E "^d" || echo "No directories listed"
# Look for what exists at the parent level (e2e/solid-start)
echo -e "\n=== Contents of e2e/solid-start parent level ==="
ls -la e2e/solid-start/ | grep -E "^d" | head -20
# Check if there's a client directory elsewhere (e.g., in packages)
echo -e "\n=== Look for 'client' in packages/ ==="
find packages/ -type d -name "client" 2>/dev/null || echo "No client directories in packages/"
# Check other e2e packages' start scripts for comparison
echo -e "\n=== Start scripts in other e2e packages ==="
rg -A 1 '"start"' e2e/solid-start/*/package.json
# Check the build output structure by looking at vite.config or tsconfig
echo -e "\n=== Check vite.config in serialization-adapters ==="
ls -la e2e/solid-start/serialization-adapters/ | grep -i vite || echo "No vite.config found in listing"
fd "vite.config" e2e/solid-start/serialization-adapters/Length of output: 5693
🏁 Script executed:
#!/bin/bash
# Description: Understand vite build output and srvx -s flag behavior
# Check vite.config.ts in serialization-adapters
echo "=== vite.config.ts in serialization-adapters ==="
cat e2e/solid-start/serialization-adapters/vite.config.ts
# Check if there's any build setup or shared client directory mentioned
echo -e "\n=== Check root vite.config or build config ==="
fd "vite.config" . --max-depth 1
fd "build" . --max-depth 1 -t f
# Look at package.json root for any build/setup scripts
echo -e "\n=== Root package.json scripts ==="
rg '"scripts"' -A 30 package.json | head -40
# Search for documentation or comments about srvx or ../client pattern
echo -e "\n=== Search for srvx or client path references ==="
rg "srvx" --type json --type ts --type js -B 2 -A 2 e2e/solid-start/basic/
# Check the vite build output by looking at what other e2e packages have
echo -e "\n=== Check dist structure in e2e/solid-start/basic after potential build ==="
ls -la e2e/solid-start/basic/dist 2>/dev/null || echo "No dist directory in basic"
# Look for srvx documentation or usage in the repo
echo -e "\n=== Search for srvx in entire repo ==="
rg "srvx" -t json | head -10Length of output: 4497
🌐 Web query:
srvx npm package -s flag documentation production server
💡 Result:
The -s shorthand is the CLI's "silent" option (same as --silent) — it suppresses the server listening message/console output. See the Server Options docs (silent option). [1][2]
Sources:
- srvx Server Options — "silent: If enabled, no server listening message will be printed." [1]
- srvx home / CLI quickstart (shows npx srvx usage). [2]
| toSerializable: ({ message, foo, bar }) => { | ||
| return { | ||
| message, | ||
| foo, | ||
| bar, | ||
| } | ||
| }, | ||
| fromSerializable: ({ message, foo, bar }) => { | ||
| return new CustomError(message, { foo, bar }) | ||
| }, | ||
| }) |
There was a problem hiding this comment.
BigInt is not JSON-serializable; stringify in toSerializable and rehydrate in fromSerializable.
Avoid runtime TypeError during transport by serializing bar as a string and parsing back to bigint.
-export const customErrorAdapter = createSerializationAdapter({
+export const customErrorAdapter = createSerializationAdapter<
+ CustomError,
+ { message: string; foo: string; bar: string }
+>({
key: 'custom-error',
- test: (v) => v instanceof CustomError,
- toSerializable: ({ message, foo, bar }) => {
- return {
- message,
- foo,
- bar,
- }
- },
- fromSerializable: ({ message, foo, bar }) => {
- return new CustomError(message, { foo, bar })
- },
+ test: (v: unknown): v is CustomError => v instanceof CustomError,
+ toSerializable: ({ message, foo, bar }) => ({
+ message,
+ foo,
+ bar: bar.toString(),
+ }),
+ fromSerializable: ({ message, foo, bar }) =>
+ new CustomError(message, { foo, bar: BigInt(bar) }),
})Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In e2e/solid-start/serialization-adapters/src/CustomError.ts around lines 21 to
31, the current serializer returns bar as a bigint which will cause
JSON.stringify to throw; modify toSerializable to convert bar to a string (e.g.
String(bar) or bar?.toString()) and modify fromSerializable to rehydrate by
calling BigInt(parsedBar) (handling null/undefined/empty values appropriately)
so bar is safely transported as a string and restored back to a bigint when
reconstructing the CustomError.
| key: 'car', | ||
| test: (value: any): value is Car => | ||
| '__type' in (value as Car) && value.__type === 'car', |
There was a problem hiding this comment.
Guard against null/primitive in adapter test to avoid runtime TypeError.
'__type' in value throws for null/undefined. Add typeof/null checks.
export const carAdapter = createSerializationAdapter({
key: 'car',
- test: (value: any): value is Car =>
- '__type' in (value as Car) && value.__type === 'car',
+ test: (value: unknown): value is Car =>
+ typeof value === 'object' &&
+ value !== null &&
+ '__type' in (value as Record<string, unknown>) &&
+ (value as any).__type === 'car',🤖 Prompt for AI Agents
In e2e/solid-start/serialization-adapters/src/data.tsx around lines 37 to 39,
the adapter test uses "'__type' in (value as Car)" which will throw for
null/undefined or primitives; change the guard to first ensure value is a
non-null object (e.g., value !== null && typeof value === 'object') before
checking the '__type' property and comparing it to 'car' so the type guard
safely returns false for null/primitive inputs.
| function RouteComponent() { | ||
| const [nestedResponse, setNestedResponse] = createSignal<NestedOuter>() | ||
|
|
||
| return ( | ||
| <div> | ||
| <button | ||
| data-testid="server-function-trigger" | ||
| onClick={() => serverFnReturningNested().then(setNestedResponse)} | ||
| > | ||
| trigger | ||
| </button> | ||
|
|
||
| {nestedResponse() ? ( | ||
| <RenderNestedData nested={nestedResponse()!} /> | ||
| ) : ( | ||
| <div data-testid="waiting-for-response">waiting for response...</div> | ||
| )} | ||
| </div> | ||
| ) |
There was a problem hiding this comment.
Fix signal typing for strict mode and handle server fn errors
Use NestedOuter | undefined and guard against rejections.
function RouteComponent() {
- const [nestedResponse, setNestedResponse] = createSignal<NestedOuter>()
+ const [nestedResponse, setNestedResponse] =
+ createSignal<NestedOuter | undefined>(undefined)
return (
<div>
<button
data-testid="server-function-trigger"
- onClick={() => serverFnReturningNested().then(setNestedResponse)}
+ onClick={async () => {
+ try {
+ const data = await serverFnReturningNested()
+ setNestedResponse(data)
+ } catch (err) {
+ console.error('serverFnReturningNested failed', err)
+ }
+ }}
>
trigger
</button>📝 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.
| function RouteComponent() { | |
| const [nestedResponse, setNestedResponse] = createSignal<NestedOuter>() | |
| return ( | |
| <div> | |
| <button | |
| data-testid="server-function-trigger" | |
| onClick={() => serverFnReturningNested().then(setNestedResponse)} | |
| > | |
| trigger | |
| </button> | |
| {nestedResponse() ? ( | |
| <RenderNestedData nested={nestedResponse()!} /> | |
| ) : ( | |
| <div data-testid="waiting-for-response">waiting for response...</div> | |
| )} | |
| </div> | |
| ) | |
| function RouteComponent() { | |
| const [nestedResponse, setNestedResponse] = | |
| createSignal<NestedOuter | undefined>(undefined) | |
| return ( | |
| <div> | |
| <button | |
| data-testid="server-function-trigger" | |
| onClick={async () => { | |
| try { | |
| const data = await serverFnReturningNested() | |
| setNestedResponse(data) | |
| } catch (err) { | |
| console.error('serverFnReturningNested failed', err) | |
| } | |
| }} | |
| > | |
| trigger | |
| </button> | |
| {nestedResponse() ? ( | |
| <RenderNestedData nested={nestedResponse()!} /> | |
| ) : ( | |
| <div data-testid="waiting-for-response">waiting for response...</div> | |
| )} | |
| </div> | |
| ) |
🤖 Prompt for AI Agents
In e2e/solid-start/serialization-adapters/src/routes/server-function/nested.tsx
around lines 15 to 33, the signal is currently typed as NestedOuter which breaks
strict mode and the server call isn't handling rejections; change the signal to
createSignal<NestedOuter | undefined>() (or use undefined union) and remove the
non-null assertion when passing to RenderNestedData, and update the onClick
handler to handle promise rejections by adding a .catch(err => { /* set error
state or setNestedResponse(undefined) and log the error */ }) so the UI is
guarded against failed serverFnReturningNested calls.
| const [resource] = Solid.createResource( | ||
| () => defer(props.promise), | ||
| // Simple passthrough - just return the promise for Solid to await | ||
| (p) => p, | ||
| { | ||
| deferStream: true, | ||
| }, | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the deferred promise handling pattern.
The identity mapper (p) => p returns the deferred promise itself rather than awaiting it. Ensure this pattern works correctly with Solid.createResource and the deferStream: true option, and that the data() call on line 44 properly unwraps the deferred promise value.
The comment "just return the promise for Solid to await" may be misleading since p is already a deferred promise from defer(props.promise). Consider clarifying whether Solid's resource system is designed to handle deferred promises directly or if additional unwrapping is needed.
Run the following script to verify how deferred promises are used elsewhere in the codebase:
🏁 Script executed:
#!/bin/bash
# Description: Search for similar patterns of createResource with defer and deferStream
# Search for other uses of deferStream option
rg -n -C3 'deferStream.*true'
# Search for other uses of defer with createResource
rg -n -C3 'createResource.*defer'
# Search for documentation or tests explaining this pattern
fd -e md -e test.ts -e test.tsx | xargs rg -l 'deferStream|defer.*promise' | head -10Length of output: 643
I need to examine the full context of the function and understand Solid.js semantics. Let me gather more information:
I need to examine how defer() is used in the codebase and understand the full context of this implementation:
Now I need to examine the actual file to understand the context and find where defer() is imported from:Perfect! Now I have the critical information. Let me verify the implementation by examining the file context directly:
[/run_scripts]
Based on my verification, I can now provide the final review comment:
Clarify the identity mapper's purpose with deferred promises in Solid resources.
The defer() function wraps a promise with a deferred state object that can be used to inspect the promise's state. However, the identity mapper (p) => p is unusual for a resource fetcher. When using deferStream: true, Solid waits for a resource before flushing the stream, but the typical pattern has the fetcher receive the source value and return a promise.
The comment "just return the promise for Solid to await" is misleading because p is already a deferred promise object from defer(props.promise), not a raw promise. The identity mapper passes this deferred object directly to Solid's resource system. Verify that:
- Solid's
createResourcecorrectly handles deferred promise objects as the resolved resource value - The
data()call on line 44 properly unwraps the deferred promise value - The implementation is intentionally different from standard
createResourceusage patterns documented in Solid's examples
If the deferred promise needs unwrapping before being passed to the resource, consider adjusting the fetcher to extract the underlying promise or applying any necessary transformation.
🏁 Script executed:
#!/bin/bash
# View the awaited.tsx file around lines 30-50 to see complete context
cat packages/solid-router/src/awaited.tsx | head -60 | tail -40Length of output: 877
Fix the resource fetcher to extract the resolved data from the deferred promise.
The current implementation has an inconsistency: the useAwaited hook correctly extracts the data via promise[TSR_DEFERRED_PROMISE].data, but the Await component's fetcher (p) => p returns the deferred promise object unchanged. Since children expects type T (not the deferred promise object), the fetcher should extract the data:
(p) => p[TSR_DEFERRED_PROMISE].dataAlternatively, if Solid's deferStream: true automatically unwraps deferred promises, clarify this assumption with a comment explaining the expected behavior. The current comment "just return the promise for Solid to await" is misleading—p is already a deferred promise object with wrapped state, not a raw promise.
🤖 Prompt for AI Agents
In packages/solid-router/src/awaited.tsx around lines 33 to 40, the resource
fetcher currently returns the deferred promise object (p) unchanged which
doesn't match the children' expected type T; change the fetcher to extract the
resolved data from the deferred promise (i.e. return
p[TSR_DEFERRED_PROMISE].data) so the resource yields the plain data, or if
relying on Solid's deferStream to unwrap deferred objects, replace the
misleading comment with a clear note stating that deferStream auto-unwraps and
leave the fetcher as-is — but do not keep the current comment claiming p is a
raw promise.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
e2e/solid-start/serialization-adapters/src/routes/index.tsx (1)
13-35: Verify ifreloadDocument={true}is necessary for all SSR links.All links in the SSR section force full page reloads with
reloadDocument={true}. This bypasses client-side navigation and may not be necessary for testing serialization adapters. Consider whether client-side navigation would better test the serialization behavior across the client-server boundary.e2e/solid-start/serialization-adapters/src/routes/server-function/custom-error.tsx (3)
24-24: Improve type safety by replacinganywith a specific type.Using
anyreduces type safety. Consider using a more specific type likestringor defining a response type.Apply this diff:
- const [validResponse, setValidResponse] = createSignal<any>(null) + const [validResponse, setValidResponse] = createSignal<string | null>(null)
30-40: Add error handling for the valid input promise.The promise chain on lines 32-33 lacks error handling. If the server function throws an unexpected error during valid input processing, it will result in an unhandled promise rejection.
Apply this diff:
<button data-testid="server-function-valid-input" onClick={() => - serverFnThrowing({ data: { hello: 'world' } }).then(setValidResponse) + serverFnThrowing({ data: { hello: 'world' } }) + .then(setValidResponse) + .catch((err) => console.error('Unexpected error:', err)) } >
57-65: Remove redundant optional chaining.Lines 60-62 use optional chaining (
invalidResponse()?.) even though line 58 already confirmsinvalidResponse()is truthy. The optional chaining is redundant and can be removed.Apply this diff:
<div data-testid="server-function-invalid-response"> {invalidResponse() ? JSON.stringify({ - message: invalidResponse()?.message, - foo: invalidResponse()?.foo, - bar: invalidResponse()?.bar.toString(), + message: invalidResponse()!.message, + foo: invalidResponse()!.foo, + bar: invalidResponse()!.bar.toString(), }) : JSON.stringify(invalidResponse())} </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
e2e/solid-start/serialization-adapters/src/routes/__root.tsx(1 hunks)e2e/solid-start/serialization-adapters/src/routes/index.tsx(1 hunks)e2e/solid-start/serialization-adapters/src/routes/server-function/custom-error.tsx(1 hunks)e2e/solid-start/serialization-adapters/src/routes/ssr/stream.tsx(1 hunks)packages/solid-router/src/ssr/renderRouterToStream.tsx(3 hunks)packages/solid-router/src/ssr/renderRouterToString.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/solid-router/src/ssr/renderRouterToStream.tsx
- e2e/solid-start/serialization-adapters/src/routes/ssr/stream.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/serialization-adapters/src/routes/__root.tsxe2e/solid-start/serialization-adapters/src/routes/server-function/custom-error.tsxpackages/solid-router/src/ssr/renderRouterToString.tsxe2e/solid-start/serialization-adapters/src/routes/index.tsx
**/src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Place file-based routes under src/routes/ directories
Files:
e2e/solid-start/serialization-adapters/src/routes/__root.tsxe2e/solid-start/serialization-adapters/src/routes/server-function/custom-error.tsxe2e/solid-start/serialization-adapters/src/routes/index.tsx
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/solid-start/serialization-adapters/src/routes/__root.tsxe2e/solid-start/serialization-adapters/src/routes/server-function/custom-error.tsxe2e/solid-start/serialization-adapters/src/routes/index.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 (3)
e2e/solid-start/serialization-adapters/src/routes/__root.tsx (2)
e2e/solid-start/serialization-adapters/src/routes/index.tsx (1)
Route(3-6)packages/router-core/src/router.ts (1)
state(1082-1084)
e2e/solid-start/serialization-adapters/src/routes/server-function/custom-error.tsx (2)
packages/start-server-core/src/request-response.ts (1)
setResponseStatus(196-204)e2e/solid-start/serialization-adapters/src/CustomError.ts (1)
CustomError(3-16)
e2e/solid-start/serialization-adapters/src/routes/index.tsx (6)
e2e/solid-start/serialization-adapters/src/routes/__root.tsx (1)
Route(15-33)e2e/solid-start/serialization-adapters/src/routes/server-function/custom-error.tsx (1)
Route(19-21)e2e/solid-start/serialization-adapters/src/routes/ssr/stream.tsx (1)
Route(5-18)e2e/solid-start/serialization-adapters/src/routes/ssr/nested.tsx (1)
Route(4-15)e2e/solid-start/serialization-adapters/src/routes/server-function/nested.tsx (1)
Route(11-13)e2e/solid-start/serialization-adapters/src/routes/ssr/data-only.tsx (1)
Route(4-51)
⏰ 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 (7)
e2e/solid-start/serialization-adapters/src/routes/index.tsx (1)
1-6: LGTM!The route definition follows the established pattern with proper imports, component binding, and error handling.
e2e/solid-start/serialization-adapters/src/routes/server-function/custom-error.tsx (3)
1-7: LGTM!All necessary imports are present and properly organized.
8-17: Verify the use of HTTP status code 499.Status code 499 is a non-standard nginx-specific code typically used for "Client Closed Request." If this is intentional for testing custom status codes, consider adding a comment explaining the choice. Otherwise, consider using a standard 4xx status code like 400 (Bad Request) or 422 (Unprocessable Entity).
19-21: LGTM!Route export follows the established pattern.
e2e/solid-start/serialization-adapters/src/routes/__root.tsx (2)
1-13: LGTM!The imports are well-organized and appropriate for a Solid Start root route with SSR hydration, devtools, and router functionality.
15-33: LGTM!The root route configuration is well-structured with appropriate head metadata, shell component, and a debugging-friendly 404 handler that serializes error data for e2e testing.
packages/solid-router/src/ssr/renderRouterToString.tsx (1)
2-4: LGTM: Import changes are appropriate.The new import for
makeSsrSerovalPluginsupports the serialization functionality, and using a type-only import forJSXElementis good practice.
| const { isLoading, status } = useRouterState({ | ||
| select: (state) => ({ isLoading: state.isLoading, status: state.status }), | ||
| })() |
There was a problem hiding this comment.
Router state won't update reactively—breaks dynamic e2e testing.
The useRouterState hook is called and immediately invoked with (), then destructured. This extracts static values rather than reactive accessors. In SolidJS, reactive values must be accessed within reactive contexts (JSX, effects, memos) to maintain reactivity.
Current implementation shows only the initial state snapshot. Since these values have data-testid attributes in a ClientOnly wrapper, they're clearly meant to test dynamic state changes during navigation. Without reactivity, e2e tests cannot verify loading state transitions.
Apply this diff to restore reactivity:
- const { isLoading, status } = useRouterState({
+ const routerState = useRouterState({
select: (state) => ({ isLoading: state.isLoading, status: state.status }),
- })()
+ })
return (
<html>
<head>
<HydrationScript />
</head>
<body>
<HeadContent />
<div class="p-2 flex gap-2 text-lg">
<h1>Serialization Adapters E2E Test</h1>
<Link
to="/"
activeProps={{
class: 'font-bold',
}}
>
Home
</Link>
</div>
<hr />
<ClientOnly>
<div>
router isLoading:{' '}
- <b data-testid="router-isLoading">{isLoading ? 'true' : 'false'}</b>
+ <b data-testid="router-isLoading">{routerState().isLoading ? 'true' : 'false'}</b>
</div>
<div>
- router status: <b data-testid="router-status">{status}</b>
+ router status: <b data-testid="router-status">{routerState().status}</b>
</div>
</ClientOnly>Also applies to: 60-61, 64-64
🤖 Prompt for AI Agents
In e2e/solid-start/serialization-adapters/src/routes/__root.tsx around lines
36-38 (and similarly at 60-61, 64), the code calls useRouterState(... )() which
immediately invokes the returned object and extracts non-reactive snapshots;
replace the immediate invocation so you keep the reactive accessors: call const
{ isLoading, status } = useRouterState({ select: s => ({ isLoading: s.isLoading,
status: s.status }) }); then update any JSX or tests to call isLoading() and
status() (or otherwise use them inside a reactive context) so the values update
reactively during navigation for e2e tests.
| <Link | ||
| data-testid="server-function-custom-error-link" | ||
| to="/server-function/custom-error" | ||
| reloadDocument={true} | ||
| > | ||
| Custom Error Serialization | ||
| </Link> | ||
| <br /> | ||
| <Link | ||
| data-testid="server-functions-nested-link" | ||
| to="/server-function/nested" | ||
| reloadDocument={true} | ||
| > | ||
| Nested Classes returned from Server Function | ||
| </Link> |
There was a problem hiding this comment.
Fix data-testid naming inconsistency.
Line 48 uses "server-functions-nested-link" (plural) while Line 40 uses "server-function-custom-error-link" (singular). This inconsistency may cause test failures or confusion.
Apply this diff to standardize the naming:
<Link
- data-testid="server-functions-nested-link"
+ data-testid="server-function-nested-link"
to="/server-function/nested"
reloadDocument={true}
>📝 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.
| <Link | |
| data-testid="server-function-custom-error-link" | |
| to="/server-function/custom-error" | |
| reloadDocument={true} | |
| > | |
| Custom Error Serialization | |
| </Link> | |
| <br /> | |
| <Link | |
| data-testid="server-functions-nested-link" | |
| to="/server-function/nested" | |
| reloadDocument={true} | |
| > | |
| Nested Classes returned from Server Function | |
| </Link> | |
| <Link | |
| data-testid="server-function-custom-error-link" | |
| to="/server-function/custom-error" | |
| reloadDocument={true} | |
| > | |
| Custom Error Serialization | |
| </Link> | |
| <br /> | |
| <Link | |
| data-testid="server-function-nested-link" | |
| to="/server-function/nested" | |
| reloadDocument={true} | |
| > | |
| Nested Classes returned from Server Function | |
| </Link> |
🤖 Prompt for AI Agents
In e2e/solid-start/serialization-adapters/src/routes/index.tsx around lines 39
to 53, the data-testid for the nested server function link is inconsistent: it
uses "server-functions-nested-link" (plural) while the custom error link uses
"server-function-custom-error-link" (singular). Update the nested link's
data-testid to "server-function-nested-link" to match the singular convention,
ensuring both test IDs follow the same "server-function-<name>-link" pattern so
tests remain consistent.
| const serializationAdapters = | ||
| (router.options as any)?.serializationAdapters || | ||
| (router.options.ssr as any)?.serializationAdapters | ||
| const serovalPlugins = serializationAdapters?.map((adapter: any) => { | ||
| const plugin = makeSsrSerovalPlugin(adapter, { didRun: false }) | ||
| return plugin | ||
| }) |
There was a problem hiding this comment.
Improve type safety and simplify the plugin mapping.
This code has several issues that violate the coding guideline to "Use TypeScript in strict mode with extensive type safety across the codebase":
- Multiple
anycasts (lines 17, 18, 19) suppress type checking - Unnecessary intermediate variable on lines 20-21
Consider applying these improvements:
- const serializationAdapters =
- (router.options as any)?.serializationAdapters ||
- (router.options.ssr as any)?.serializationAdapters
- const serovalPlugins = serializationAdapters?.map((adapter: any) => {
- const plugin = makeSsrSerovalPlugin(adapter, { didRun: false })
- return plugin
- })
+ const serializationAdapters =
+ router.options.serializationAdapters ||
+ router.options.ssr?.serializationAdapters
+ const serovalPlugins = serializationAdapters?.map((adapter) =>
+ makeSsrSerovalPlugin(adapter, { didRun: false })
+ )If TypeScript errors occur, add proper type definitions to the router options instead of suppressing them with any casts.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/solid-router/src/ssr/renderRouterToString.tsx around lines 16 to 22,
remove the three "any" casts and the unnecessary intermediate variable: instead
declare a properly typed interface for router.options (or extend the router
type) that exposes serializationAdapters and ssr.serializationAdapters, then
inline the mapping to produce serovalPlugins in one expression that safely
handles undefined (e.g. use nullish coalescing or optional chaining) and passes
the correctly typed adapter into makeSsrSerovalPlugin with the { didRun: false }
option; if TypeScript errors arise, add the minimal type definitions for
serializationAdapters and adapter items rather than using any.
| let html = Solid.renderToString(children, { | ||
| nonce: router.options.ssr?.nonce, | ||
| }) | ||
| plugins: serovalPlugins, | ||
| } as any) |
There was a problem hiding this comment.
🧩 Analysis chain
Remove as any cast and verify Solid's renderToString types.
The as any cast on line 27 suppresses TypeScript's type checking, violating the strict type safety guideline. If the plugins property is not recognized by Solid's types, either:
- Update Solid types (if the runtime supports it)
- Extend the interface properly with a typed override
Additionally, verify that passing plugins: undefined (when no serialization adapters exist) is acceptable behavior.
Run the following script to check consistency with the streaming renderer and verify the implementation:
🏁 Script executed:
#!/bin/bash
# Verify consistency with renderRouterToStream and check for similar type issues
# Check if renderRouterToStream has similar implementation
rg -A 10 'makeSsrSerovalPlugin' packages/solid-router/src/ssr/
# Look for type definitions of serializationAdapters in router options
rg -n 'serializationAdapters' packages/
# Check Solid's renderToString type signature
rg -n 'renderToString.*plugins' packages/Length of output: 8262
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for Solid.renderToString and renderToStream type definitions or usage
# Check imports of Solid and its types in both files
rg -n "import.*Solid\|from.*solid-js" packages/solid-router/src/ssr/ -A 2
# Check if there are type definitions for renderToString options
rg -n "renderToString\|renderToStream" packages/solid-router/src/ssr/ -B 2 -A 5
# Look for type definitions in solid-router package for Solid types
find packages/solid-router -name "*.ts" -o -name "*.tsx" | head -20
# Check if there's a tsconfig or type declaration that might define Solid types
find packages/solid-router -name "tsconfig.json" -o -name "*.d.ts" | head -10
Length of output: 848
🏁 Script executed:
#!/bin/bash
# Check the complete renderRouterToString implementation to see full context
cat packages/solid-router/src/ssr/renderRouterToString.tsxLength of output: 1372
🏁 Script executed:
#!/bin/bash
# Check renderRouterToStream for comparison
cat packages/solid-router/src/ssr/renderRouterToStream.tsxLength of output: 1530
🏁 Script executed:
#!/bin/bash
# Search for any type-safe plugin handling elsewhere in the codebase
rg -n "serovalPlugins|makeSsrSerovalPlugin" packages/ -B 2 -A 2Length of output: 9697
---
🌐 Web query:
```web
Solid.js renderToString plugins option type definition
💡 Result:
Do you mean the TypeScript type for the "plugins" option on Solid's renderToString (which package/version)? If so, which Solid (solid-js/solid-start) version are you using so I can look up the exact type definition?
---
🏁 Script executed:
```shell
#!/bin/bash
# Find solid-js version in solid-router package.json
cat packages/solid-router/package.json | grep -A 5 -B 5 "solid-js\|dependencies"
Length of output: 593
🏁 Script executed:
#!/bin/bash
# Also check if there are any type declarations or .d.ts files for Solid in node_modules info
cat packages/solid-router/package.jsonLength of output: 3754
---
🌐 Web query:
```web
solid-js 1.9 renderToString options plugins parameter
💡 Result:
Short answer: renderToString in Solid v1.9 does not accept a "plugins" parameter. Its supported options are just the SSR-related fields (nonce and renderId for renderToString; renderToStringAsync adds timeoutMs). See the official API docs. [1][2]
Sources
- renderToString (Solid docs). [1]
- renderToStringAsync / renderToStream (Solid docs — shows other SSR options like timeoutMs/onComplete). [2]
---
🌐 Web query:
```web
seroval Solid.js integration plugins renderToString
💡 Result:
Short answer
- Seroval is the serializer Solid / SolidStart uses for SSR state transfer; Solid exposes renderToString / renderToStringAsync for SSR. [1][2][3]
- There is a seroval-plugins package (plugins for seroval) maintained in the seroval repo/monorepo. [4]
- Common issue: switching to renderToStringAsync / streaming can cause seroval serialization errors (e.g., trying to serialize signals/functions/Store proxies). SolidStart currently doesn’t let you arbitrarily inject custom seroval plugins for security reasons (see issue/discussion). [2][5][6]
Pointers and links
- Solid SSR render APIs (renderToString, renderToStringAsync, renderToStream). Use renderToStringAsync when you need to wait for Suspense; these APIs affect what gets serialized. [2]
- seroval package (what it serializes, async serialization helpers and references). [3]
- seroval-plugins package / repo (plugins for seroval). [4]
- Community threads / issues about seroval errors when serializing signals/Stores and the request to allow custom plugins in SolidStart (feature request / rationale: security). [5][6]
If you want, I can:
- show example patterns to avoid seroval errors (convert non-serializable values to plain objects, use unwrap/JSON-safe values), or
- search the repo/issues for the exact SolidStart version you’re using and any recent changes to plugin support. Which would you prefer?
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for TODO, FIXME, or comments about plugins in solid-router SSR files
rg -n "TODO|FIXME|plugins|serovalPlugins" packages/solid-router/src/ssr/ -B 2 -A 2
Length of output: 1982
🏁 Script executed:
#!/bin/bash
# Check if serializationAdapters is used anywhere else in solid-router
rg -n "serializationAdapters" packages/solid-router/ -B 1 -A 1Length of output: 1279
🏁 Script executed:
#!/bin/bash
# Search for tests of renderRouterToString to see if plugins are tested
find packages/solid-router -name "*.test.*" -o -name "*.spec.*" | xargs grep -l "renderRouterToString\|serovalPlugins" 2>/dev/null || echo "No matching test files found"Length of output: 28
Remove unsupported plugins parameter from Solid render calls — violates strict type safety.
Solid.js v1.9's renderToString and renderToStream do not accept a plugins parameter. Passing it via as any bypass suppresses TypeScript errors while the parameter is silently ignored at runtime, making the serialization adapter code non-functional.
In both packages/solid-router/src/ssr/renderRouterToString.tsx and packages/solid-router/src/ssr/renderRouterToStream.tsx:
- Remove the
serializationAdapterslookup (lines 16–18 and 24–26) - Remove the
serovalPluginscomputation (lines 19–21 and 27–29) - Remove the
plugins: serovalPlugins,line from the options object (lines 26 and 41) - Remove the
as anycast (lines 27 and 42)
Solid intentionally does not expose custom plugin configuration for security reasons; state serialization is handled internally by Solid/SolidStart.
🤖 Prompt for AI Agents
In packages/solid-router/src/ssr/renderRouterToString.tsx around lines 24–27,
the call to Solid.renderToString passes an unsupported plugins option and uses
an as any cast; remove the serialization-adapter lookup and serovalPlugins
computation, drop the plugins: serovalPlugins property from the options object,
and remove the as any cast so the call uses only the supported options (e.g.,
nonce). Also remove any now-unused imports or variables related to
serializationAdapters/serovalPlugins.
Blocked by
Summary by CodeRabbit
New Features
Improvements
Tests