fix: eliminate react-dom/server.edge dual-import Rollup warning in SSR entry#41
Conversation
|
/bigbonk review |
There was a problem hiding this comment.
Clean, correct fix. The SSR entry was already statically importing renderToReadableStream from react-dom/server.edge, so adding renderToStaticMarkup to the same import and dropping the redundant dynamic import() is the right call. The dynamic import provided no lazy-loading benefit since the module was already eagerly loaded.
Verified:
- No other dynamic imports of
react-dom/server.edgeremain in the file - No other server files (
prod-server.ts,worker-entry.ts) userenderToStaticMarkup, so there's no parity concern - The remaining
await import("react")on line 2257 is fine —reacthas no static import in the SSR entry, so it won't trigger the same Rollup warning
One minor nit below, but not blocking.
|
Review submitted. Here's a summary: Approved — This is a clean, correct fix. The change hoists Key verification points:
Left one optional nit suggesting the 3-line explanatory comment could be trimmed to a single line. |
1ab00bb to
a8406ba
Compare
…ort warning The generated SSR entry imported renderToReadableStream statically from react-dom/server.edge at the top level, but later used a dynamic import() for renderToStaticMarkup from the same module. Rollup detects this mixed static+dynamic import pattern and emits a warning for every build: "react-dom/server.edge" is imported as both a static and dynamic import in the same module Fix: add renderToStaticMarkup to the existing static import, removing the redundant dynamic import(). The module was already loaded — the dynamic import added no lazy-loading benefit since it runs unconditionally during SSR.
a8406ba to
23417dd
Compare
Removed comment about static import of renderToStaticMarkup.
|
LGTM! Thanks @nianyi778 |
commit: |
Problem
The generated SSR entry module (
generateSsrEntry()) importsreact-dom/server.edgein two ways:import { renderToReadableStream } from "react-dom/server.edge"handleSsr):const { renderToStaticMarkup } = await import("react-dom/server.edge")Rollup detects this mixed static+dynamic import pattern and emits a warning on every production build:
This is cosmetic (the build succeeds), but it's noisy — especially for projects that suppress it via
onwarnworkarounds in theirvite.config.ts.Fix
Add
renderToStaticMarkupto the existing static import at the top of the generated SSR entry, and remove the redundant dynamicimport(). The module was already loaded — the dynamic import added no lazy-loading benefit sincehandleSsr()runs unconditionally during SSR andrenderToReadableStreamfrom the same module is always available.Verification
Tested with a vinext App Router project (Cloudflare Workers):
vinext buildtsccompiles without errors