-
Notifications
You must be signed in to change notification settings - Fork 327
fix(build): avoid __dirname shim conflicts and cover node_modules #1385
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,150 @@ | ||
| import type { Plugin } from "vite"; | ||
| import { maskStringsAndComments } from "../utils/mask-source.js"; | ||
|
|
||
| /** | ||
| * Detect whether the source contains a bare reference to `__dirname` or | ||
| * `__filename` outside of strings, template literals, and comments. Mirrors | ||
| * the conservative "skip masked spans first" strategy used by | ||
| * {@link findUserDeclaredCjsGlobals} so a docstring like | ||
| * `/* uses __dirname *\/` doesn't force the shim into modules that don't | ||
| * actually need it. | ||
| * | ||
| * The return value reports each identifier independently so the injected | ||
| * preamble can hoist only the bindings actually used by the module — keeping | ||
| * the per-module patch size minimal and avoiding spurious imports. | ||
| */ | ||
| export function detectCjsGlobalReferences(source: string): { | ||
| __dirname: boolean; | ||
| __filename: boolean; | ||
| } { | ||
| // Quick reject: the strict masked scan below is comparatively expensive on | ||
| // hot paths (every node_modules .js/.mjs/.cjs goes through the transform | ||
| // pipeline), so first verify the source contains the identifier at all. | ||
| if (!/__dirname|__filename/.test(source)) { | ||
| return { __dirname: false, __filename: false }; | ||
| } | ||
|
|
||
| const masked = maskStringsAndComments(source); | ||
|
|
||
| return { | ||
| __dirname: /\b__dirname\b/.test(masked), | ||
| __filename: /\b__filename\b/.test(masked), | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Per-module preamble that defines `__dirname` / `__filename` for ESM-bundled | ||
| * CommonJS code. Used as the body of the {@link cjsGlobalsShimPlugin} | ||
| * transform output. | ||
| * | ||
| * The bindings are scoped to the transformed module (Vite's `transform` | ||
| * runs before bundling, so each module's source is patched independently). | ||
| * After Rolldown concatenates modules, the bindings become unique top-level | ||
| * `const`s in the bundle thanks to the bundler's identifier renaming. | ||
| * | ||
| * `import.meta.url` resolves to the *bundle's* output URL at runtime, not | ||
| * the original `node_modules` location. That is intentional: the goal of | ||
| * this shim is to prevent `ReferenceError: __dirname is not defined` for | ||
| * libraries that touch these identifiers defensively (e.g. `typeof | ||
| * __dirname === "string"`, log paths, debug strings). Libraries that load | ||
| * sibling assets through `__dirname` need to be externalized instead — a | ||
| * shim cannot recreate the original on-disk layout post-bundle. | ||
| * | ||
| * The helper imports are aliased so they cannot collide with user-declared | ||
| * names like `dirname` or `fileURLToPath` already imported by the module. | ||
| */ | ||
| export function buildCjsGlobalsShimPreamble(needs: { | ||
| __dirname: boolean; | ||
| __filename: boolean; | ||
| }): string { | ||
| if (!needs.__dirname && !needs.__filename) return ""; | ||
|
|
||
| const lines: string[] = [`import { fileURLToPath as __vinext_fileURLToPath } from "node:url";`]; | ||
| if (needs.__dirname) { | ||
| lines.push(`import { dirname as __vinext_dirname } from "node:path";`); | ||
| } | ||
| if (needs.__filename) { | ||
| lines.push(`const __filename = __vinext_fileURLToPath(import.meta.url);`); | ||
| } | ||
| if (needs.__dirname) { | ||
| // Reuse the local __filename when we already declared it, otherwise | ||
| // compute it inline. Avoids declaring an unused __filename binding. | ||
| if (needs.__filename) { | ||
| lines.push(`const __dirname = __vinext_dirname(__filename);`); | ||
| } else { | ||
| lines.push(`const __dirname = __vinext_dirname(__vinext_fileURLToPath(import.meta.url));`); | ||
| } | ||
| } | ||
| return lines.join("\n") + "\n"; | ||
| } | ||
|
|
||
| /** | ||
| * Vite plugin that injects local `__dirname` / `__filename` shims into | ||
| * third-party `node_modules` code that references those CJS globals but is | ||
| * bundled into an ESM server output (`"type": "module"`). | ||
| * | ||
| * Why: vinext's production server build emits ESM (`output.format: "esm"`, | ||
| * `package.json#type === "module"`). The Node ESM module scope does not | ||
| * expose the CJS-style `__dirname` / `__filename` globals, so any | ||
| * `node_modules` dependency that touches them — even defensively, like | ||
| * `typeof __dirname === "string"` or for log messages — crashes the bundled | ||
| * worker with `ReferenceError: __dirname is not defined in ES module scope`. | ||
| * | ||
| * vinext already injects these shims into the user's `next.config.ts` | ||
| * (see `config/next-config.ts`), but that transform is keyed to the config | ||
| * file path. Third-party packages bundled into the server entry need the | ||
| * same treatment, scoped per-module so user code declaring its own | ||
| * `__dirname` is unaffected. | ||
| * | ||
| * Scope: | ||
| * - Only `node_modules` files (user code stays untouched — user-source | ||
| * `__dirname` is a real bug we want to surface, not silently shim). | ||
| * - Only `.js` / `.mjs` / `.cjs` extensions (TypeScript files inside | ||
| * `node_modules` are unusual and typically pass through their own | ||
| * compiler first). | ||
| * - Only server-side environments (`ssr`, `rsc`). The browser environment | ||
| * doesn't ship CJS globals at all and clients almost never reference | ||
| * them — adding a shim there would inject `node:url` imports that the | ||
| * browser-target build would have to externalize. | ||
| * | ||
| * Limitation: `import.meta.url` after bundling points at the *bundle*, not | ||
| * the original module location. Libraries that load sibling files through | ||
| * `__dirname` (native `.node` addons, embedded `.wasm`, etc.) should be | ||
| * externalized via `next.config.serverExternalPackages` instead. This shim | ||
| * only prevents the `ReferenceError`; it does not preserve filesystem | ||
| * semantics. | ||
| */ | ||
| export const cjsGlobalsShimPlugin: Plugin = { | ||
| name: "vinext:cjs-globals-shim", | ||
| enforce: "pre", | ||
|
|
||
| transform: { | ||
| filter: { id: /\/node_modules\/.+\.(?:m?js|cjs)(?:\?.*)?$/ }, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Plugin ordering confirmation: This plugin runs with Also worth noting: |
||
| handler(code, id) { | ||
| // Server-only: see plugin docstring. The Vite environment API exposes | ||
| // the current environment name to plugin handlers via `this.environment`. | ||
| const envName = this.environment?.name; | ||
| if (envName !== "ssr" && envName !== "rsc") return null; | ||
|
|
||
| // `id` is consumed by the Vite filter (the regex above admits ids | ||
| // with optional `?…` cache busters) and is not used inside the | ||
| // handler body — the transform only operates on the source text. | ||
| void id; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
nit: consider |
||
|
|
||
| const needs = detectCjsGlobalReferences(code); | ||
| if (!needs.__dirname && !needs.__filename) return null; | ||
|
|
||
| const preamble = buildCjsGlobalsShimPreamble(needs); | ||
| return { | ||
| code: preamble + code, | ||
| // Source map is intentionally null: the preamble is prepended without | ||
| // line offsets in the original source map. A precise map would | ||
| // require MagicString plumbing; the trade-off here favours zero | ||
| // overhead in the hot transform path. Stack traces still resolve to | ||
| // the original source via the bundler's combined sourcemap once the | ||
| // module is concatenated. | ||
| map: null, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Source map trade-off is reasonable but worth noting: when the preamble is multi-line (e.g., 4 lines for both If this ever becomes a debugging pain point, |
||
| }; | ||
| }, | ||
| }, | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| /** | ||
| * Source-masking helpers shared by the CJS-global shim plugins. | ||
| * | ||
| * Both the `next.config.ts` injector (`config/next-config.ts`) and the | ||
| * `node_modules` shim (`plugins/cjs-globals-shim.ts`) need to scan source | ||
| * code for bare identifier references / declarations *without* matching | ||
| * tokens that happen to appear inside string literals, template literals, | ||
| * or comments. They originally inlined the same regex; this module owns | ||
| * the canonical version so the two callers cannot drift apart. | ||
| */ | ||
|
|
||
| /** | ||
| * Single alternation that captures (and consumes) each kind of span we | ||
| * want to ignore. Order matters: the block-comment branch must come | ||
| * before the line-comment branch so `/* ... *\/` containing `//` is | ||
| * matched as one block, and string-literal branches must consume the | ||
| * full literal so embedded `*\/` or `//` don't terminate the wrong span. | ||
| * | ||
| * Template literals are matched segment-by-segment, stopping before | ||
| * `${` so the interpolation contents themselves remain visible to the | ||
| * scan (`__dirname` inside `${__dirname}/views` is a real reference). | ||
| */ | ||
| const MASK_REGEX = | ||
| /\/\*[\s\S]*?\*\/|\/\/[^\n]*|`(?:[^`\\$]|\\.|\$(?!\{))*`|"(?:[^"\\]|\\.)*"|'(?:[^'\\]|\\.)*'/g; | ||
|
|
||
| /** | ||
| * Replace strings, template literals, and comments in `source` with an | ||
| * equal-length run of spaces. Preserves byte/character offsets so the | ||
| * returned string can be scanned with regexes whose match indices line up | ||
| * with positions in the original source — useful when callers later need | ||
| * to extract surrounding context (e.g. statement boundaries). | ||
| * | ||
| * Template-literal *expressions* (`${...}`) remain visible after masking | ||
| * because the regex stops before `${`. Identifiers used inside an | ||
| * interpolation are real references and must not be hidden. | ||
| */ | ||
| export function maskStringsAndComments(source: string): string { | ||
| return source.replace(MASK_REGEX, (m) => " ".repeat(m.length)); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor note on the
declStmtregex./\b(const|let|var)\b([^;]*)/gcaptures everything up to the next;. This works for the common case, but relies on semicolons as statement boundaries. Two observations:ASI-eligible code without semicolons — if the user writes
const __dirname = dirname(...)without a trailing;, the[^;]*consumes past it into subsequent lines. This is actually still correct (it just captures more text than one statement), so no bug.for-loop initializers like
for (let __dirname = 0; ...)— the[^;]*stops at the first;in the for-header, capturinglet __dirname = 0and reporting a false positive. In practice nobody writesfor (let __dirname ...)so this is purely academic.No action needed — failure modes are safe or unrealistic. The docstring already covers the important invariant (false negatives surface the collision error, false positives skip the shim → ReferenceError). Just flagging for completeness.