Repository navigation
fix(rendering): compile client and module output for the render mode - #3845
Conversation
|
Warning Review limit reached
Next review available in: 59 minutes Limit details: You’ve used all 1 included review currently available under your plan. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (20)
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 |
📦 Client bundle boundary
A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in |
Two transform call sites still hardcoded `dev: true`, so the hosted production path shipped development-compiled output: unminified, not tree-shaken, and carrying an inline sourcemap of the project source (`stages/compile.ts` derives `minify`, `treeShaking` and `sourcemap` from `dev`). Both are reached on every hosted production render. Site A, the client hydration bundle. `bundleComponentForClient` (`rendering/component-handling.ts`) had no mode parameter at all, and `page-renderer.ts` reaches it for every TSX page render that does not supply a cached client module. It now takes the compile mode, and `handleComponentPage` derives it from the render mode with the same `options?.mode === "development"` line the SSR half of the file already used. `buildComponentHydrationCacheHash` omitted the compile mode, so fixing the flag alone would let a development bundle be served to a production render; the hash now carries it. Site B, the MDX ESM module fetcher. `transformResolvedModuleSource` hardcoded `dev: true` and `createModuleFetcherContext` had no way to express the render mode, so every `/_vf_modules/*` import of every SSR module was compiled for development. The fetcher context now takes a compile mode, `resolveVfModuleImports` requires one, and the SSR module loader passes `this.options.dev`. The Site B cache identity had to change in the same commit. The compile mode was absent from `getTransformCacheKey`, which feeds the distributed transform cache, so entries are shared across requests and across instances with a TTL: fixing the transform without the key would have let one instance serve development-compiled modules to a hosted production render, which is worse than the current bug. The compile mode is now part of `getMdxModuleCacheVariant`, the single segment builder behind the distributed transform key, the module path-cache key, and the SSR loader and orchestrator lookups, so every writer and reader of those key spaces agrees. Development artifacts take an `on:compile-dev` segment and production keeps the unsegmented key; the MDX-ESM cache namespace is rolled in the same change, so the legacy entries written under the unsegmented key (all of them development-compiled) cannot be read back by a production render. Artifact filenames already hash the transformed code, so the on-disk layout is unchanged. The compiled-MDX entry path (`loader-helpers.ts`) carries no render mode yet and keeps the development compile mode it has always used. It is now explicit, and the compile mode in the cache identity keeps its artifacts isolated from the production-compiled ones. Refs veryfront/veryfront-issue-inbox#555
af861c2 to
6c82e90
Compare
|
@coderabbitai review |
|
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@coderabbitai review |
|
Follow-up to #3841, #3844 and #3845. Three items from the #3845 review are live on main. Site C, hosted production MDX pages. `loader-helpers.ts` passed an explicit `dev: true` for every `/_vf_modules/*` import of a compiled-MDX entry, and both `rendering/page-rendering.ts` and the RSC `render-handler.ts` reach that path on a hosted production render. `ESMLoaderContext` had no mode field, so the render mode could not be threaded. It has one now, both call sites pass it, and a context that names no mode compiles for production. Measured on a probe module: development emits 1031 bytes with an inline sourcemap, retained dead code and unminified identifiers; production emits 220 bytes with none of those, and the two land on different cache keys. The render cache identity. `buildRenderCachePrefix` took the project, the "preview" | "production" environment and the release key, none of which imply the compile mode. `isLocalProject` decides the compile mode and never reached the prefix, so a local development server and a hosted preview server for the same project and branch both produced `<project>:preview:main:<version>`. A cached render carries its hydration bundle (`cachedClientModule` in `handleComponentPage`, fed from `cachedResult.pageModule`), so a development-compiled bundle could be served straight out of the render cache to a production-mode render, around the hydration-cache fix in #3845. The prefix now carries a required compile-mode segment, `parseRenderCacheKey` reads it back, and both prefix builders derive it from the same value they already use for `mode`. Both modes gain the segment, so entries written under the old prefix shape cannot be misread after deploy; they age out like any `VERSION` roll. The cache-namespace invariant. `buildMdxEsmCacheSchemaSample` names the compile-mode variant only to roll `MDX_ESM_CACHE_NAMESPACE`, and that roll is the only thing keeping a production render off a legacy, always development-compiled entry. Every test referenced the namespace symbolically, so deleting the line reopened the hole silently. The sample builder is now exported and a test rebuilds the pre-roll namespace from it, then asserts no current production key can reach that namespace. It asserts the isolation property, not the hash. Tests now pin the compile mode through the five #3845 source files that nothing detected a regression in. Reverting each one to its pre-#3845 state fails at least one test. Refs veryfront/veryfront-issue-inbox#555
Summary
This is a production behaviour change. Two transform call sites still hardcoded
dev: trueafter #3841, so the hosted production render path shipped development-compiled output on every request. Persrc/transforms/pipeline/stages/compile.ts,dev: truemeansminify: false,treeShaking: falseand an inline sourcemap, so production paid a payload-size cost and disclosed the project source. Both sites are fixed here, and each one's cache identity is updated in the same commit.Refs veryfront/veryfront-issue-inbox#555
Site A: the client hydration bundle
bundleComponentForClient(src/rendering/component-handling.ts) hardcodeddev: trueand had no mode parameter.src/rendering/page-renderer.tsreaches it on every TSX page render that does not supply a cached client module.bundleComponentForClienttakes the compile mode, defaulting to production.handleComponentPagederives it from the render mode with the sameoptions?.mode === "development"line the SSR half of the file already used.Cache identity, updated alongside the flag.
buildComponentHydrationCacheHashdid not include the compile mode. Fixing the flag alone would let a development bundle be served to a production render, or the reverse, because both modes resolve to one hydration cache entry. The hash now carries the compile mode.Site B: the MDX ESM module fetcher
transformResolvedModuleSource(src/transforms/mdx/esm-module-loader/module-fetcher/source-transform.ts) hardcodeddev: true, andcreateModuleFetcherContexthad no way to express the render mode. The chain isssr-module-loader/loader.tstovf-module-resolver.tstocreateModuleFetcherContexttofetchAndCacheModule, and it fires for every/_vf_modules/*import of every SSR module.resolveVfModuleImportsrequires one, so a caller cannot silently omit it.this.options.dev.Cache identity, mandatory in the same commit.
getTransformCacheKeyhad no compile-mode segment, and it feeds the distributed transform cache, whose entries are shared across requests and across instances with a TTL. Fixing the transform without the key would have been worse than the original bug: one instance could serve a development-compiled module to a hosted production render, and the reverse.The compile mode is now part of
getMdxModuleCacheVariant, the single segment builder behind the distributed transform key (getTransformCacheKeytobuildMdxEsmTransformCacheKey), the module path-cache key (getVersionedPathCacheKeyandcacheModule), and the SSR loader and render orchestrator lookups. All of these share one key space, so every writer and reader of it had to agree; a partial fix would have left the same cross-mode reuse through the path cache, which short-circuits before any transform runs.Development artifacts take an
on:compile-devsegment and production keeps the unsegmented key. The MDX-ESM cache namespace is rolled in the same change (the compile-mode split is named in the namespace schema sample), so the legacy entries written under the unsegmented key, all of them development-compiled, cannot be read back by a production render.Artifact filenames already hash the transformed code, so the two modes cannot collide on disk and the artifact directory layout is unchanged.
Scope
The compiled-MDX entry path (
esm-module-loader/loader-helpers.ts) carries no render mode yet, so it keeps the development compile mode it has always used. That is now explicit rather than implied by a hardcoded literal, and the compile mode in the cache identity keeps its artifacts isolated from the production-compiled ones. Threading a render mode intoMDXRenderer.loadModuleESMis a separate change.No
RenderContext.modevalue ("development" | "production") is forwarded into a"preview" | "production"field. Only the compile-mode boolean is threaded.Tests
src/rendering/component-handling.test.ts: a production-modehandleComponentPagerender emits a hydration bundle with no inline sourcemap, minified and tree-shaken; a development-mode render still emits the debuggable bundle. Both assert on the emitted code. A third case pins that the two modes take separate hydration cache entries.src/modules/react-loader/ssr-module-loader/vf-module-resolver.test.ts: a development resolve runs first, then a production resolve of the same module for the same project and content source. The production render gets a different artifact with no inline sourcemap and no dead code, which fails if either the flag or the cache identity is missing.src/modules/react-loader/ssr-module-loader/loader.test.ts: an end-to-endSSRModuleLoaderproduction load of a module with a/_vf_modules/import, asserting the cached artifact.src/transforms/mdx/esm-module-loader/module-fetcher/cache-keys.test.ts: the compile mode changes the distributed transform key and the path-cache key, an unset mode reads as production, and the compile segment stays separate from the pin and server-external segments.src/transforms/mdx/esm-module-loader/module-fetcher/source-transform.test.ts: the requested compile mode reaches the transform.Each new behaviour test was confirmed to fail against the unfixed code.
Verification