-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat: add start context bridge #6593
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughAdds an end-to-end React Start context-bridge test app, a generic createStartContextBridge utility for serializing selected server context to the client, route/router wiring to consume the bridged context, Playwright tests, and an isServer export-map update in router-core. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Playwright
participant DevServer as Dev Server (Vite)
participant Server as Start Instance
participant Browser as Client (Browser)
participant Router as TanStack Router
Test->>DevServer: start webServer
DevServer->>Server: forward HTTP requests
Server->>Server: mwA -> mwB -> mwC (augment request context)
Server->>Browser: serve HTML (router.dehydrate includes [bridgeKey]: selected)
Browser->>Router: hydrate (router.hydrate merges bridged data into router.context)
Browser->>Router: render route -> component reads bridged context via createStartContextBridge.get()
Test->>Browser: navigate to /next and re-check bridged context
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
View your CI Pipeline Execution ↗ for commit 2cd5db8
☁️ Nx Cloud last updated this comment at |
615893a to
3eec39b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@e2e/react-start/context-bridge/package.json`:
- Around line 14-30: Update the package.json dependency specifiers for internal
TanStack packages to use the workspace protocol wildcard: replace "workspace:^"
with "workspace:*" for the packages named "@tanstack/react-router",
"@tanstack/react-router-devtools", "@tanstack/react-start", and
"@tanstack/router-e2e-utils" so they use workspace:* instead of workspace:^.
In `@e2e/react-start/context-bridge/src/routes/__root.tsx`:
- Around line 23-58: RootDocument renders the lazily-loaded RouterDevtools
(RouterDevtools) directly which will throw without a Suspense boundary; update
RootDocument to wrap the RouterDevtools invocation in a React.Suspense boundary
(with a lightweight fallback such as null or a small loader) so the lazy import
resolves correctly, and ensure React.Suspense is imported/available in the file;
keep the existing production-guarded RouterDevtools definition unchanged.
In `@e2e/react-start/context-bridge/vite.config.ts`:
- Around line 7-10: The Vite config currently hardcodes server.port to 3000
inside the defineConfig call; update the server.port assignment in
vite.config.ts to read process.env.VITE_SERVER_PORT and parse it as an integer
(fallback to 3000 when missing) so Vite respects the port Playwright provides;
modify the server.port expression used in defineConfig accordingly (use
parseInt(process.env.VITE_SERVER_PORT) with a default).
🧹 Nitpick comments (2)
e2e/react-start/context-bridge/src/utils/sortJson.ts (1)
1-17: Tighten typing to JSON values to avoid unsafe casts.The current
as T/as unknown as Tcasts sidestep strict typing. Consider a JSON-specific type to keep this utility safe and self-documenting.♻️ Suggested refactor
+type JsonPrimitive = string | number | boolean | null +type JsonValue = JsonPrimitive | JsonValue[] | { [key: string]: JsonValue } + -export function sortJson<T>(value: T): T { +export function sortJson(value: JsonValue): JsonValue { if (Array.isArray(value)) { - return value.map(sortJson) as T + return value.map(sortJson) } if (!value || typeof value !== 'object') { return value } - const obj = value as Record<string, unknown> - const out: Record<string, unknown> = {} + const obj = value as Record<string, JsonValue> + const out: Record<string, JsonValue> = {} for (const key of Object.keys(obj).sort()) { out[key] = sortJson(obj[key]) } - return out as T + return out }As per coding guidelines: Use TypeScript strict mode with extensive type safety for all code.
packages/start-client-core/src/createStartContextBridge.ts (1)
70-80: Avoidanyon the hydrate payload to preserve strict typing.
dehydrated: anyand related casts bypass the strict-mode intent. Preferunknown+ narrowing (or a shared Dehydrated type) to keep safety intact.♻️ Suggested refactor
- router.options.hydrate = async (dehydrated: any) => { - await ogHydrate?.(dehydrated) - const selected = dehydrated?.[key] as - | Record<string, unknown> - | undefined - if (!selected) return + router.options.hydrate = async (dehydrated: unknown) => { + await ogHydrate?.(dehydrated as unknown) + if (!dehydrated || typeof dehydrated !== 'object') return + const selected = (dehydrated as Record<string, unknown>)[key] + if (!selected || typeof selected !== 'object') return router.update({ - context: safeObjectMerge(router.options.context, selected), + context: safeObjectMerge( + router.options.context, + selected as Record<string, unknown>, + ), }) }As per coding guidelines: Use TypeScript strict mode with extensive type safety for all code.
| "dependencies": { | ||
| "@tanstack/react-router": "workspace:^", | ||
| "@tanstack/react-router-devtools": "workspace:^", | ||
| "@tanstack/react-start": "workspace:^", | ||
| "express": "^5.1.0", | ||
| "http-proxy-middleware": "^3.0.5", | ||
| "react": "^19.0.0", | ||
| "react-dom": "^19.0.0" | ||
| }, | ||
| "devDependencies": { | ||
| "@playwright/test": "^1.50.1", | ||
| "@tailwindcss/vite": "^4.1.18", | ||
| "@tanstack/router-e2e-utils": "workspace:^", | ||
| "@types/node": "^22.10.2", | ||
| "@types/react": "^19.0.8", | ||
| "@types/react-dom": "^19.0.3", | ||
| "@vitejs/plugin-react": "^4.3.4", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use workspace:* for internal TanStack dependencies.
🔧 Suggested update
"dependencies": {
- "@tanstack/react-router": "workspace:^",
- "@tanstack/react-router-devtools": "workspace:^",
- "@tanstack/react-start": "workspace:^",
+ "@tanstack/react-router": "workspace:*",
+ "@tanstack/react-router-devtools": "workspace:*",
+ "@tanstack/react-start": "workspace:*",
"express": "^5.1.0",
"http-proxy-middleware": "^3.0.5",
"react": "^19.0.0",
"react-dom": "^19.0.0"
},
"devDependencies": {
"@playwright/test": "^1.50.1",
"@tailwindcss/vite": "^4.1.18",
- "@tanstack/router-e2e-utils": "workspace:^",
+ "@tanstack/router-e2e-utils": "workspace:*",As per coding guidelines: Use workspace protocol workspace:* for internal dependencies in package.json files.
📝 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.
| "dependencies": { | |
| "@tanstack/react-router": "workspace:^", | |
| "@tanstack/react-router-devtools": "workspace:^", | |
| "@tanstack/react-start": "workspace:^", | |
| "express": "^5.1.0", | |
| "http-proxy-middleware": "^3.0.5", | |
| "react": "^19.0.0", | |
| "react-dom": "^19.0.0" | |
| }, | |
| "devDependencies": { | |
| "@playwright/test": "^1.50.1", | |
| "@tailwindcss/vite": "^4.1.18", | |
| "@tanstack/router-e2e-utils": "workspace:^", | |
| "@types/node": "^22.10.2", | |
| "@types/react": "^19.0.8", | |
| "@types/react-dom": "^19.0.3", | |
| "@vitejs/plugin-react": "^4.3.4", | |
| "dependencies": { | |
| "@tanstack/react-router": "workspace:*", | |
| "@tanstack/react-router-devtools": "workspace:*", | |
| "@tanstack/react-start": "workspace:*", | |
| "express": "^5.1.0", | |
| "http-proxy-middleware": "^3.0.5", | |
| "react": "^19.0.0", | |
| "react-dom": "^19.0.0" | |
| }, | |
| "devDependencies": { | |
| "@playwright/test": "^1.50.1", | |
| "@tailwindcss/vite": "^4.1.18", | |
| "@tanstack/router-e2e-utils": "workspace:*", | |
| "@types/node": "^22.10.2", | |
| "@types/react": "^19.0.8", | |
| "@types/react-dom": "^19.0.3", | |
| "@vitejs/plugin-react": "^4.3.4", |
🤖 Prompt for AI Agents
In `@e2e/react-start/context-bridge/package.json` around lines 14 - 30, Update the
package.json dependency specifiers for internal TanStack packages to use the
workspace protocol wildcard: replace "workspace:^" with "workspace:*" for the
packages named "@tanstack/react-router", "@tanstack/react-router-devtools",
"@tanstack/react-start", and "@tanstack/router-e2e-utils" so they use
workspace:* instead of workspace:^.
| const RouterDevtools = | ||
| process.env.NODE_ENV === 'production' | ||
| ? () => null | ||
| : React.lazy(() => | ||
| import('@tanstack/react-router-devtools').then((res) => ({ | ||
| default: res.TanStackRouterDevtools, | ||
| })), | ||
| ) | ||
|
|
||
| function RootDocument({ children }: { children: React.ReactNode }) { | ||
| return ( | ||
| <html> | ||
| <head> | ||
| <HeadContent /> | ||
| </head> | ||
| <body> | ||
| <div className="p-2 flex gap-3 text-lg"> | ||
| <Link | ||
| to="/" | ||
| activeOptions={{ exact: true }} | ||
| activeProps={{ className: 'font-bold' }} | ||
| > | ||
| Home | ||
| </Link> | ||
| <Link | ||
| to="/next" | ||
| activeOptions={{ exact: true }} | ||
| activeProps={{ className: 'font-bold' }} | ||
| > | ||
| Next | ||
| </Link> | ||
| </div> | ||
| <hr /> | ||
| {children} | ||
| <RouterDevtools position="bottom-right" /> | ||
| <Scripts /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
React.lazy Suspense boundary requirement React 18 19 runtime error
💡 Result:
In React 18 and React 19, a React.lazy() component must be rendered under an ancestor <Suspense> boundary because rendering a lazy component while its code is still loading “suspends” (it throws a Promise). React then walks up the rendered tree looking for the nearest <Suspense> to show its fallback; if it can’t find one, you’ll get a runtime failure/blank screen. [1]
Key “boundary requirement” gotchas that commonly cause runtime errors:
- The
<Suspense>must be above the lazy component in the rendered tree. A<Suspense>inside the component that suspends won’t help, because React can’t reach it when that component suspends during its own render. [2] - If you see
A component suspended while responding to synchronous input…in React 18+, it usually means a user-driven synchronous update (navigation, typing, click) caused something to suspend (often a lazy route or Suspense-based data). Typical fixes are:- move/add a higher Suspense boundary (e.g., wrap your router / route outlet), and/or
- wrap the update that triggers the suspend in
startTransition. [3]
Minimal correct pattern
import { Suspense, lazy } from "react";
const Page = lazy(() => import("./Page"));
export function App() {
return (
<Suspense fallback={<div>Loading…</div>}>
<Page />
</Suspense>
);
}Sources: React lazy docs (v19.2) [1], explanation of boundary placement (React walks up to nearest <Suspense>) [2], React 18 runtime error (“suspended while responding to synchronous input”) discussion and fixes [3].
[1] (react.dev)
[2] (stackoverflow.com)
[3] (stackoverflow.com)
Citations:
- 1: https://react.dev/reference/react/lazy?utm_source=openai
- 2: https://stackoverflow.com/questions/77485440/react-query-suspense-issue-suspended-while-responding-to-synchronous-input?utm_source=openai
- 3: https://stackoverflow.com/questions/72167518/a-component-suspended-while-responding-to-synchronous-input?utm_source=openai
Wrap the lazy devtools in a Suspense boundary.
Rendering a React.lazy component without Suspense will throw at runtime. The <Suspense> boundary must be placed above the lazy component in the rendered tree for React to properly handle code-splitting and suspense.
✅ Suggested fix
- <RouterDevtools position="bottom-right" />
+ <React.Suspense fallback={null}>
+ <RouterDevtools position="bottom-right" />
+ </React.Suspense>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const RouterDevtools = | |
| process.env.NODE_ENV === 'production' | |
| ? () => null | |
| : React.lazy(() => | |
| import('@tanstack/react-router-devtools').then((res) => ({ | |
| default: res.TanStackRouterDevtools, | |
| })), | |
| ) | |
| function RootDocument({ children }: { children: React.ReactNode }) { | |
| return ( | |
| <html> | |
| <head> | |
| <HeadContent /> | |
| </head> | |
| <body> | |
| <div className="p-2 flex gap-3 text-lg"> | |
| <Link | |
| to="/" | |
| activeOptions={{ exact: true }} | |
| activeProps={{ className: 'font-bold' }} | |
| > | |
| Home | |
| </Link> | |
| <Link | |
| to="/next" | |
| activeOptions={{ exact: true }} | |
| activeProps={{ className: 'font-bold' }} | |
| > | |
| Next | |
| </Link> | |
| </div> | |
| <hr /> | |
| {children} | |
| <RouterDevtools position="bottom-right" /> | |
| <Scripts /> | |
| const RouterDevtools = | |
| process.env.NODE_ENV === 'production' | |
| ? () => null | |
| : React.lazy(() => | |
| import('@tanstack/react-router-devtools').then((res) => ({ | |
| default: res.TanStackRouterDevtools, | |
| })), | |
| ) | |
| function RootDocument({ children }: { children: React.ReactNode }) { | |
| return ( | |
| <html> | |
| <head> | |
| <HeadContent /> | |
| </head> | |
| <body> | |
| <div className="p-2 flex gap-3 text-lg"> | |
| <Link | |
| to="/" | |
| activeOptions={{ exact: true }} | |
| activeProps={{ className: 'font-bold' }} | |
| > | |
| Home | |
| </Link> | |
| <Link | |
| to="/next" | |
| activeOptions={{ exact: true }} | |
| activeProps={{ className: 'font-bold' }} | |
| > | |
| Next | |
| </Link> | |
| </div> | |
| <hr /> | |
| {children} | |
| <React.Suspense fallback={null}> | |
| <RouterDevtools position="bottom-right" /> | |
| </React.Suspense> | |
| <Scripts /> |
🤖 Prompt for AI Agents
In `@e2e/react-start/context-bridge/src/routes/__root.tsx` around lines 23 - 58,
RootDocument renders the lazily-loaded RouterDevtools (RouterDevtools) directly
which will throw without a Suspense boundary; update RootDocument to wrap the
RouterDevtools invocation in a React.Suspense boundary (with a lightweight
fallback such as null or a small loader) so the lazy import resolves correctly,
and ensure React.Suspense is imported/available in the file; keep the existing
production-guarded RouterDevtools definition unchanged.
| export default defineConfig({ | ||
| server: { | ||
| port: 3000, | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify port alignment between Vite and Playwright config
rg -n "getTestServerPort|baseURL|port" e2e/react-start/context-bridge/playwright.config.ts e2e/react-start/context-bridge/vite.config.tsRepository: TanStack/router
Length of output: 1541
🏁 Script executed:
cat e2e/react-start/context-bridge/vite.config.tsRepository: TanStack/router
Length of output: 504
🏁 Script executed:
cat e2e/react-start/context-bridge/playwright.config.tsRepository: TanStack/router
Length of output: 788
Vite config must respect the VITE_SERVER_PORT environment variable that Playwright passes.
The vite.config.ts hardcodes port: 3000, but playwright.config.ts is configured to pass a dynamically-derived port via the VITE_SERVER_PORT environment variable (line 16 in playwright.config.ts: VITE_SERVER_PORT=${PORT}). The Vite config ignores this, causing test failures when the Playwright webServer tries to reach the dev server on the derived port while Vite listens on 3000.
Update vite.config.ts to read the environment variable:
server: {
port: process.env.VITE_SERVER_PORT ? parseInt(process.env.VITE_SERVER_PORT) : 3000,
},
🤖 Prompt for AI Agents
In `@e2e/react-start/context-bridge/vite.config.ts` around lines 7 - 10, The Vite
config currently hardcodes server.port to 3000 inside the defineConfig call;
update the server.port assignment in vite.config.ts to read
process.env.VITE_SERVER_PORT and parse it as an integer (fallback to 3000 when
missing) so Vite respects the port Playwright provides; modify the server.port
expression used in defineConfig accordingly (use
parseInt(process.env.VITE_SERVER_PORT) with a default).
3eec39b to
2cd5db8
Compare
Summary by CodeRabbit
New Features
Tests
Chores