test(rsc): add dev-mode HMR e2e for RSC routes with a co-located server fn - #7901
Open
sreetamdas wants to merge 3 commits into
Open
test(rsc): add dev-mode HMR e2e for RSC routes with a co-located server fn#7901sreetamdas wants to merge 3 commits into
sreetamdas wants to merge 3 commits into
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (18)
🚧 Files skipped from review as they are similar to previous changes (18)
📝 WalkthroughWalkthroughAdds a React Start RSC HMR E2E fixture supporting Vite and Rsbuild, with routed server-component examples, client state, CSS-module updates, isolated test-server lifecycle management, and Playwright tests covering state-preserving hot updates. ChangesRSC HMR fixture
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Playwright
participant HMRFileEditor
participant DevServer
participant Browser
Playwright->>Browser: Navigate and wait for hydration
Playwright->>Browser: Seed counter and reload sentinel
Playwright->>HMRFileEditor: Edit route or CSS fixture
HMRFileEditor->>DevServer: Write changed source
DevServer->>Browser: Deliver HMR update
Browser-->>Playwright: Report updated content and preserved state
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
View your CI Pipeline Execution ↗ for commit 5fab4fa
☁️ Nx Cloud last updated this comment at |
![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: 1
🧹 Nitpick comments (1)
e2e/react-start/rsc-hmr/package.json (1)
17-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
workspace:*for internal dependencies.The internal packages currently use
workspace:^, but the repository guideline requiresworkspace:*. Update@tanstack/react-router,@tanstack/react-start, and@tanstack/router-e2e-utils.Proposed change
- "`@tanstack/react-router`": "workspace:^", - "`@tanstack/react-start`": "workspace:^", + "`@tanstack/react-router`": "workspace:*", + "`@tanstack/react-start`": "workspace:*", ... - "`@tanstack/router-e2e-utils`": "workspace:^", + "`@tanstack/router-e2e-utils`": "workspace:*",Also applies to: 26-26
🤖 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/rsc-hmr/package.json` around lines 17 - 18, Update the internal dependency entries for `@tanstack/react-router`, `@tanstack/react-start`, and `@tanstack/router-e2e-utils` in package.json to use the workspace:* protocol instead of workspace:^, preserving all other dependency declarations.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/rsc-hmr/tests/rsc-hmr.spec.ts`:
- Around line 69-76: Remove the any casts used for __hmrReloadSentinel in the
setup and expectNoReloadHappened flows. Extend the Window type once with the
sentinel property, then access window.__hmrReloadSentinel directly while
preserving the existing sentinel behavior.
---
Nitpick comments:
In `@e2e/react-start/rsc-hmr/package.json`:
- Around line 17-18: Update the internal dependency entries for
`@tanstack/react-router`, `@tanstack/react-start`, and `@tanstack/router-e2e-utils` in
package.json to use the workspace:* protocol instead of workspace:^, preserving
all other dependency declarations.
🪄 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 Plus
Run ID: 077b829f-ab94-4e34-bd46-33b92439816b
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
e2e/react-start/rsc-hmr/.gitignoree2e/react-start/rsc-hmr/package.jsone2e/react-start/rsc-hmr/playwright.config.tse2e/react-start/rsc-hmr/rsbuild.config.tse2e/react-start/rsc-hmr/src/routeTree.gen.tse2e/react-start/rsc-hmr/src/router.tsxe2e/react-start/rsc-hmr/src/routes/-separateFile.tsxe2e/react-start/rsc-hmr/src/routes/__root.tsxe2e/react-start/rsc-hmr/src/routes/co-located-css-module.tsxe2e/react-start/rsc-hmr/src/routes/co-located.tsxe2e/react-start/rsc-hmr/src/routes/coLocatedCssModule.module.csse2e/react-start/rsc-hmr/src/routes/index.tsxe2e/react-start/rsc-hmr/src/routes/separate-file.tsxe2e/react-start/rsc-hmr/tests/rsc-hmr.spec.tse2e/react-start/rsc-hmr/tests/setup/global.setup.tse2e/react-start/rsc-hmr/tests/setup/global.teardown.tse2e/react-start/rsc-hmr/tsconfig.jsone2e/react-start/rsc-hmr/vite.config.ts
Comment on lines
+69
to
+76
| await page.evaluate(() => { | ||
| ;(window as any).__hmrReloadSentinel = 'alive' | ||
| }) | ||
| } | ||
|
|
||
| async function expectNoReloadHappened(page: Page) { | ||
| await expect | ||
| .poll(() => page.evaluate(() => (window as any).__hmrReloadSentinel), { |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the any escape from the reload sentinel.
Declare Window.__hmrReloadSentinel once and access it directly so this strict TypeScript fixture remains type-safe.
Proposed fix
+declare global {
+ interface Window {
+ __hmrReloadSentinel?: string
+ }
+}
+
async function seedClientState(page: Page, testIdPrefix: string) {
// ...
await page.evaluate(() => {
- ;(window as any).__hmrReloadSentinel = 'alive'
+ window.__hmrReloadSentinel = 'alive'
})
}
async function expectNoReloadHappened(page: Page) {
await expect
- .poll(() => page.evaluate(() => (window as any).__hmrReloadSentinel), {
+ .poll(() => page.evaluate(() => window.__hmrReloadSentinel), {📝 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
| await page.evaluate(() => { | |
| ;(window as any).__hmrReloadSentinel = 'alive' | |
| }) | |
| } | |
| async function expectNoReloadHappened(page: Page) { | |
| await expect | |
| .poll(() => page.evaluate(() => (window as any).__hmrReloadSentinel), { | |
| declare global { | |
| interface Window { | |
| __hmrReloadSentinel?: string | |
| } | |
| } | |
| async function seedClientState(page: Page, testIdPrefix: string) { | |
| // ... | |
| await page.evaluate(() => { | |
| window.__hmrReloadSentinel = 'alive' | |
| }) | |
| } | |
| async function expectNoReloadHappened(page: Page) { | |
| await expect | |
| .poll(() => page.evaluate(() => window.__hmrReloadSentinel), { |
🤖 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/rsc-hmr/tests/rsc-hmr.spec.ts` around lines 69 - 76, Remove
the any casts used for __hmrReloadSentinel in the setup and
expectNoReloadHappened flows. Extend the Window type once with the sentinel
property, then access window.__hmrReloadSentinel directly while preserving the
existing sentinel behavior.
Source: Coding guidelines
sreetamdas
force-pushed
the
test/rsc-hmr-e2e
branch
from
July 27, 2026 11:25
5fab4fa to
ad1d72f
Compare
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.
Description
Adds
e2e/react-start/rsc-hmr: a dev-mode HMR e2e project for TanStack Start RSC routes, covering route files that co-locate acreateServerFnwith the route component.Replaces #7621, which was a deliberate CI canary for a bug that has since been fixed upstream (vitejs/vite-plugin-react#1248, released in
@vitejs/plugin-rsc@0.5.30). This is the regression suite @schiller-manuel asked for on that PR instead.Built per his review there: a separate project rather than bolting onto
e2e/react-start/rsc, followinge2e/react-start/hmr's CI hardening, multiple routes including one with a CSS module and one with no CSS, and both the vite and rsbuild adapters.Why a Start-level test, given the fix was upstream
Upstream's own regression test (
plugin-rsc'sexamples/client-first) exercises the plugin in isolation. It does not go through Start's?tss-serverfn-splittransform, which is what puts a co-located route file into the rsc module graph in the first place, and it cannot cover the rsbuild adapter at all.Worth being precise about the two lanes:
@vitejs/plugin-rscis Vite-only, so the vite lane is what guards against this specific bug regressing. The rsbuild lane goes through Rspack /react-server-dom-rspackand is adapter-parity coverage — valuable, but a different code path.Layout
rsc-hmr=hmr's harness +rsc's configs:playwright.config.ts,tests/setup/global.{setup,teardown}.ts— frome2e/react-start/hmr, so it inheritswaitForServer+preOptimizeDevServer(cold compile and Vite's dep-optimize reload storm happen outside any test's timeout) andcreateHmrFileEditorwithnormalizeSource, so a run killed mid-edit self-heals instead of cascading.vite.config.ts/rsbuild.config.ts— frome2e/react-start/rsc, both withrsc: { enabled: true }.playwrightModes:vite-ssrandrsbuild-ssr.workers: 1and no shards, since the tests mutate fixture files on disk.mode: 'ssr'and then runs a dev server, matching whathmrdoes — the nx playwright plugin's mode allowlist is['ssr','spa','prerender','preview']with nodev. Happy to wire this differently if you'd prefer.Fixtures
/co-locatedcreateServerFn+ component in one file, no CSS — the original bug/co-located-css-module/separate-fileEach test seeds client state (a counter plus a
windowsentinel) before editing, then asserts the new text renders and the state survived — so a full reload that happens to render the right text fails rather than passes.The CSS cases are the ones I'd most want kept. The upstream guard decides whether to suppress client HMR by classifying importers as CSS vs non-CSS, so the interaction most at risk is a route with both kinds — in both directions: the component must still Fast Refresh, and a CSS-only edit must still hot-swap without a full reload. Both are asserted.
Verification
Run locally against both adapters:
E2E_TOOLCHAIN=vite— 4/4 passE2E_TOOLCHAIN=rsbuild— 4/4 passtsc --noEmit— cleanAnd, more usefully, the suite was checked against the broken plugin version. Pinning this project to
@vitejs/plugin-rsc@0.5.27and re-running gives:with the failure mode from the original report —
Received: "co-located-baseline", and only(rsc) hmr update /src/routes/co-located.tsx?tss-serverfn-splitin the terminal with no client update. The CSS-only edit and the separate-file control stay green, since neither was ever affected. So the suite fails specifically on this bug rather than merely passing on a fixed version.Note
0.5.27is inside the>=0.5.20peer range that@tanstack/react-start-rsccurrently declares, which is what #7900 addresses.Notes
rsc-hmrisprivate: trueand unpublished.nx.json,pnpm-workspace.yaml, or any workflow:e2e/react-start/*is globbed as a workspace, andscripts/nx/playwright-plugin.tssynthesises targets from thenx.metadata.playwrightModesblock in the project's ownpackage.json.-separateFile.tsx, deliberately without a.server.suffix — import protection treats those as server-only and swaps in a client mock, which breaks the route instead of exercising the workaround. Flagging it because TanStack Start (RSC): route component does not HMR when its file has a co-located createServerFn #7618's workaround wording suggests*.server.ts, which is misleading for a module a route imports directly.Summary by CodeRabbit