fix: retain content-addressed development CSS assets - #154
Conversation
commit: |
Benchmark results
|
Merging this PR will not alter performance
Comparing Footnotes
|
|
The CSS CI failure is fixed locally by disabling extract-loader fallback HMR through the NormalModule loader hook only in the web development environment where Router handles committed CSS manifests. No dependency source patch or test skip is used. All 15 CSS/lazy-loading matrix tests pass, including both Vanilla Extract state-preservation cases. Added exact edit/preload/restore coverage for Vanilla Extract; both variants pass. Build, full typechecking, and 844 core tests pass. A broader local HMR run has hit loader-update timeouts and remains under investigation; the PR stays draft while CI and broader validation run. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fbcf2a0a84
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const manifest = runtime.getCommittedManifest(); | ||
| if ( | ||
| sessions.getActiveBinding() === binding && | ||
| isHmrEnabled() && | ||
| manifest | ||
| ) | ||
| client.send('custom', manifestPayload(manifest)); |
There was a problem hiding this comment.
Replay the first commit to already-connected clients
When a browser reconnects during a dev-server recreation before the initial web/node generation commits, getCommittedManifest() returns undefined, so this callback sends nothing. The first commit also emits no manifest update because both cssManifestChanged and routeManifestMetadataChanged require a previous generation, leaving that client on the old session's manifest until another qualifying edit occurs; this can surface after route-topology restarts as stale routes or CSS. Retain such clients until the first commit or publish that initial commit to connected clients.
Useful? React with 👍 / 👎.
Fixes #153.
Restoring a CSS file to its original contents could make the browser reuse edited CSS from its preload cache. This change gives each version of development CSS a content-addressed URL and retains earlier assets so existing manifests keep serving the same bytes.
Changes
Validation
The build, workspace typechecks, all 849 core tests, and all five CSS/HMR/HDR browser regressions pass locally on
d91d6c8. This head also covers clients that connect before the first manifest commits.All nine CI checks passed on
d91d6c8, including the full framework and example-app end-to-end suites, package validation, CodeQL, and benchmarks.The restoration regressions check stylesheet contents, preserved form state, and browser errors. Six request-level regressions fail with the original compiler-pairing bypass and pass with this fix.
Temporary workaround
Remove
devCssOwnershipPluginonce every supported Rspack version includes the stylesheet-ownership fix. Re-run the restoration and async CSS HMR tests before removing it. Immutable CSS URLs and compiler pairing are still required.Development CSS aliases remain on disk until output cleanup or restart. Server-runtime reuse and on-demand SSR compilation remain separate work in #140 and #155.