tests(start): update spa mode e2e to use production build #5284
Conversation
WalkthroughAdds SPA-mode workflows to e2e React and Solid start examples: new Express-based servers with proxying, SPA build/start scripts and dependencies, Playwright config updates with START_PORT/env, Vite SPA configuration changes, and test adjustments (URL waits, conditional expectations) across navigation, not-found, redirect, and search-params specs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User (Browser)
participant SPA as SPA Server (Express) #f6f6ff
participant PROXY as Proxy Middleware #fff6f6
participant START as Start Server (Node handler) #f6fff6
participant STATIC as dist/client (Static)
U->>SPA: GET /any-route
alt Static asset
SPA-->>STATIC: Serve /assets/* from dist/client
STATIC-->>U: 200 Static file
else API or server-fn
SPA->>PROXY: /api/* or /_serverFn/*
PROXY->>START: Proxy request to START_PORT
START-->>PROXY: Response
PROXY-->>U: Proxied response
else Client route
SPA-->>U: 200 index.html
end
sequenceDiagram
autonumber
participant PW as Playwright
participant WS as Web Server Runner
participant SPA as SPA Server : PORT
participant START as Start Server : START_PORT
PW->>WS: Start servers (SPA mode)
WS->>START: load dist/server, toNodeHandler, listen START_PORT
WS->>SPA: start Express: static + proxy -> START_PORT, listen PORT
PW->>SPA: run tests against PORT
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2)**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
e2e/**📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧬 Code graph analysis (1)e2e/react-start/basic/playwright.config.ts (4)
⏰ 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)
Comment |
|
View your CI Pipeline Execution ↗ for commit 090b0c7
☁️ 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-start
@tanstack/solid-start-client
@tanstack/solid-start-server
@tanstack/start-client-core
@tanstack/start-plugin-core
@tanstack/start-server-core
@tanstack/start-static-server-functions
@tanstack/start-storage-context
@tanstack/valibot-adapter
@tanstack/virtual-file-routes
@tanstack/zod-adapter
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (14)
e2e/react-start/basic/tests/not-found.spec.ts (1)
4-4: Align import path with local tests conventionUse a simpler relative path for consistency with other files under tests/.
-import { isSpaMode } from '../tests/utils/isSpaMode' +import { isSpaMode } from './utils/isSpaMode'e2e/react-start/basic/tests/redirect.spec.ts (2)
48-53: Avoid hardcoded localhost in request predicateBind to the current page origin to work in environments that resolve to 127.0.0.1 or custom hosts.
- const requestPromise = new Promise<void>((resolve) => { + const origin = new URL(page.url()).origin + const requestPromise = new Promise<void>((resolve) => { page.on('request', (request) => { - if ( - request.url().startsWith(`http://localhost:${PORT}/_serverFn/`) - ) { + if (request.url().startsWith(`${origin}/_serverFn/`)) { requestHappened = true resolve() } }) })
91-95: Wait for the redirect URL to settle to reduce flakinessDirect-visit flow can race; explicitly wait for the expected URL before asserting.
- const url = `http://localhost:${PORT}/posts` - - expect(page.url()).toBe(url) + const url = `http://localhost:${PORT}/posts` + await page.waitForURL(url) + expect(page.url()).toBe(url)e2e/react-start/basic/vite.config.ts (1)
5-5: Decouple build config from tests utilRead
process.env.MODEdirectly in the config to avoid importing from tests and reduce coupling.-import { isSpaMode } from './tests/utils/isSpaMode' +const isSpa = process.env.MODE === 'spa' @@ - tanstackStart({ - spa: isSpaMode ? spaModeConfiguration : undefined, - }), + tanstackStart({ + spa: isSpa ? spaModeConfiguration : undefined, + }),Also applies to: 24-25
e2e/solid-start/basic/vite.config.ts (1)
5-12: Optional: inline env check instead of importing from testsKeep config self-contained by reading
process.env.MODEhere.-import { isSpaMode } from './tests/utils/isSpaMode' +const isSpa = process.env.MODE === 'spa' @@ - tanstackStart({ - spa: isSpaMode ? spaModeConfiguration : undefined, - }), + tanstackStart({ + spa: isSpa ? spaModeConfiguration : undefined, + }),Also applies to: 22-25
e2e/solid-start/basic/package.json (1)
10-10: Cross‑platform env var for MODEOn Windows shells,
MODE=spawon’t work. Considercross-envfor portability.- "build:spa": "MODE=spa vite build && tsc --noEmit", + "build:spa": "cross-env MODE=spa vite build && tsc --noEmit"Add to devDependencies:
+ "cross-env": "^7.0.3"e2e/solid-start/basic/playwright.config.ts (2)
9-14: Ports/env look correct for SPA; consider scoping START_PORT to SPA onlyDeriving distinct PORT/START_PORT keys per mode is good. Minor: in non‑SPA runs START_PORT equals PORT and isn’t used—omit it from env when
!isSpaModeto reduce noise.webServer: { command: isSpaMode ? spaModeCommand : ssrModeCommand, url: baseURL, reuseExistingServer: !process.env.CI, stdout: 'pipe', env: { MODE: process.env.MODE || '', VITE_NODE_ENV: 'test', VITE_EXTERNAL_PORT: String(EXTERNAL_PORT), VITE_SERVER_PORT: String(PORT), - START_PORT: String(START_PORT), + ...(isSpaMode ? { START_PORT: String(START_PORT) } : {}), PORT: String(PORT), }, },Also applies to: 48-48
20-20: Gate the config log behind a flagConsole noise can bloat Playwright output. Suggest logging only when
DEBUGorCIis set.-console.log('running in spa mode: ', isSpaMode.toString()) +if (process.env.DEBUG) console.log('running in spa mode:', isSpaMode)e2e/solid-start/basic/tests/navigation.spec.ts (1)
60-64: Assert page readiness before script globalsTo avoid evaluating globals before DOM is painted, mirror the client‑nav test by asserting the heading first.
await page.goto('/scripts') await page.waitForURL('/scripts') await page.waitForLoadState('networkidle') +await expect(page.getByTestId('scripts-test-heading')).toBeVisible() expect(await page.evaluate('window.SCRIPT_1')).toBe(true)e2e/react-start/basic/tests/navigation.spec.ts (2)
45-50: Mirror direct‑nav readiness checkAs in Solid, add a heading visibility assertion before reading script globals for deterministic timing.
await page.getByRole('link', { name: 'Scripts', exact: true }).click() await expect(page.getByTestId('scripts-test-heading')).toBeInViewport() expect(await page.evaluate('window.SCRIPT_1')).toBe(true) expect(await page.evaluate('window.SCRIPT_2')).toBe(undefined)And for direct navigation:
await page.goto('/scripts') await page.waitForURL('/scripts') await page.waitForLoadState('networkidle') +await expect(page.getByTestId('scripts-test-heading')).toBeVisible() expect(await page.evaluate('window.SCRIPT_1')).toBe(true) expect(await page.evaluate('window.SCRIPT_2')).toBe(undefined)Also applies to: 54-57
21-21: Consider adding the same initial wait in the first testFor consistency, add
await page.waitForURL('/')after the firstgoto('/')in “Navigating to post”.e2e/react-start/basic/server.js (3)
2-2: Harden file system paths (avoid CWD dependency in CI/Playwright)Relative paths to
./dist/...can break when the process CWD differs. Resolve from this file’s directory.Apply this diff:
import path from 'node:path' +import { fileURLToPath } from 'node:url' @@ -const startPort = process.env.START_PORT || 3001 +const startPort = process.env.START_PORT || 3001 + +const __dirname = path.dirname(fileURLToPath(import.meta.url)) +const CLIENT_DIR = path.resolve(__dirname, 'dist/client') +const SERVER_ENTRY = path.resolve(__dirname, 'dist/server/server.js') @@ - const server = (await import('./dist/server/server.js')).default + const server = (await import(SERVER_ENTRY)).default @@ - app.use(express.static('./dist/client')) + app.use(express.static(CLIENT_DIR)) @@ - app.use(express.static('./dist/client')) + app.use(express.static(CLIENT_DIR)) @@ - res.sendFile(path.resolve('./dist/client/index.html')) + res.sendFile(path.join(CLIENT_DIR, 'index.html'))Also applies to: 11-11, 16-16, 48-49, 51-51
57-67: Reduce startup race: ensure Start server is listening before SPA proxies to itCurrently both servers start concurrently; the SPA may proxy to a port that isn’t listening yet. Start the Start server first, then the SPA.
Apply this diff (ESM with top‑level await):
-createSpaServer().then(async ({ app }) => - app.listen(port, () => { - console.info(`Client Server: http://localhost:${port}`) - }), -) - -createStartServer().then(async ({ app }) => - app.listen(startPort, () => { - console.info(`Start Server: http://localhost:${startPort}`) - }), -) +const start = await createStartServer() +await new Promise((resolve) => + start.app.listen(startPort, () => { + console.info(`Start Server: http://localhost:${startPort}`) + resolve() + }), +) +const spa = await createSpaServer() +spa.app.listen(port, () => { + console.info(`Client Server: http://localhost:${port}`) +})
14-15: Minor hardening: hide Express fingerprintDisable the
X-Powered-Byheader.Apply this diff:
const app = express() + app.disable('x-powered-by') @@ const app = express() + app.disable('x-powered-by')Also applies to: 30-31
📜 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 (16)
e2e/react-start/basic/package.json(2 hunks)e2e/react-start/basic/playwright.config.ts(2 hunks)e2e/react-start/basic/server.js(1 hunks)e2e/react-start/basic/tests/navigation.spec.ts(3 hunks)e2e/react-start/basic/tests/not-found.spec.ts(3 hunks)e2e/react-start/basic/tests/redirect.spec.ts(1 hunks)e2e/react-start/basic/tests/search-params.spec.ts(1 hunks)e2e/react-start/basic/vite.config.ts(1 hunks)e2e/solid-start/basic/package.json(2 hunks)e2e/solid-start/basic/playwright.config.ts(2 hunks)e2e/solid-start/basic/server.js(1 hunks)e2e/solid-start/basic/tests/navigation.spec.ts(5 hunks)e2e/solid-start/basic/tests/not-found.spec.ts(2 hunks)e2e/solid-start/basic/tests/redirect.spec.ts(1 hunks)e2e/solid-start/basic/tests/search-params.spec.ts(3 hunks)e2e/solid-start/basic/vite.config.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
e2e/react-start/basic/tests/not-found.spec.tse2e/solid-start/basic/tests/redirect.spec.tse2e/react-start/basic/tests/search-params.spec.tse2e/react-start/basic/tests/redirect.spec.tse2e/solid-start/basic/tests/search-params.spec.tse2e/solid-start/basic/tests/not-found.spec.tse2e/react-start/basic/tests/navigation.spec.tse2e/solid-start/basic/vite.config.tse2e/solid-start/basic/tests/navigation.spec.tse2e/react-start/basic/playwright.config.tse2e/react-start/basic/vite.config.tse2e/solid-start/basic/playwright.config.ts
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/react-start/basic/tests/not-found.spec.tse2e/solid-start/basic/tests/redirect.spec.tse2e/react-start/basic/tests/search-params.spec.tse2e/react-start/basic/tests/redirect.spec.tse2e/solid-start/basic/tests/search-params.spec.tse2e/solid-start/basic/tests/not-found.spec.tse2e/solid-start/basic/package.jsone2e/solid-start/basic/server.jse2e/react-start/basic/package.jsone2e/react-start/basic/tests/navigation.spec.tse2e/solid-start/basic/vite.config.tse2e/solid-start/basic/tests/navigation.spec.tse2e/react-start/basic/server.jse2e/react-start/basic/playwright.config.tse2e/react-start/basic/vite.config.tse2e/solid-start/basic/playwright.config.ts
**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Use workspace:* protocol for internal dependencies in package.json files
Files:
e2e/solid-start/basic/package.jsone2e/react-start/basic/package.json
🧬 Code graph analysis (7)
e2e/solid-start/basic/tests/redirect.spec.ts (3)
e2e/react-start/server-functions/playwright.config.ts (1)
PORT(5-5)e2e/solid-start/server-functions/playwright.config.ts (1)
PORT(5-5)e2e/e2e-utils/src/derivePort.ts (1)
getTestServerPort(25-27)
e2e/react-start/basic/tests/search-params.spec.ts (1)
e2e/react-start/basic/tests/utils/isSpaMode.ts (1)
isSpaMode(1-1)
e2e/react-start/basic/tests/redirect.spec.ts (2)
e2e/e2e-utils/src/derivePort.ts (1)
getTestServerPort(25-27)e2e/react-start/basic/tests/utils/isSpaMode.ts (1)
isSpaMode(1-1)
e2e/solid-start/basic/server.js (1)
e2e/react-start/basic/server.js (8)
port(6-6)startPort(8-8)createStartServer(10-27)server(11-11)nodeHandler(12-12)app(14-14)app(30-30)createSpaServer(29-55)
e2e/react-start/basic/playwright.config.ts (5)
e2e/react-start/server-functions/playwright.config.ts (1)
PORT(5-5)e2e/solid-start/server-functions/playwright.config.ts (1)
PORT(5-5)e2e/e2e-utils/src/derivePort.ts (2)
getTestServerPort(25-27)getDummyServerPort(21-23)e2e/solid-start/basic/tests/utils/isSpaMode.ts (1)
isSpaMode(1-1)e2e/react-start/basic/tests/utils/isSpaMode.ts (1)
isSpaMode(1-1)
e2e/react-start/basic/vite.config.ts (1)
e2e/react-start/basic/tests/utils/isSpaMode.ts (1)
isSpaMode(1-1)
e2e/solid-start/basic/playwright.config.ts (2)
e2e/e2e-utils/src/derivePort.ts (2)
getTestServerPort(25-27)getDummyServerPort(21-23)e2e/solid-start/basic/tests/utils/isSpaMode.ts (1)
isSpaMode(1-1)
⏰ 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 (28)
e2e/react-start/basic/package.json (3)
10-10: LGTM! SPA build script follows the pattern.The build:spa script correctly sets MODE=spa and follows the existing build process.
12-12: LGTM! SPA start script references the custom server.The start:spa script correctly points to the custom server implementation.
21-22: Dependencies are secure and appropriately versioned.Both express@5.1.0 and http-proxy-middleware@3.0.5 are secure versions. The http-proxy-middleware version includes fixes for CVE-2025-32997 and CVE-2024-21536.
As per coding guidelines
e2e/solid-start/basic/tests/redirect.spec.ts (2)
10-10: LGTM! Consistent import pattern across test files.The import follows the established pattern for accessing SPA mode state in tests.
15-17: LGTM! SPA-aware port selection ensures correct test server.The conditional port naming ensures tests connect to the appropriate server instance (SPA vs SSR).
e2e/solid-start/basic/tests/not-found.spec.ts (2)
4-4: LGTM! Consistent import pattern.The import follows the established pattern for accessing SPA mode state.
18-18: LGTM! Correct status code handling for SPA vs SSR modes.In SPA mode, the server returns 200 for all routes since client-side routing handles not-found scenarios. This is the expected behavioral difference.
e2e/react-start/basic/tests/search-params.spec.ts (2)
30-32: LGTM! Correct handling of SPA mode redirect behavior.In SPA mode, redirects are handled client-side, so server redirect responses won't occur. The conditional logic correctly skips these assertions for SPA tests.
55-57: LGTM! Consistent redirect handling pattern.The conditional logic maintains consistency with the redirect behavior handling for SPA mode.
e2e/solid-start/basic/tests/search-params.spec.ts (2)
3-3: LGTM! Consistent import pattern.The import follows the established pattern for accessing SPA mode state in tests.
29-31: LGTM! Appropriate conditional logic for SPA mode.The conditional logic correctly handles the behavioral difference between SPA and SSR modes. In SPA mode, redirects are handled client-side, so server redirect responses won't occur.
Also applies to: 53-55
e2e/react-start/basic/tests/not-found.spec.ts (1)
18-18: Good: not-found status accounts for SPA fallback200 vs 404 based on SPA mode mirrors real server behavior. LGTM.
e2e/react-start/basic/tests/redirect.spec.ts (2)
15-17: LGTM: spa-aware PORT avoids collisions across modesSuffixing the derive key with _spa yields a distinct port. Nice.
10-10: Type check for JSON import assertionEnsure tsconfig for these tests has
resolveJsonModule: trueand a compatible module resolution (e.g.,"moduleResolution": "bundler"or"nodeNext") sowith { type: 'json' }compiles cleanly under tsc.Would you confirm tsconfig settings for the e2e React Start project?
e2e/react-start/basic/vite.config.ts (1)
24-25: LGTM: SPA wiring into tanstackStartPassing spa configuration via isSpaMode is correct.
e2e/solid-start/basic/vite.config.ts (1)
22-26: LGTM: SPA mode correctly plumbed into Solid Start pluginConfiguration matches the intended SPA build behavior.
e2e/solid-start/basic/package.json (2)
18-27: Deps look appropriate for SPA proxy serverexpress@^5 and http-proxy-middleware@^3 are suitable for the custom SPA server. No further changes.
12-12: server.js already uses ESM imports—no changes required
All imports useimportsyntax and norequire()calls were found.e2e/solid-start/basic/tests/navigation.spec.ts (2)
12-13: Good stability improvements with URL/load waitsThe added
waitForURL/networkidlepoints should reduce flakiness across SPA/SSR.Also applies to: 23-24, 33-34, 50-55, 60-64, 69-70, 79-80
54-55: No change needed—SSR omitsscript2.js, sowindow.SCRIPT_2beingundefinedis correct
Inscripts.tsx,isProdistrueduring the production build that SSR uses, causing theisProd ? undefined : { src: 'script2.js' }branch to dropscript2.jsentirely. Hence in SSR modewindow.SCRIPT_2is undefined as the tests assert.e2e/react-start/basic/playwright.config.ts (2)
9-14: Ports/env wiring LGTMDistinct PORT/START_PORT derivation and env exposure look good for SPA orchestration.
Also applies to: 47-48
17-17: Confirmed SPA scripts existe2e/react-start/basic/tests/navigation.spec.ts (2)
21-21: Stability waits look goodThe added
waitForURLandnetworkidlesyncs should reduce flakes across runs.Also applies to: 29-29, 45-45, 54-55, 57-57, 62-62, 71-71
49-50: Sanity-check SSR mode Confirm that your SSR setup does not includepublic/script2.js(sowindow.SCRIPT_2remains undefined).e2e/solid-start/basic/server.js (2)
32-46: Proxy targets/path joiningUsing
app.use('/api', proxy({ target: http://localhost:${startPort}/api }))is valid in Express (mount strips/api). No change required; just noting the intent is correct.
1-1: Import path correct: All e2e examples importtoNodeHandlerfrom'srvx/node'consistently; no change required.Likely an incorrect or invalid review comment.
e2e/solid-start/basic/playwright.config.ts (1)
17-17: Scripts and port bindings are correctly defined
build:spaandstart:spaare present in package.json, and Playwright’swebServerenv passesPORTandSTART_PORTintoserver.js, which listens on both ports.e2e/react-start/basic/server.js (1)
1-1: VerifytoNodeHandlerimport path
Ensure your installedsrvxversion actually exportstoNodeHandlerfromsrvx/node(e.g. check itspackage.json > exportsor inspect the package files). If that subpath isn’t available, switch to the known adapter invinxi/http:-import { toNodeHandler } from 'srvx/node' +import { toNodeHandler } from 'vinxi/http'
1 participant
This PR follows on #5262 and adds a custom server implementation to serve the spa mode production build for e2e tests on react-start-basic and solid-start-basic
Summary by CodeRabbit
New Features
Tests
Chores