fix: client-safe next/constants shim and memoized hot-path dynamic imports#2663
Open
jlucaso1 wants to merge 11 commits into
Open
fix: client-safe next/constants shim and memoized hot-path dynamic imports#2663jlucaso1 wants to merge 11 commits into
jlucaso1 wants to merge 11 commits into
Conversation
commit: |
Contributor
Performance benchmarksCompared 0 improved · 3 regressed · 3 within ±1.5%
View detailed results and traces 🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head |
Member
|
Looks like there's a regression in bundle size. |
james-elicx
marked this pull request as draft
July 21, 2026 21:05
…ng survive Wrapping the flag-gated imports in memoizeModuleLoader at module scope defeated two Rolldown optimizations: dead branches behind HAS_CONFIG_* define-folding no longer eliminated their chunks before emission (config-matchers and config-headers shipped as orphan chunks nothing references), and hiding the import() inside a helper forced whole namespace preservation for the target modules (shims/headers.js went from a tree-shaken subset to a full unmangled namespace pulled eagerly into the RSC entry closure). Server bundle regressed +7.3% gzip. Keep the retry-on-failure semantics but with the import() expression inline at the call position, which Rolldown tracks through await (p ??= import(x).catch(reset)). memoizeModuleLoader remains for the generated entry thunks, whose targets are not flag-gated. Measured on the benchmark app: server bundle back to +0.5% of base (173.9k vs 173.0k gzip, was 185.6k), orphan chunks gone, RSC entry closure back to 7 files. entry-templates, shims, constants-shim and memoize-module-loader suites pass (1307 tests).
…ng survive Wrapping the flag-gated imports in memoizeModuleLoader at module scope defeated two Rolldown optimizations: dead branches behind HAS_CONFIG_* define-folding no longer eliminated their chunks before emission (config-matchers and config-headers shipped as orphan chunks nothing references), and hiding the import() inside a helper forced whole namespace preservation for the target modules (shims/headers.js went from a tree-shaken subset to a full unmangled namespace pulled eagerly into the RSC entry closure). Server bundle regressed +7.3% gzip. Keep the retry-on-failure semantics but with the import() expression inline at the call position, which Rolldown tracks through await (p ??= import(x).catch(reset)). memoizeModuleLoader remains for the generated entry thunks, whose targets are not flag-gated. Measured on the benchmark app: server bundle back to +0.5% of base (173.9k vs 173.0k gzip, was 185.6k), orphan chunks gone, RSC entry closure back to 7 files. entry-templates, shims, constants-shim and memoize-module-loader suites pass (1307 tests).
jlucaso1
force-pushed
the
fix/client-constants-and-per-request-imports
branch
from
July 21, 2026 22:50
0c77071 to
c796f39
Compare
jlucaso1
marked this pull request as ready for review
July 21, 2026 22:50
Author
Nice catch, I've submitted this commit to mitigate that: c796f39 Also rebased the PR. Thank you |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c796f39890
ℹ️ 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".
…//github.com/jlucaso1/vinext into fix/client-constants-and-per-request-imports
Projects may define NODE_ENV as production while running vite dev; the module runner still serves an HMR-enabled environment there, so the memoized SSR/RSC entry namespace would go stale after the first request. import.meta.env.PROD reflects the actual serve/build mode and is immune to user-provided NODE_ENV defines.
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
2 participants
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.
Closes #2661
Closes #2662
Two related production fixes, one commit each. Happy to split into separate PRs if you prefer.
1.
fix(shims): guard theprocessreference innext/constantsThe shim is a valid client import (it is aliased for every environment, and
@sentry/nextjsfor example reaches it from client code), butCONFIG_FILESreferencedprocessat module top level. Browsers have noprocessand optional chaining does not guard an undeclared identifier, so evaluating the module threwReferenceError: process is not definedand took down the sharedvinext-*client chunk. Real Next.js has the same expression but is saved by webpack'sprocesspolyfill, which Vite does not inject. Fixed with atypeof processcheck, identical behavior on any runtime that hasprocess.Verified with a
"use client"page importingPHASE_PRODUCTION_BUILDin theapp-basicfixture, production build, real Chrome: 3 page errors before, 0 after, same rendered output.2.
perf(server): memoize hot-path dynamic imports in productionSeveral
import()calls run on every request. The modules are cached by Node, but each call re-runs ESM resolution, and with loader hooks registered (Sentry, OpenTelemetry viamodule.register()) every resolution is a synchronous round trip to the hooks thread. On a production app with Sentry instrumentation this path alone was about 17% of process CPU.Counted with
diagnostics_channel.tracingChannel("module.import")inside the prod server ofapp-basic, 120 requests across 4 routes:config-matchersconfig-headersmetadata-route-responsethunkapp-page-cachefile-based-metadatathunkloadModule("ssr", "index")app-route-handler-dispatchthunkheaders.jsviaconnection()Internal framework modules are memoized unconditionally (safe in dev: when the importer is invalidated its module scope cache resets with it). The
import.meta.viteRsc.loadModule("ssr" | "rsc", "index")sites bundle user code, so those are memoized only whenprocess.env.NODE_ENV === "production", keeping the per call runner import that dev HMR relies on.headers.jsstays a dynamic import on purpose (the existinguse cacheconstraint), only the promise is cached.Tests
entry-templates(codegen assert updated accordingly),shims,app-router-production-server,app-router-production-build,app-flight-framing-production,app-router-dev-server,dev-route-discovery,vite-hmr-websocket,app-page-dispatch,app-page-render,metadata-route-build-dataandpages-routersuites pass, andtscis clean.