Repository navigation
fix(rsc): resolve facade client entry with strictExecutionOrder - #1480
Merged
Merged
Conversation
james-elicx
force-pushed
the
fix/rsc-facade-client-entry
branch
from
October 2, 2026 12:57
1d5d337 to
97ba487
Compare
Copying `examples/starter` from a separate spec file could race with the `starter.test.ts` HMR tests editing it on the other worker, building a fixture with `Client [edit] Counter`. Colocate it so the copy runs serially after those edits are reset.
- Space out editor writes to the same file. Vite's chokidar watcher drops a `change` event within 50ms of the previous one, so a fast HMR round trip could swallow `editor.reset()` (e.g. `dev-no-ssr > client hmr`). - Wait for the `rsc:update` refetch before reloading in `use-cache-persistent`, so the reload neither aborts it (unhandled `Load failed` in WebKit) nor receives the late update before hydration (`setPayload is not a function`).
james-elicx
force-pushed
the
fix/rsc-facade-client-entry
branch
from
October 2, 2026 15:08
21d6ebb to
62fb48a
Compare
`setupIsolatedFixture` runs `pnpm i` over the network inside a `beforeAll` hook. On macOS runners installs take 16-34s, and the `react-server-dom-webpack` fixture installs twice, so the hook kept hitting the default 30s timeout.
james-elicx
marked this pull request as ready for review
October 2, 2026 16:07
|
I'm checking if it's possible to support |
Return the chunk-level map from `collectAssetDeps` alongside the module-id map, and find the `index` entry there. This reuses the deps already computed for every chunk instead of walking the entry again. Co-Authored-By: Claude Opus 5.5 <[email protected]>
The starter suite already fails on the client build without the fix, so it covers the regression. The removed test asserted rolldown's chunk shape and the internal assets manifest, which would break on bundler changes unrelated to the plugin. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Base the fixture on `examples/starter-extra` like other inline fixtures, so it no longer copies `examples/starter` while starter's dev tests edit it in place. `rolldown.test.ts` already has the same rolldown-only skip. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This was referenced Oct 5, 2026
hi-ogawa
approved these changes
Oct 5, 2026
hi-ogawa
left a comment
Contributor
There was a problem hiding this comment.
Fix looks good! Thanks for e2e fix too. I'm splitting changes for e2e to just sort out some polish.
…-entry Take main's e2e/fixture.ts and e2e/use-cache-persistent.test.ts. The e2e deflake fixes from this PR landed separately in vitejs#1487, with the write spacing moved to vitejs#1489.
hi-ogawa
approved these changes
Oct 5, 2026
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
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
With
output.strictExecutionOrder: trueon the client environment, whichcodeSplitting.experimentalInlineCommonChunksalso turns on by default, every build with at least one client component fails:Rolldown keeps the browser entry's modules out of the
indexchunk when they're shared with client reference chunks (React alone is enough).indexbecomes an empty facade that only imports and runs the shared chunk, e.g.import{t as e}from"./entry.browser-….js";e();. The assets-manifest hook finds the entry throughcollectAssetDeps, which maps module id to chunk. The facade has nomoduleIds, so it's never found.The fix looks up the
indexentry chunk directly in the bundle and walks its deps withcollectAssetDepsInner. For non-facade entries the result is the same as before. For a facade entry,clientEntryDepsnow starts at the facade and includes the shared chunk it imports, so modulepreload still covers the entry's code.I checked the other module-id lookups in this hook. Client reference groups are their own dynamic entries and keep their modules, so they're unaffected.
Tests
New
strict-execution-ordere2e block ine2e/starter.test.ts: an inlineexamples/starterfixture withstrictExecutionOrder: trueon the client environment. It's skipped on Rollup-based Vite (thevite-7job) the same waye2e/rolldown.test.tsis, since Rollup has nostrictExecutionOrder.defineStarterTestsuite in build mode: hydration, client component, server action with and without JS, CSS, and assets.indexentry (nomoduleIds, at least one import). The fixture records the chunk shape fromgenerateBundle. If a future rolldown stops producing this shape, the test fails instead of silently passing.clientEntryUrlpoints at the facade and thatclientEntryDeps.jsincludes the facade and the chunks it imports.It lives in
starter.test.tsrather than its own spec file on purpose. As a separate file it ran on the other worker and could copyexamples/starterwhiledev-default > client hmrhad it mid-edit, building a fixture withClient [edit] Counter.The commits are split to show red then green:
test(rsc): …adds only the test. The fixture build fails with the error above.fix(rsc): …makes the test pass: 8 passed, 3 HMR cases skipped in build mode.e2e/client-entry-url.test.tsstill passes.E2E race fixes
CI on this PR kept failing on existing dev tests that are also flaky on
mainnightlies, so I tracked two of them down (last commit):useCreateEditorwrites are spaced at least 100ms apart per file. Vite's chokidar watcher drops achangeevent that arrives within 50ms of the previous one for the same path. When the HMR round trip is fast,editor.reset()lands inside that window and Vite never sees it. A CI trace ofdev-no-ssr > client hmrshowed the reset written about 55ms after the edit, with only onehot updatedmessage. All three attempts got stuck onClient [edit] Counter: 1. A standalone probe against Vite's watcher reproduces the drop with a 20ms gap.use-cache-persistentwaits for thersc:updaterefetch beforepage.reload(). Previously the reload could abort the in-flight_.rscGET, which WebKit reports as an unhandledTypeError: Load failed. The late update could also reach the reloaded page before its effect assignedsetPayload, givingsetPayload is not a function. Delaying that GET by 300ms reproduces the WebKit failure locally; with the change it passes.setupIsolatedFixturegets 60s instead of the default 30s hook timeout. It runspnpm iover the network, which took 16-34s on macOS runners. Thereact-server-dom-webpackfixture installs twice, so itsbeforeAllregularly timed out, including onmainnightlies.Context
Found while trying rolldown's
codeSplitting.experimentalInlineCommonChunks(rolldown/rolldown#11040) in an RSC framework (vinext). The repo's rolldown 1.2.9 reproduces it withstrictExecutionOrderalone, so the test doesn't need the experimental option.