ssr streaming per request#7714
Open
schiller-manuel wants to merge 5 commits into
Open
Conversation
github-actions
Bot
added
documentation
Contributor
📝 WalkthroughWalkthroughAdds request-scoped SSR streaming resolution and ChangesCore SSR streaming policy library
New
Streaming-policy additions to existing e2e apps
ChangesCore SSR streaming policy library
New
Streaming-policy additions to existing e2e apps
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
|
View your CI Pipeline Execution ↗ for commit e19c8a9
☁️ Nx Cloud last updated this comment at |
Contributor
🚀 Changeset Version Preview1 package(s) bumped directly, 13 bumped as dependents. 🟩 Patch bumps
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
Contributor
Bundle Size Benchmarks
Current gzip tracks all emitted client JS chunks. Initial gzip tracks only the entry/import graph. Trend sparkline is historical current gzip ending with this PR measurement; lower is better. |
Merging this PR will not alter performance
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Memory | mem serialization-payload (vue) |
6.8 MB | 9.4 MB | -27.86% |
| ❌ | Memory | mem request-churn (solid) |
1.1 MB | 1.4 MB | -16.22% |
| ❌ | Memory | mem serialization-payload (react) |
31.8 MB | 34.2 MB | -7.26% |
| ❌ | Memory | mem aborted-requests (vue) |
1,021 KB | 1,061 KB | -3.78% |
| ⚡ | Memory | mem aborted-requests (solid) |
2.4 MB | 1.5 MB | +56.89% |
| ⚡ | Memory | mem streaming-peak chunked (solid) |
38.3 MB | 32.8 MB | +16.98% |
| ⚡ | Memory | mem loader-data-retention (solid) |
158.1 KB | 152.2 KB | +3.86% |
| ⚡ | Memory | mem serialization-payload (solid) |
6.8 MB | 6.6 MB | +3.7% |
| ⚡ | Memory | mem peak-large-page (solid) |
3.9 MB | 3.7 MB | +3.62% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing ssr-streaming-request (400192c) with main (ba52d2b)1
Footnotes
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 20
🧹 Nitpick comments (3)
packages/start-server-core/tests/createStartHandler.test.ts (1)
58-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the new streaming test helper typed.
These new lines erase the router type with
any, so future changes toserverSsr.shouldStream()orattachRouterServerSsrUtils()won't be caught by the compiler in the exact tests added for this API. Please switchonLoad,router, and the attach call toAnyRouter(or the concrete helper return type) instead. As per coding guidelines,**/*.{ts,tsx}: Use TypeScript strict mode with extensive type safety.Also applies to: 81-84
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/start-server-core/tests/createStartHandler.test.ts` around lines 58 - 60, The new streaming test helper is widening router types to any, which removes strict type safety from the tests. Update makeRouter so opts.onLoad, the local router variable, and the attachRouterServerSsrUtils call all use AnyRouter (or the concrete return type) instead of any. Keep the typing consistent in the related test cases that exercise serverSsr.shouldStream() so compiler checks still catch API changes.Source: Coding guidelines
e2e/solid-start/basic/src/server.ts (1)
60-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDrop the
as anyon the default-entry fetch call.defaultServerEntry.fetchalready acceptsssr.streamingthroughRequestOptions<TRegister>, so the cast only hides type drift here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/solid-start/basic/src/server.ts` around lines 60 - 64, Remove the unnecessary type cast from the defaultServerEntry.fetch call in server.ts; the fetch invocation already supports ssr.streaming via RequestOptions<TRegister>, so the cast is masking type safety. Update the return path in the server entry logic to pass the options object directly, and rely on the existing types around defaultServerEntry.fetch and getStreamingOverride(request) instead of forcing it to any.Source: Coding guidelines
e2e/vue-start/basic/src/server.ts (1)
57-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the
as anycast from thedefault-entrybranch.defaultServerEntry.fetchis already typed, so this bypasses strict checking on that request path; thread the real options type through instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/vue-start/basic/src/server.ts` around lines 57 - 64, Remove the unnecessary type escape in the default-entry branch of createServerEntry’s fetch handler: defaultServerEntry.fetch already has a proper options type, so replace the current any cast with the correct SSR options typing and pass that through directly. Update the fetch call in the STREAMING_SSR_ENTRY_FORM === 'default-entry' path so strict type checking is preserved without using a cast.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/react-start/streaming-ssr/src/server.ts`:
- Around line 58-62: The call to defaultServerEntry.fetch is using an
unnecessary as any cast even though RequestOptions<TRegister> already includes
ssr.streaming. Remove the cast and pass the typed options object directly in
server.ts so the fetch call remains type-safe; use defaultServerEntry.fetch and
getStreamingOverride(request) as the key symbols to locate the change.
In `@e2e/solid-start/streaming-ssr/src/routes/__root.tsx`:
- Around line 34-68: The demo navigation in the root shell is missing the new
/streaming-policy route, so update the nav in __root.tsx to include a Link for
that path alongside the other route entries. Keep the existing styling and
placement consistent with the other links so the streaming-policy scenario is
reachable from the in-app menu.
In `@e2e/solid-start/streaming-ssr/src/routes/index.tsx`:
- Around line 11-61: The home page in the streaming SSR route list is missing
links to several available scenarios, so add entries for the new routes
alongside the existing Link items in the index component. Update the route hub
in the index page to include /concurrent, /many-promises, and /query-heavy with
matching data-testid values and short descriptions, keeping the structure
consistent with the existing Link usage in the index route.
In `@e2e/solid-start/streaming-ssr/src/routes/query-heavy.tsx`:
- Around line 44-46: The slow-query timing values in the Solid route are too
high and no longer match the intended 200–400ms scenario used by the suite.
Update the delay arguments on the slowAsyncQuery1, slowAsyncQuery2, and
slowAsyncQuery3 definitions in query-heavy.tsx so they align with the same
pacing as the Vue counterpart and the route label. Keep the existing
makeQueryOptions calls and only correct the millisecond values.
In `@e2e/solid-start/streaming-ssr/src/server.ts`:
- Around line 57-62: The defaultServerEntry.fetch call in server.ts is using an
unnecessary as any cast on the SSR options object. Remove the cast and pass the
options directly, relying on the existing request-options shape; use
defaultServerEntry.fetch and the ssr.streaming object as the reference points
when updating the call.
In `@e2e/solid-start/streaming-ssr/tests/concurrent.spec.ts`:
- Around line 74-85: The concurrent streaming test currently only checks that
batch 1 and batch 3 both appear, so it does not prove ordering. Update the test
in concurrent.spec.ts within test('batch 1 resolves before batch 3') to assert
an intermediate state using the existing page.getByTestId selectors, such as
verifying the batch 3 element is still hidden or not yet present when batch 1
first becomes visible, before later confirming batch 3 eventually appears.
In `@e2e/solid-start/streaming-ssr/tests/fast-serial.spec.ts`:
- Around line 21-39: The fast-serial test is not actually proving immediate
availability because it waits for networkidle before checking any content;
update the streaming SSR test in fast-serial.spec.ts so the first assertion on
getByTestId('server-data') happens before waiting for the page to fully settle,
and keep the remaining checks after that to confirm the data is visible while
the request is still in progress.
In `@e2e/solid-start/streaming-ssr/tests/fixtures.ts`:
- Around line 50-73: The page fixture only captures console error output, so
uncaught browser exceptions can still slip through the SSR/hydration checks.
Update the `page` fixture in `fixtures.ts` to also listen for `pageerror`
events, collect those messages alongside `errorMessages`, and apply the same
`whitelistErrors` filtering used in the `page.on('console')` handler. Then
assert both collected error lists are empty after `await use(page)`.
In `@e2e/solid-start/streaming-ssr/tests/many-promises.spec.ts`:
- Around line 83-92: The test in many-promises.spec.ts does not actually prove
fast-before-slow ordering. Update the existing "fast promises resolve before
slow ones" case to verify ordering using the visible markers from the page, such
as the immediate and very-slow test ids. At the first checkpoint after
page.goto(), assert that the slow group is still absent before confirming the
immediate item is visible, or otherwise capture and compare first-visible timing
for the two groups. Keep the assertions tied to the current test names and
getByTestId checks so the ordering guarantee is explicit.
In `@e2e/solid-start/streaming-ssr/tests/nested-deferred.spec.ts`:
- Around line 66-75: The “expected order” check in nested-deferred.spec.ts is
only asserting eventual visibility and does not prove that level1 resolves
before level2, level3, or plain-deferred. Update the test around test('data
resolves in expected order (fastest first)') to add a directional assertion
using page.getByTestId('level1-data') and the other deferred markers, such as
checking the later ones are still not visible when level1 first appears or
capturing resolution/visibility timestamps and comparing them, so the race order
is actually verified.
In `@e2e/solid-start/streaming-ssr/tests/stream.spec.ts`:
- Around line 14-28: The streaming SSR test in testWithHydration('stream chunks
arrive incrementally') only validates the final buffered state, so adjust it to
prove incremental delivery. In stream.spec.ts, use the existing
page.getByTestId('stream-chunk-*') checks to assert an early chunk appears
before waiting for stream-complete, then verify the remaining chunks and
completion afterward. This keeps the test focused on the streaming behavior
rather than just the end result.
In `@e2e/solid-start/streaming-ssr/tests/sync-only.spec.ts`:
- Around line 50-54: The route in `sync-only.spec.ts` is matching only the path,
so it may not intercept the navigation request and leave `responseHtml` empty.
Update the `page.route` handler to match the full document URL using a glob like
`**/sync-only` or a URL predicate, and keep the existing
`route.fetch()`/`route.fulfill()` flow so the HTML assertions read the real
response.
In `@e2e/vue-start/streaming-ssr/src/routes/__root.tsx`:
- Around line 34-68: The demo navigation in __root.tsx is missing the
/streaming-policy route link, leaving the route unreachable from the app shell.
Update the nav list in the root layout component to include a Link for
/streaming-policy alongside the other demo routes, matching the existing Link
pattern used in the same nav block.
In `@e2e/vue-start/streaming-ssr/src/routes/streaming-policy.tsx`:
- Around line 6-17: The Streaming Policy test component is reading server-only
SSR state during render via useRouter() and router.serverSsr, which can diverge
between SSR and hydration. Move the render/head values out of the component
render path in streaming-policy.tsx and into serialized loader data (or another
client-available source) using Route.useLoaderData so the initial server render
and hydrated client render produce the same text.
In `@e2e/vue-start/streaming-ssr/tests/concurrent.spec.ts`:
- Around line 74-85: The concurrent SSR test does not באמת verify ordering
because it only checks that both batches appear within separate timeouts. Update
the test in concurrent.spec.ts so that the batch 1 visibility check using
getByTestId('concurrent-1-1') is paired with a negative assertion or short
polling check that getByTestId('concurrent-3-5') is still not visible when batch
1 first appears. Keep the existing test structure, but make the assertion around
the batch 1 and batch 3 elements explicitly prove that batch 3 lags behind batch
1.
In `@e2e/vue-start/streaming-ssr/tests/fixtures.ts`:
- Around line 50-72: The page fixture in fixtures.ts only captures console
errors, so uncaught runtime exceptions can slip through unnoticed. Update the
existing page setup in the page fixture to also listen for pageerror events and
route those messages through the same whitelist check and errorMessages
collection used by page.on('console'). Keep the final
expect(errorMessages).toEqual([]) assertion unchanged so both console errors and
page errors fail the test consistently.
In `@e2e/vue-start/streaming-ssr/tests/many-promises.spec.ts`:
- Around line 83-91: The test in many-promises.spec.ts does not באמת verify
fast-before-slow ordering because it only checks that an immediate item becomes
visible and later that a very-slow item eventually appears. Update the fast
promises resolve before slow ones test to first assert that a slow or very-slow
marker is not visible before the immediate group resolves, then assert the
immediate marker appears, and only afterward wait for the slow/very-slow marker
to become visible; use the existing page.getByTestId assertions in this test to
anchor the new negative and positive checks.
In `@e2e/vue-start/streaming-ssr/tests/nested-deferred.spec.ts`:
- Around line 29-36: The streaming loading-state test only waits for the final
resolved content in nested-deferred.spec.ts, so it can pass even if the fallback
UI never appears. Update the test case in shows loading states while data is
loading to assert the loading/fallback marker first after page.goto with
waitUntil: 'commit', then continue waiting for level3-data to become visible so
the test actually verifies the loading state behavior.
- Around line 66-75: The order assertion in nested-deferred.spec.ts only
verifies eventual visibility and does not prove the streaming sequence; update
the test around test('data resolves in expected order (fastest first)') to
assert that later elements like getByTestId('level2-data'),
getByTestId('level3-data'), and getByTestId('plain-deferred') are not visible
before level1-data resolves. Use at least one negative assertion before waiting
for the earlier item so the test actually checks relative ordering rather than
just final presence.
In `@e2e/vue-start/streaming-ssr/tests/stream.spec.ts`:
- Around line 14-27: The current `testWithHydration('stream chunks arrive
incrementally')` in `stream.spec.ts` only checks the final rendered state after
`stream-complete`, so it can miss regressions where `stream` buffers everything
before display. Update the test to assert incremental delivery by checking an
early chunk from `page.getByTestId('stream-chunk-0')` appears before waiting for
`stream-complete`, and verify one or more later chunks are still absent at that
moment, then keep the existing final assertions for all chunks.
---
Nitpick comments:
In `@e2e/solid-start/basic/src/server.ts`:
- Around line 60-64: Remove the unnecessary type cast from the
defaultServerEntry.fetch call in server.ts; the fetch invocation already
supports ssr.streaming via RequestOptions<TRegister>, so the cast is masking
type safety. Update the return path in the server entry logic to pass the
options object directly, and rely on the existing types around
defaultServerEntry.fetch and getStreamingOverride(request) instead of forcing it
to any.
In `@e2e/vue-start/basic/src/server.ts`:
- Around line 57-64: Remove the unnecessary type escape in the default-entry
branch of createServerEntry’s fetch handler: defaultServerEntry.fetch already
has a proper options type, so replace the current any cast with the correct SSR
options typing and pass that through directly. Update the fetch call in the
STREAMING_SSR_ENTRY_FORM === 'default-entry' path so strict type checking is
preserved without using a cast.
In `@packages/start-server-core/tests/createStartHandler.test.ts`:
- Around line 58-60: The new streaming test helper is widening router types to
any, which removes strict type safety from the tests. Update makeRouter so
opts.onLoad, the local router variable, and the attachRouterServerSsrUtils call
all use AnyRouter (or the concrete return type) instead of any. Keep the typing
consistent in the related test cases that exercise serverSsr.shouldStream() so
compiler checks still catch API changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 347f41c4-b03e-4287-aeef-ce735b9827b3
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (117)
docs/start/framework/react/guide/server-entry-point.mde2e/react-start/streaming-ssr/package.jsone2e/react-start/streaming-ssr/playwright.config.tse2e/react-start/streaming-ssr/src/routeTree.gen.tse2e/react-start/streaming-ssr/src/routes/index.tsxe2e/react-start/streaming-ssr/src/routes/streaming-policy.tsxe2e/react-start/streaming-ssr/src/server.tse2e/react-start/streaming-ssr/tests/home.spec.tse2e/react-start/streaming-ssr/tests/preview-streaming.spec.tse2e/react-start/streaming-ssr/tests/streaming-policy.spec.tse2e/react-start/streaming-ssr/vite.config.tse2e/solid-start/basic/playwright.config.tse2e/solid-start/basic/src/routeTree.gen.tse2e/solid-start/basic/src/routes/streaming-policy.tsxe2e/solid-start/basic/src/server.tse2e/solid-start/basic/tests/streaming-policy.spec.tse2e/solid-start/streaming-ssr/package.jsone2e/solid-start/streaming-ssr/playwright.config.tse2e/solid-start/streaming-ssr/src/routeTree.gen.tse2e/solid-start/streaming-ssr/src/router.tsxe2e/solid-start/streaming-ssr/src/routes/__root.tsxe2e/solid-start/streaming-ssr/src/routes/concurrent.tsxe2e/solid-start/streaming-ssr/src/routes/deferred-rejection.tsxe2e/solid-start/streaming-ssr/src/routes/deferred.tsxe2e/solid-start/streaming-ssr/src/routes/fast-serial.tsxe2e/solid-start/streaming-ssr/src/routes/index.tsxe2e/solid-start/streaming-ssr/src/routes/many-promises.tsxe2e/solid-start/streaming-ssr/src/routes/nested-deferred.tsxe2e/solid-start/streaming-ssr/src/routes/query-heavy.tsxe2e/solid-start/streaming-ssr/src/routes/slow-render.tsxe2e/solid-start/streaming-ssr/src/routes/stream.tsxe2e/solid-start/streaming-ssr/src/routes/streaming-policy.tsxe2e/solid-start/streaming-ssr/src/routes/sync-only.tsxe2e/solid-start/streaming-ssr/src/server.tse2e/solid-start/streaming-ssr/tests/client-navigation.spec.tse2e/solid-start/streaming-ssr/tests/concurrent.spec.tse2e/solid-start/streaming-ssr/tests/deferred-rejection.spec.tse2e/solid-start/streaming-ssr/tests/deferred.spec.tse2e/solid-start/streaming-ssr/tests/fast-serial.spec.tse2e/solid-start/streaming-ssr/tests/fixtures.tse2e/solid-start/streaming-ssr/tests/home.spec.tse2e/solid-start/streaming-ssr/tests/many-promises.spec.tse2e/solid-start/streaming-ssr/tests/nested-deferred.spec.tse2e/solid-start/streaming-ssr/tests/preview-streaming.spec.tse2e/solid-start/streaming-ssr/tests/query-heavy.spec.tse2e/solid-start/streaming-ssr/tests/slow-render.spec.tse2e/solid-start/streaming-ssr/tests/stream.spec.tse2e/solid-start/streaming-ssr/tests/streaming-policy.spec.tse2e/solid-start/streaming-ssr/tests/sync-only.spec.tse2e/solid-start/streaming-ssr/tsconfig.jsone2e/solid-start/streaming-ssr/vite.config.tse2e/vue-start/basic/playwright.config.tse2e/vue-start/basic/src/routeTree.gen.tse2e/vue-start/basic/src/routes/streaming-policy.tsxe2e/vue-start/basic/src/server.tse2e/vue-start/basic/tests/streaming-policy.spec.tse2e/vue-start/streaming-ssr/package.jsone2e/vue-start/streaming-ssr/playwright.config.tse2e/vue-start/streaming-ssr/src/routeTree.gen.tse2e/vue-start/streaming-ssr/src/router.tsxe2e/vue-start/streaming-ssr/src/routes/__root.tsxe2e/vue-start/streaming-ssr/src/routes/concurrent.tsxe2e/vue-start/streaming-ssr/src/routes/deferred-rejection.tsxe2e/vue-start/streaming-ssr/src/routes/deferred.tsxe2e/vue-start/streaming-ssr/src/routes/fast-serial.tsxe2e/vue-start/streaming-ssr/src/routes/index.tsxe2e/vue-start/streaming-ssr/src/routes/many-promises.tsxe2e/vue-start/streaming-ssr/src/routes/nested-deferred.tsxe2e/vue-start/streaming-ssr/src/routes/query-heavy.tsxe2e/vue-start/streaming-ssr/src/routes/slow-render.tsxe2e/vue-start/streaming-ssr/src/routes/stream.tsxe2e/vue-start/streaming-ssr/src/routes/streaming-policy.tsxe2e/vue-start/streaming-ssr/src/routes/sync-only.tsxe2e/vue-start/streaming-ssr/src/server.tse2e/vue-start/streaming-ssr/tests/client-navigation.spec.tse2e/vue-start/streaming-ssr/tests/concurrent.spec.tse2e/vue-start/streaming-ssr/tests/deferred-rejection.spec.tse2e/vue-start/streaming-ssr/tests/deferred.spec.tse2e/vue-start/streaming-ssr/tests/fast-serial.spec.tse2e/vue-start/streaming-ssr/tests/fixtures.tse2e/vue-start/streaming-ssr/tests/home.spec.tse2e/vue-start/streaming-ssr/tests/many-promises.spec.tse2e/vue-start/streaming-ssr/tests/nested-deferred.spec.tse2e/vue-start/streaming-ssr/tests/preview-streaming.spec.tse2e/vue-start/streaming-ssr/tests/query-heavy.spec.tse2e/vue-start/streaming-ssr/tests/slow-render.spec.tse2e/vue-start/streaming-ssr/tests/stream.spec.tse2e/vue-start/streaming-ssr/tests/streaming-policy.spec.tse2e/vue-start/streaming-ssr/tests/sync-only.spec.tse2e/vue-start/streaming-ssr/tsconfig.jsone2e/vue-start/streaming-ssr/vite.config.tspackages/react-router/package.jsonpackages/react-router/src/ssr/renderRouterToStream.tsxpackages/react-router/tests/renderRouterToStream.test.tsxpackages/router-core/package.jsonpackages/router-core/src/router.tspackages/router-core/src/ssr/createRequestHandler.tspackages/router-core/src/ssr/server.tspackages/router-core/src/ssr/ssr-server.tspackages/router-core/src/ssr/streaming.tspackages/router-core/tests/createRequestHandler-streaming.test.tspackages/router-core/tests/ssr-server-cleanup.test.tspackages/router-core/tests/ssr-server-manifest.test.tspackages/router-core/tests/ssr-server-streaming.test.tspackages/router-core/tests/ssr-streaming.test-d.tspackages/router-core/tests/ssr-streaming.test.tspackages/router-core/tests/transformStreamWithRouter.test.tspackages/solid-router/package.jsonpackages/solid-router/src/ssr/renderRouterToStream.tsxpackages/solid-router/tests/renderRouterToStream.test.tsxpackages/start-server-core/src/createStartHandler.tspackages/start-server-core/src/index.tsxpackages/start-server-core/src/request-handler.tspackages/start-server-core/tests/createStartHandler.test.tspackages/vue-router/package.jsonpackages/vue-router/src/ssr/renderRouterToStream.tsxpackages/vue-router/tests/renderRouterToStream.test.tsx
Comment on lines
+58
to
+62
| return defaultServerEntry.fetch(request, { | ||
| ssr: { | ||
| streaming: getStreamingOverride(request), | ||
| }, | ||
| } as any) |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find the server-entry implementation and relevant option types =="
fd -i 'server-entry*' packages e2e | xargs -r rg -n -C3 'createServerEntry|defaultServerEntry|fetch\s*\(|streaming'
echo
echo "== Find request option / SSR streaming type definitions =="
rg -n -C3 'RequestOptions|SsrStreamingResolverResult|SsrStreamingOption|streaming\??:' \
packages/start-server-core \
packages/router-core \
e2e/react-start/streaming-ssr/src/server.tsRepository: TanStack/router
Length of output: 226
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate defaultServerEntry and its fetch signature =="
rg -n -C4 'defaultServerEntry|fetch\s*\(' e2e packages --glob '!**/dist/**' --glob '!**/build/**'
echo
echo "== Locate SSR streaming option types =="
rg -n -C4 'streaming\??:|SsrStreaming|RequestOptions|request options|server entry' e2e packages --glob '!**/dist/**' --glob '!**/build/**'
echo
echo "== Inspect the target file =="
sed -n '1,120p' e2e/react-start/streaming-ssr/src/server.tsRepository: TanStack/router
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Target file with line numbers =="
sed -n '1,120p' e2e/react-start/streaming-ssr/src/server.ts | cat -n
echo
echo "== Request handler signature =="
sed -n '1,220p' packages/start-server-core/src/request-handler.ts | cat -n
echo
echo "== Default server entry implementation =="
sed -n '1,120p' packages/solid-start/src/default-entry/server.ts | cat -n
sed -n '1,120p' packages/vue-start/src/default-entry/server.ts | cat -n
echo
echo "== Any request-options/SSR option types =="
rg -n -C3 'type .*Options|interface .*Options|ssr:|streaming:|RequestHandler<|createServerEntry' packages/start-server-core packages/solid-start packages/vue-start --glob '!**/dist/**' --glob '!**/build/**'Repository: TanStack/router
Length of output: 29440
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== React start server-entry implementation =="
fd -i 'server.ts' packages/react-start e2e/react-start | xargs -r rg -n -C3 'createServerEntry|RequestHandler|defaultServerEntry|fetch\s*\('
echo
echo "== React start package exports =="
rg -n -C3 'server-entry|default-entry|RequestHandler|createServerEntry' packages/react-start packages/start-server-core --glob '!**/dist/**' --glob '!**/build/**'
echo
echo "== Target file exact context =="
sed -n '52,66p' e2e/react-start/streaming-ssr/src/server.ts | cat -nRepository: TanStack/router
Length of output: 22769
Remove the as any cast on defaultServerEntry.fetch. RequestOptions<TRegister> already exposes ssr.streaming, so this just silences the type check at the call site.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e/react-start/streaming-ssr/src/server.ts` around lines 58 - 62, The call
to defaultServerEntry.fetch is using an unnecessary as any cast even though
RequestOptions<TRegister> already includes ssr.streaming. Remove the cast and
pass the typed options object directly in server.ts so the fetch call remains
type-safe; use defaultServerEntry.fetch and getStreamingOverride(request) as the
key symbols to locate the change.
Source: Coding guidelines
Comment on lines
+34
to
+68
| <nav style={{ padding: '10px', 'border-bottom': '1px solid #ccc' }}> | ||
| <Link to="/" style={{ 'margin-right': '10px' }}> | ||
| Home | ||
| </Link> | ||
| <Link to="/sync-only" style={{ 'margin-right': '10px' }}> | ||
| Sync Only | ||
| </Link> | ||
| <Link to="/deferred" style={{ 'margin-right': '10px' }}> | ||
| Deferred | ||
| </Link> | ||
| <Link to="/deferred-rejection" style={{ 'margin-right': '10px' }}> | ||
| Deferred Rejection | ||
| </Link> | ||
| <Link to="/stream" style={{ 'margin-right': '10px' }}> | ||
| Stream | ||
| </Link> | ||
| <Link to="/fast-serial" style={{ 'margin-right': '10px' }}> | ||
| Fast Serial | ||
| </Link> | ||
| <Link to="/slow-render" style={{ 'margin-right': '10px' }}> | ||
| Slow Render | ||
| </Link> | ||
| <Link to="/nested-deferred" style={{ 'margin-right': '10px' }}> | ||
| Nested Deferred | ||
| </Link> | ||
| <Link to="/many-promises" style={{ 'margin-right': '10px' }}> | ||
| Many Promises | ||
| </Link> | ||
| <Link to="/query-heavy" style={{ 'margin-right': '10px' }}> | ||
| Query Heavy | ||
| </Link> | ||
| <Link to="/concurrent" style={{ 'margin-right': '10px' }}> | ||
| Concurrent | ||
| </Link> | ||
| </nav> |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add the /streaming-policy route to the demo nav.
Line 34 starts the route list, but this shell never links to /streaming-policy even though that route is part of the new suite. That makes the app UI incomplete and prevents reaching that scenario through in-app navigation.
Suggested fix
<Link to="/concurrent" style={{ 'margin-right': '10px' }}>
Concurrent
</Link>
+ <Link to="/streaming-policy" style={{ 'margin-right': '10px' }}>
+ Streaming Policy
+ </Link>
</nav>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <nav style={{ padding: '10px', 'border-bottom': '1px solid #ccc' }}> | |
| <Link to="/" style={{ 'margin-right': '10px' }}> | |
| Home | |
| </Link> | |
| <Link to="/sync-only" style={{ 'margin-right': '10px' }}> | |
| Sync Only | |
| </Link> | |
| <Link to="/deferred" style={{ 'margin-right': '10px' }}> | |
| Deferred | |
| </Link> | |
| <Link to="/deferred-rejection" style={{ 'margin-right': '10px' }}> | |
| Deferred Rejection | |
| </Link> | |
| <Link to="/stream" style={{ 'margin-right': '10px' }}> | |
| Stream | |
| </Link> | |
| <Link to="/fast-serial" style={{ 'margin-right': '10px' }}> | |
| Fast Serial | |
| </Link> | |
| <Link to="/slow-render" style={{ 'margin-right': '10px' }}> | |
| Slow Render | |
| </Link> | |
| <Link to="/nested-deferred" style={{ 'margin-right': '10px' }}> | |
| Nested Deferred | |
| </Link> | |
| <Link to="/many-promises" style={{ 'margin-right': '10px' }}> | |
| Many Promises | |
| </Link> | |
| <Link to="/query-heavy" style={{ 'margin-right': '10px' }}> | |
| Query Heavy | |
| </Link> | |
| <Link to="/concurrent" style={{ 'margin-right': '10px' }}> | |
| Concurrent | |
| </Link> | |
| </nav> | |
| <nav style={{ padding: '10px', 'border-bottom': '1px solid `#ccc`' }}> | |
| <Link to="/" style={{ 'margin-right': '10px' }}> | |
| Home | |
| </Link> | |
| <Link to="/sync-only" style={{ 'margin-right': '10px' }}> | |
| Sync Only | |
| </Link> | |
| <Link to="/deferred" style={{ 'margin-right': '10px' }}> | |
| Deferred | |
| </Link> | |
| <Link to="/deferred-rejection" style={{ 'margin-right': '10px' }}> | |
| Deferred Rejection | |
| </Link> | |
| <Link to="/stream" style={{ 'margin-right': '10px' }}> | |
| Stream | |
| </Link> | |
| <Link to="/fast-serial" style={{ 'margin-right': '10px' }}> | |
| Fast Serial | |
| </Link> | |
| <Link to="/slow-render" style={{ 'margin-right': '10px' }}> | |
| Slow Render | |
| </Link> | |
| <Link to="/nested-deferred" style={{ 'margin-right': '10px' }}> | |
| Nested Deferred | |
| </Link> | |
| <Link to="/many-promises" style={{ 'margin-right': '10px' }}> | |
| Many Promises | |
| </Link> | |
| <Link to="/query-heavy" style={{ 'margin-right': '10px' }}> | |
| Query Heavy | |
| </Link> | |
| <Link to="/concurrent" style={{ 'margin-right': '10px' }}> | |
| Concurrent | |
| </Link> | |
| <Link to="/streaming-policy" style={{ 'margin-right': '10px' }}> | |
| Streaming Policy | |
| </Link> | |
| </nav> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e/solid-start/streaming-ssr/src/routes/__root.tsx` around lines 34 - 68,
The demo navigation in the root shell is missing the new /streaming-policy
route, so update the nav in __root.tsx to include a Link for that path alongside
the other route entries. Keep the existing styling and placement consistent with
the other links so the streaming-policy scenario is reachable from the in-app
menu.
Comment on lines
+11
to
+61
| <p>This e2e project tests various SSR streaming scenarios:</p> | ||
| <ul> | ||
| <li> | ||
| <Link to="/sync-only" data-testid="link-sync-only"> | ||
| Sync Only | ||
| </Link>{' '} | ||
| - Tests synchronous serialization with no deferred/streaming data | ||
| </li> | ||
| <li> | ||
| <Link to="/deferred" data-testid="link-deferred"> | ||
| Deferred Data | ||
| </Link>{' '} | ||
| - Tests deferred promises resolving after initial render | ||
| </li> | ||
| <li> | ||
| <Link to="/deferred-rejection" data-testid="link-deferred-rejection"> | ||
| Deferred Rejection | ||
| </Link>{' '} | ||
| - Tests deferred promise rejections render through the error boundary | ||
| </li> | ||
| <li> | ||
| <Link to="/stream" data-testid="link-stream"> | ||
| ReadableStream | ||
| </Link>{' '} | ||
| - Tests streaming data via ReadableStream | ||
| </li> | ||
| <li> | ||
| <Link to="/fast-serial" data-testid="link-fast-serial"> | ||
| Fast Serialization | ||
| </Link>{' '} | ||
| - Tests when serialization completes before render finishes | ||
| </li> | ||
| <li> | ||
| <Link to="/slow-render" data-testid="link-slow-render"> | ||
| Slow Render | ||
| </Link>{' '} | ||
| - Tests when render takes longer than serialization | ||
| </li> | ||
| <li> | ||
| <Link to="/nested-deferred" data-testid="link-nested-deferred"> | ||
| Nested Deferred | ||
| </Link>{' '} | ||
| - Tests nested components with deferred data | ||
| </li> | ||
| <li> | ||
| <Link to="/streaming-policy" data-testid="link-streaming-policy"> | ||
| Streaming Policy | ||
| </Link>{' '} | ||
| - Tests request-controlled SSR streaming policy | ||
| </li> | ||
| </ul> |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add the missing scenario links to the home page.
This index omits /concurrent, /many-promises, and /query-heavy, even though those routes are part of the new streaming-SSR app. That leaves three scenarios unreachable from the landing page the suite uses as its scenario hub.
Suggested patch
<li>
+ <Link to="/concurrent" data-testid="link-concurrent">
+ Concurrent
+ </Link>{' '}
+ - Tests concurrent deferred promises resolving independently
+ </li>
+ <li>
+ <Link to="/many-promises" data-testid="link-many-promises">
+ Many Promises
+ </Link>{' '}
+ - Tests many concurrent deferred promises
+ </li>
+ <li>
+ <Link to="/query-heavy" data-testid="link-query-heavy">
+ Query Heavy
+ </Link>{' '}
+ - Tests heavier query/deferred serialization scenarios
+ </li>
+ <li>
<Link to="/nested-deferred" data-testid="link-nested-deferred">
Nested Deferred
</Link>{' '}
- Tests nested components with deferred data
</li>
- <li>
- <Link to="/streaming-policy" data-testid="link-streaming-policy">
- Streaming Policy
- </Link>{' '}
- - Tests request-controlled SSR streaming policy
- </li>
+ <li>
+ <Link to="/streaming-policy" data-testid="link-streaming-policy">
+ Streaming Policy
+ </Link>{' '}
+ - Tests request-controlled SSR streaming policy
+ </li>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <p>This e2e project tests various SSR streaming scenarios:</p> | |
| <ul> | |
| <li> | |
| <Link to="/sync-only" data-testid="link-sync-only"> | |
| Sync Only | |
| </Link>{' '} | |
| - Tests synchronous serialization with no deferred/streaming data | |
| </li> | |
| <li> | |
| <Link to="/deferred" data-testid="link-deferred"> | |
| Deferred Data | |
| </Link>{' '} | |
| - Tests deferred promises resolving after initial render | |
| </li> | |
| <li> | |
| <Link to="/deferred-rejection" data-testid="link-deferred-rejection"> | |
| Deferred Rejection | |
| </Link>{' '} | |
| - Tests deferred promise rejections render through the error boundary | |
| </li> | |
| <li> | |
| <Link to="/stream" data-testid="link-stream"> | |
| ReadableStream | |
| </Link>{' '} | |
| - Tests streaming data via ReadableStream | |
| </li> | |
| <li> | |
| <Link to="/fast-serial" data-testid="link-fast-serial"> | |
| Fast Serialization | |
| </Link>{' '} | |
| - Tests when serialization completes before render finishes | |
| </li> | |
| <li> | |
| <Link to="/slow-render" data-testid="link-slow-render"> | |
| Slow Render | |
| </Link>{' '} | |
| - Tests when render takes longer than serialization | |
| </li> | |
| <li> | |
| <Link to="/nested-deferred" data-testid="link-nested-deferred"> | |
| Nested Deferred | |
| </Link>{' '} | |
| - Tests nested components with deferred data | |
| </li> | |
| <li> | |
| <Link to="/streaming-policy" data-testid="link-streaming-policy"> | |
| Streaming Policy | |
| </Link>{' '} | |
| - Tests request-controlled SSR streaming policy | |
| </li> | |
| </ul> | |
| <p>This e2e project tests various SSR streaming scenarios:</p> | |
| <ul> | |
| <li> | |
| <Link to="/sync-only" data-testid="link-sync-only"> | |
| Sync Only | |
| </Link>{' '} | |
| - Tests synchronous serialization with no deferred/streaming data | |
| </li> | |
| <li> | |
| <Link to="/deferred" data-testid="link-deferred"> | |
| Deferred Data | |
| </Link>{' '} | |
| - Tests deferred promises resolving after initial render | |
| </li> | |
| <li> | |
| <Link to="/deferred-rejection" data-testid="link-deferred-rejection"> | |
| Deferred Rejection | |
| </Link>{' '} | |
| - Tests deferred promise rejections render through the error boundary | |
| </li> | |
| <li> | |
| <Link to="/stream" data-testid="link-stream"> | |
| ReadableStream | |
| </Link>{' '} | |
| - Tests streaming data via ReadableStream | |
| </li> | |
| <li> | |
| <Link to="/fast-serial" data-testid="link-fast-serial"> | |
| Fast Serialization | |
| </Link>{' '} | |
| - Tests when serialization completes before render finishes | |
| </li> | |
| <li> | |
| <Link to="/slow-render" data-testid="link-slow-render"> | |
| Slow Render | |
| </Link>{' '} | |
| - Tests when render takes longer than serialization | |
| </li> | |
| <li> | |
| <Link to="/concurrent" data-testid="link-concurrent"> | |
| Concurrent | |
| </Link>{' '} | |
| - Tests concurrent deferred promises resolving independently | |
| </li> | |
| <li> | |
| <Link to="/many-promises" data-testid="link-many-promises"> | |
| Many Promises | |
| </Link>{' '} | |
| - Tests many concurrent deferred promises | |
| </li> | |
| <li> | |
| <Link to="/query-heavy" data-testid="link-query-heavy"> | |
| Query Heavy | |
| </Link>{' '} | |
| - Tests heavier query/deferred serialization scenarios | |
| </li> | |
| <li> | |
| <Link to="/nested-deferred" data-testid="link-nested-deferred"> | |
| Nested Deferred | |
| </Link>{' '} | |
| - Tests nested components with deferred data | |
| </li> | |
| <li> | |
| <Link to="/streaming-policy" data-testid="link-streaming-policy"> | |
| Streaming Policy | |
| </Link>{' '} | |
| - Tests request-controlled SSR streaming policy | |
| </li> | |
| </ul> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e/solid-start/streaming-ssr/src/routes/index.tsx` around lines 11 - 61, The
home page in the streaming SSR route list is missing links to several available
scenarios, so add entries for the new routes alongside the existing Link items
in the index component. Update the route hub in the index page to include
/concurrent, /many-promises, and /query-heavy with matching data-testid values
and short descriptions, keeping the structure consistent with the existing Link
usage in the index route.
Comment on lines
+44
to
+46
| const slowAsyncQuery1 = makeQueryOptions('slow-async', 1, 'slow-async-1', 2000) | ||
| const slowAsyncQuery2 = makeQueryOptions('slow-async', 2, 'slow-async-2', 3000) | ||
| const slowAsyncQuery3 = makeQueryOptions('slow-async', 3, 'slow-async-3', 4000) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fix the Solid slow-query delays to match the intended 200–400ms case.
These values are 10x larger than the route label and the Vue counterpart, so the Solid suite is exercising a different scenario and paying a much longer wait penalty.
Suggested fix
-const slowAsyncQuery1 = makeQueryOptions('slow-async', 1, 'slow-async-1', 2000)
-const slowAsyncQuery2 = makeQueryOptions('slow-async', 2, 'slow-async-2', 3000)
-const slowAsyncQuery3 = makeQueryOptions('slow-async', 3, 'slow-async-3', 4000)
+const slowAsyncQuery1 = makeQueryOptions('slow-async', 1, 'slow-async-1', 200)
+const slowAsyncQuery2 = makeQueryOptions('slow-async', 2, 'slow-async-2', 300)
+const slowAsyncQuery3 = makeQueryOptions('slow-async', 3, 'slow-async-3', 400)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const slowAsyncQuery1 = makeQueryOptions('slow-async', 1, 'slow-async-1', 2000) | |
| const slowAsyncQuery2 = makeQueryOptions('slow-async', 2, 'slow-async-2', 3000) | |
| const slowAsyncQuery3 = makeQueryOptions('slow-async', 3, 'slow-async-3', 4000) | |
| const slowAsyncQuery1 = makeQueryOptions('slow-async', 1, 'slow-async-1', 200) | |
| const slowAsyncQuery2 = makeQueryOptions('slow-async', 2, 'slow-async-2', 300) | |
| const slowAsyncQuery3 = makeQueryOptions('slow-async', 3, 'slow-async-3', 400) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e/solid-start/streaming-ssr/src/routes/query-heavy.tsx` around lines 44 -
46, The slow-query timing values in the Solid route are too high and no longer
match the intended 200–400ms scenario used by the suite. Update the delay
arguments on the slowAsyncQuery1, slowAsyncQuery2, and slowAsyncQuery3
definitions in query-heavy.tsx so they align with the same pacing as the Vue
counterpart and the route label. Keep the existing makeQueryOptions calls and
only correct the millisecond values.
Comment on lines
+57
to
+62
| if (process.env.STREAMING_SSR_ENTRY_FORM === 'default-entry') { | ||
| return defaultServerEntry.fetch(request, { | ||
| ssr: { | ||
| streaming: getStreamingOverride(request), | ||
| }, | ||
| } as any) |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- defaultServerEntry.fetch call sites ---'
rg -n -C2 'defaultServerEntry\.fetch\(' e2e packages --glob '*.ts' --glob '*.tsx'
echo '--- request option typing around SSR streaming ---'
rg -n -C3 'type RequestOptions|interface RequestOptions|SsrStreamingResolverResult|streaming\??:' packages --glob '*.ts' --glob '*.tsx'Repository: TanStack/router
Length of output: 42160
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- e2e/solid-start/streaming-ssr/src/server.ts ---'
cat -n e2e/solid-start/streaming-ssr/src/server.ts | sed -n '1,120p'
echo '--- defaultServerEntry typing ---'
rg -n -C3 'defaultServerEntry|fetch\(' e2e packages --glob '*.ts' --glob '*.tsx' | sed -n '1,220p'
echo '--- server-entry package exports / types ---'
fd -a 'server-entry*' packages e2eRepository: TanStack/router
Length of output: 19590
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- locate server-entry source ---'
fd -a 'server-entry*' packages/solid-start packages/router-core packages/vue-start packages/react-start
echo '--- outline solid-start server-entry files ---'
ast-grep outline packages/solid-start/src/server-entry.ts --view expanded || true
ast-grep outline packages/solid-start/src/server-entry.tsx --view expanded || true
ast-grep outline packages/solid-start/src/index.ts --view expanded || true
echo '--- read likely server-entry implementation ---'
sed -n '1,260p' packages/solid-start/src/server-entry.ts 2>/dev/null || true
sed -n '1,260p' packages/solid-start/src/server-entry.tsx 2>/dev/null || true
sed -n '1,260p' packages/solid-start/src/index.ts 2>/dev/null || true
echo '--- inspect request-handler / createServerEntry types ---'
sed -n '1,240p' packages/start-server-core/src/request-handler.ts
sed -n '1,220p' packages/router-core/src/ssr/createRequestHandler.ts
sed -n '1,240p' packages/router-core/src/ssr/server.tsRepository: TanStack/router
Length of output: 10448
Remove the as any from this defaultServerEntry.fetch call. The options object already matches the SSR request-options shape, so the cast only bypasses strict type checking at the integration point this path is meant to cover.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e/solid-start/streaming-ssr/src/server.ts` around lines 57 - 62, The
defaultServerEntry.fetch call in server.ts is using an unnecessary as any cast
on the SSR options object. Remove the cast and pass the options directly,
relying on the existing request-options shape; use defaultServerEntry.fetch and
the ssr.streaming object as the reference points when updating the call.
Source: Coding guidelines
Comment on lines
+50
to
+72
| page: async ({ page, whitelistErrors }, use) => { | ||
| const errorMessages: Array<string> = [] | ||
|
|
||
| page.on('console', (m) => { | ||
| if (m.type() === 'error') { | ||
| const text = m.text() | ||
| for (const whitelistError of whitelistErrors) { | ||
| if ( | ||
| (typeof whitelistError === 'string' && | ||
| text.includes(whitelistError)) || | ||
| (whitelistError instanceof RegExp && whitelistError.test(text)) | ||
| ) { | ||
| return | ||
| } | ||
| } | ||
| errorMessages.push(text) | ||
| } | ||
| }) | ||
|
|
||
| await use(page) | ||
|
|
||
| // Assert no unexpected console errors | ||
| expect(errorMessages).toEqual([]) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files 'e2e/vue-start/streaming-ssr/tests/*' 'e2e/vue-start/streaming-ssr/**' | sed -n '1,200p'Repository: TanStack/router
Length of output: 1987
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- fixtures.ts ---'
sed -n '1,220p' e2e/vue-start/streaming-ssr/tests/fixtures.ts
echo
echo '--- pageerror search ---'
rg -n "pageerror|page\.on\('console'|page\.on\\(\" e2e/vue-start/streaming-ssr/tests e2e/vue-start/streaming-ssr/src -g '!**/node_modules/**'Repository: TanStack/router
Length of output: 2957
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- pageerror / console monitoring search ---'
rg -n "pageerror|console error monitoring|Assert no unexpected console errors|errorMessages" e2e/vue-start/streaming-ssr e2e -g '!**/node_modules/**' || true
echo
echo '--- Playwright pageerror mentions in repo ---'
rg -n "pageerror" . -g '!**/node_modules/**' || trueRepository: TanStack/router
Length of output: 2891
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- e2e-utils fixture ---'
sed -n '1,180p' e2e/e2e-utils/src/fixture.ts
echo
echo '--- streaming-ssr fixtures (react/solid) ---'
sed -n '1,120p' e2e/react-start/streaming-ssr/tests/fixtures.ts
echo
sed -n '1,120p' e2e/solid-start/streaming-ssr/tests/fixtures.tsRepository: TanStack/router
Length of output: 7570
Handle pageerror here too. Console errors miss uncaught runtime exceptions, so a client crash can still leave this suite green. Wire page.on('pageerror') into the same whitelist/assertion path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e/vue-start/streaming-ssr/tests/fixtures.ts` around lines 50 - 72, The page
fixture in fixtures.ts only captures console errors, so uncaught runtime
exceptions can slip through unnoticed. Update the existing page setup in the
page fixture to also listen for pageerror events and route those messages
through the same whitelist check and errorMessages collection used by
page.on('console'). Keep the final expect(errorMessages).toEqual([]) assertion
unchanged so both console errors and page errors fail the test consistently.
Comment on lines
+83
to
+91
| test('fast promises resolve before slow ones', async ({ page }) => { | ||
| // Navigate and check ordering - faster promises should be visible first | ||
| await page.goto('/many-promises', { waitUntil: 'commit' }) | ||
|
|
||
| // Immediate promises should appear first | ||
| await expect(page.getByTestId('immediate-1')).toBeVisible({ timeout: 2000 }) | ||
|
|
||
| // By the time we check very-slow, all should be visible | ||
| await expect(page.getByTestId('very-slow-2')).toBeVisible({ timeout: 5000 }) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
This does not actually verify fast-before-slow ordering.
Line 88 proves immediate-1 becomes visible within 2s, but it never proves the slow group was still unresolved at that point. If all 15 promises finished before the first assertion ran, this test would still pass. Add a negative check for a slow/very-slow marker before the immediate group resolves, then wait for the slow group afterward.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e/vue-start/streaming-ssr/tests/many-promises.spec.ts` around lines 83 -
91, The test in many-promises.spec.ts does not באמת verify fast-before-slow
ordering because it only checks that an immediate item becomes visible and later
that a very-slow item eventually appears. Update the fast promises resolve
before slow ones test to first assert that a slow or very-slow marker is not
visible before the immediate group resolves, then assert the immediate marker
appears, and only afterward wait for the slow/very-slow marker to become
visible; use the existing page.getByTestId assertions in this test to anchor the
new negative and positive checks.
Comment on lines
+29
to
+36
| test('shows loading states while data is loading', async ({ page }) => { | ||
| // Use fast navigation to catch loading states | ||
| await page.goto('/nested-deferred', { waitUntil: 'commit' }) | ||
|
|
||
| // Eventually all data should be visible | ||
| await expect(page.getByTestId('level3-data')).toBeVisible({ | ||
| timeout: 10000, | ||
| }) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
The loading-state test never observes a loading state.
After Line 31, this only waits for the final resolved content on Line 34. A build that skips the loading UI entirely would still pass. Assert the fallback/loading marker before level3-data resolves if you want this test to protect that behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e/vue-start/streaming-ssr/tests/nested-deferred.spec.ts` around lines 29 -
36, The streaming loading-state test only waits for the final resolved content
in nested-deferred.spec.ts, so it can pass even if the fallback UI never
appears. Update the test case in shows loading states while data is loading to
assert the loading/fallback marker first after page.goto with waitUntil:
'commit', then continue waiting for level3-data to become visible so the test
actually verifies the loading state behavior.
Comment on lines
+66
to
+75
| test('data resolves in expected order (fastest first)', async ({ page }) => { | ||
| await page.goto('/nested-deferred') | ||
|
|
||
| // Wait for all to be visible | ||
| await expect(page.getByTestId('level1-data')).toBeVisible({ timeout: 5000 }) | ||
| await expect(page.getByTestId('level2-data')).toBeVisible({ timeout: 5000 }) | ||
| await expect(page.getByTestId('level3-data')).toBeVisible({ timeout: 5000 }) | ||
| await expect(page.getByTestId('plain-deferred')).toBeVisible({ | ||
| timeout: 5000, | ||
| }) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
The order test only checks eventual visibility.
Lines 70-73 wait for each element in sequence, but they do not prove level1 appeared before level2, level3, or plain-deferred. If everything renders at once, or in the wrong order, this still passes. Add at least one negative assertion for the later levels before the earlier one resolves.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e/vue-start/streaming-ssr/tests/nested-deferred.spec.ts` around lines 66 -
75, The order assertion in nested-deferred.spec.ts only verifies eventual
visibility and does not prove the streaming sequence; update the test around
test('data resolves in expected order (fastest first)') to assert that later
elements like getByTestId('level2-data'), getByTestId('level3-data'), and
getByTestId('plain-deferred') are not visible before level1-data resolves. Use
at least one negative assertion before waiting for the earlier item so the test
actually checks relative ordering rather than just final presence.
Comment on lines
+14
to
+27
| testWithHydration('stream chunks arrive incrementally', async ({ page }) => { | ||
| await page.goto('/stream') | ||
|
|
||
| // Wait for stream to complete | ||
| await expect(page.getByTestId('stream-complete')).toBeVisible({ | ||
| timeout: 10000, | ||
| }) | ||
|
|
||
| // All chunks should be present | ||
| await expect(page.getByTestId('stream-chunk-0')).toContainText('chunk-0') | ||
| await expect(page.getByTestId('stream-chunk-1')).toContainText('chunk-1') | ||
| await expect(page.getByTestId('stream-chunk-2')).toContainText('chunk-2') | ||
| await expect(page.getByTestId('stream-chunk-3')).toContainText('chunk-3') | ||
| await expect(page.getByTestId('stream-chunk-4')).toContainText('chunk-4') |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
This only validates the final buffered state, not incremental streaming.
Line 18 waits for stream-complete before any chunk assertions, so a regression that buffers the whole response and renders everything at once would still pass. To cover incremental delivery, assert an early chunk appears before completion and that later chunks are still absent at that moment.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e/vue-start/streaming-ssr/tests/stream.spec.ts` around lines 14 - 27, The
current `testWithHydration('stream chunks arrive incrementally')` in
`stream.spec.ts` only checks the final rendered state after `stream-complete`,
so it can miss regressions where `stream` buffers everything before display.
Update the test to assert incremental delivery by checking an early chunk from
`page.getByTestId('stream-chunk-0')` appears before waiting for
`stream-complete`, and verify one or more later chunks are still absent at that
moment, then keep the existing final assertions for all chunks.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/solid-router/src/awaited.tsx`:
- Around line 55-57: The Awaited rendering in awaited.tsx is using truthiness to
decide whether to render, which incorrectly skips valid resolved values like 0,
false, and empty string. Update the Awaited component’s render logic around
resource(), Solid.Show, and props.children so it gates on the resource’s
readiness/loading state instead of the resolved value itself, ensuring children
always run once the resource is ready regardless of the payload’s truthiness.
- Around line 33-34: Restore the no-fallback path in Awaited/AwaitInner so SSR
stays blocking instead of rendering through a truthy `Show when={resource()}`
check; the current logic in `awaited.tsx` filters out valid falsy values like 0,
empty string, and false. Update the no-fallback branch to either keep the
existing blocking render behavior or explicitly gate on `resource() !==
undefined` so `AwaitInner` only waits for missing data and does not hide
legitimate resolved values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 602d5f80-ccf2-45e2-8ecc-1da07acf20bf
📒 Files selected for processing (10)
e2e/react-start/streaming-ssr/tests/deferred.spec.tse2e/react-start/streaming-ssr/tests/query-heavy.spec.tse2e/solid-start/streaming-ssr/tests/deferred.spec.tse2e/solid-start/streaming-ssr/tests/query-heavy.spec.tse2e/streaming-ssr-assertions.tse2e/vue-start/streaming-ssr/tests/deferred.spec.tse2e/vue-start/streaming-ssr/tests/query-heavy.spec.tspackages/router-core/src/ssr/transformStreamWithRouter.tspackages/router-core/tests/transformStreamWithRouter.test.tspackages/solid-router/src/awaited.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- e2e/vue-start/streaming-ssr/tests/deferred.spec.ts
- e2e/vue-start/streaming-ssr/tests/query-heavy.spec.ts
- e2e/solid-start/streaming-ssr/tests/query-heavy.spec.ts
- e2e/solid-start/streaming-ssr/tests/deferred.spec.ts
Comment on lines
33
to
+34
| if (!('fallback' in props)) { | ||
| const [resource] = Solid.createResource( | ||
| () => defer(props.promise), | ||
| (p) => p, | ||
| { | ||
| deferStream: true, | ||
| }, | ||
| ) | ||
|
|
||
| return ( | ||
| <Solid.Show when={resource()}> | ||
| {(data) => props.children(data())} | ||
| </Solid.Show> | ||
| ) | ||
| return <AwaitInner {...props} /> |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== target file ==\n'
sed -n '1,220p' packages/solid-router/src/awaited.tsx | cat -n
printf '\n== deferStream references ==\n'
rg -n "deferStream|AwaitInner|<Show|when=\{resource\(\)\}" packages/solid-router/src -S
printf '\n== awaited-related tests/usages ==\n'
rg -n "\bAwait\b|awaited" packages/solid-router/src packages/solid-router/test packages/solid-router/src -SRepository: TanStack/router
Length of output: 3085
🌐 Web query:
SolidJS createResource deferStream server-side rendering suspend stream fallback Show truthy values
💡 Result:
In SolidJS, the interaction between createResource, deferStream, Suspense, and Show is designed to provide granular control over server-side rendering (SSR) and data loading [1][2]. deferStream When used with createResource or createAsync, the deferStream: true option forces the server to wait for that specific resource to resolve before sending the initial HTML shell [1][3][2]. By default, deferStream is false, allowing the server to stream the HTML shell immediately (including Suspense fallbacks) while data is fetched asynchronously [1][2][4]. Setting it to true essentially turns the resource into a blocking operation for the SSR process [3][4]. Suspense and Streaming Suspense boundaries manage asynchronous dependencies [5]. During SSR, if a resource is not "deferred," the server will render and stream the fallback content of the Suspense boundary immediately [1][4]. Once the resource resolves, the server streams the actual component content to the client, replacing the fallback [1][4]. If multiple Suspense boundaries are used, each can stream its own data independently as it becomes ready [1][5]. Show and Truthy Values The Show component conditionally renders its children or a fallback based on whether the when prop is truthy [6][7]. - Behavior: When when is truthy, it renders the children; otherwise, it renders the fallback [6][7]. - Keyed vs. Non-Keyed: - Default (Non-keyed): Show tracks only the truthiness of the when value [6]. Switching between two different truthy values will not cause the children to be destroyed and recreated; instead, the existing component instance is updated [6][8]. - Keyed: By setting keyed={true}, Show treats the when value's identity as the condition [6]. If the value changes (even if it remains truthy), Show will destroy and recreate its children [6][8]. - Function Children: Using Show with a function child allows you to access the narrowed (non-nullable) value [6][9]. In non-keyed mode, the function receives an accessor for the value; in keyed mode, it receives the value directly [6][9]. Combining these allows you to create patterns like: <Suspense fallback={
Loading...
}> {(data) =>{data.someField}
} In this example, the Suspense boundary manages the loading state (showing the fallback during initial load), while Show manages the conditional rendering logic once the data is available [5][6]. [1][2][6]
Citations:
- 1: https://docs.solidjs.com/solid-router/data-fetching/streaming
- 2: https://docs.solidjs.com/reference/basic-reactivity/create-resource
- 3: How to fetch on server before rendering? solidjs/solid-start#1421
- 4: SSR with
<Suspense/>doesn't work anymore as it used to solidjs/solid-start#1402 - 5: https://docs.solidjs.com/reference/components/suspense
- 6: https://docs.solidjs.com/reference/components/show
- 7: https://docs.solidjs.com/concepts/control-flow/conditional-rendering
- 8: https://dev.to/daelmaak/solidjs-purpose-of-the-keyed-attribute-in-33bg
- 9: https://github.com/solidjs/solid/blob/next/documentation/solid-2.0/03-control-flow.md
Restore the no-fallback SSR behavior and stop filtering falsy results.
AwaitInner now routes the no-fallback case through Show when={resource()}, which can render an empty shell before the promise settles and also hides legitimate falsy values like 0, '', or false. Keep the no-fallback path on the blocking SSR behavior, or gate on resource() !== undefined instead of truthiness.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/solid-router/src/awaited.tsx` around lines 33 - 34, Restore the
no-fallback path in Awaited/AwaitInner so SSR stays blocking instead of
rendering through a truthy `Show when={resource()}` check; the current logic in
`awaited.tsx` filters out valid falsy values like 0, empty string, and false.
Update the no-fallback branch to either keep the existing blocking render
behavior or explicitly gate on `resource() !== undefined` so `AwaitInner` only
waits for missing data and does not hide legitimate resolved values.
Comment on lines
+55
to
+57
| return ( | ||
| <Solid.Show when={resource()}> | ||
| {(data) => props.children(data())} |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Render on readiness, not truthiness packages/solid-router/src/awaited.tsx:55 — <Solid.Show when={resource()}> skips valid resolved values like 0, false, and '', so children never runs for those cases. Gate this on the resource state/loading flag instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/solid-router/src/awaited.tsx` around lines 55 - 57, The Awaited
rendering in awaited.tsx is using truthiness to decide whether to render, which
incorrectly skips valid resolved values like 0, false, and empty string. Update
the Awaited component’s render logic around resource(), Solid.Show, and
props.children so it gates on the resource’s readiness/loading state instead of
the resolved value itself, ensuring children always run once the resource is
ready regardless of the payload’s truthiness.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit