Skip to content

bundler: spell import.meta.require as the require of node, CommonJS and dev-server output - #46299

Open
robobun wants to merge 1 commit into
mainfrom
robobun/c30c2b9e/lower-import-meta-require
Open

robobun wants to merge 1 commit into
mainfrom
robobun/c30c2b9e/lower-import-meta-require

Conversation

@robobun

@robobun robobun commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #9416

Problem

  • A second bun build over a file that bun build --target=bun wrote exits 0, and the output fails at start: TypeError: __require is not a function (for node), SyntaxError: import.meta is only valid inside modules (as cjs).
  • Bun writes require there as var __require = import.meta.require; (src/bundler/ParseTask.rs:422). The EImportMeta arm (src/js_parser/fold.rs:597) leaves it as written for every output.

Fix

  • That arm returns a new node, E::Special::ImportMetaRequire, in CommonJS output for Bun or node, in ES-module output for node, and on the dev server. The printer spells it as that output's require: require, __require, hmr.require.
  • The node is not the parser's require: nothing is resolved at build time.
  • The sqlite loader's module calls require in CommonJS output (bundler: use require() for embedded sqlite modules in cjs output #38173).
  • Verified: test/bundler/bundler_bun.test.ts (54 new tests, 43 fail on main), dev-and-prod.test.ts, react-compiler.test.ts, and all of test/bundler/. Self-reviewed: 23 concerns raised, 22 addressed (Notes).

Background

Downsides

  • In output for node, a test of import.meta.require now finds Bun, and import.meta.require("bun:sqlite") behind it throws. --define import.meta.require=undefined gives the old value.
  • cjs output with no require ran on main when node loaded it as an ES module ("type": "module"). It throws now. Notes have two more shapes.
  • Release binary text: +768 B. +2 instructions per import.meta.x.
Notes

The reports, each run on main (9b4284f) and on this branch

# #9416: a library for Bun, then an app for node over it
echo 'export const events = require("events");' > first.js
bun build --target=bun first.js --outfile=second.js     # writes: var __require = import.meta.require;
echo 'console.log(typeof require("./second").events.EventEmitter);' > third.js
bun build --target=node third.js --outfile=out.mjs && node out.mjs
# main: TypeError: __require is not a function     branch: function

# The same kind of library, then an app as cjs for Bun
mkdir -p src && echo 'export function sep() { return require("node:path").sep; }' > src/index.js
bun build --target=bun src/index.js --outdir=dist
echo 'import { sep } from "./dist/index.js"; console.log("app: " + sep());' > app.js
bun build --format=cjs --target=bun app.js --outdir=out && bun out/app.js
# main: SyntaxError: import.meta is only valid inside modules.     branch: app: /

What changes, by output

Build and output import.meta.require prints as
for Bun, ES module import.meta.require (no change)
for Bun, CommonJS (also --bytecode) require
for node, ES module __require, which is createRequire(import.meta.url)
for node, CommonJS require
a file for the browser, every format import.meta.require (no change)
dev server, both sides hmr.require
iife import.meta.require (no change: iife output has no __require for a value, #38077)
  • Only bun build with bundling. --no-bundle and Bun.Transpiler do not change.
  • The rule is one option for each file, set in ParseTask.rs. In CommonJS output the target of the file decides: a file for Bun or node has require. In an ES module the target of the build decides as well: the __require of a bundle is createRequire only in a build for node. The parser puts the node in place of import.meta.require when it is not assigned and not deleted. import.meta?.require and const { require } = import.meta are other forms. They stay as written.
  • An entry that starts with #!/usr/bin/env bun has Bun as its target in every build on main (target_from_hashbang, which bundler: apply an explicit target to a bun hashbang entry point #38598 changes). In a build with no --target its ES-module output does not change, and its CommonJS output is lowered: that chunk is Bun's CommonJS wrapper. In a build for node it is lowered like its dependencies, so ES-module output of a command-line tool that Bun built runs under node as the first entry too.
  • A --define or a --drop of the dotted import.meta.require wins. A define reaches the import.meta["require"] spelling only with --minify-syntax, as for every define on main.
  • The node stays in the tree until the printer. It has no side effects. A declaration or a top-level statement of it that nothing uses goes, and the __require of the bundle goes with it. A nested statement import.meta.require; prints as __require; and keeps the createRequire line, as a nested require; does. On main that statement is removed.
  • A use of the runtime's __require is recorded with the node, as for require. --minify-syntax can put import.meta in place of a name and visit the access again (const m = import.meta; return m.require || null). That visit cannot record a use. The access stays on import.meta there, as in the build that is not minified.
  • After a read of an import.meta.hot method that is not a call (const on = import.meta.hot.on), main leaves the parser flag for dropped calls set until the next call (js_parser: set the import.meta.hot call-drop flag only for a call target #36875). The arm reads that flag for --drop. An import.meta.require in that window stays as written.
  • The sqlite loader builds import.meta.require(path, { type: "sqlite" }).db with no visit pass. In CommonJS output it now builds the call on require. That is the arm of bundler: use require() for embedded sqlite modules in cjs output #38173.

Dev server

  • Both sides are lowered. The TODO in test/bake/dev-and-prod.test.ts asked for hmr.require. The value is the hmr.require that require gives there. Before, it was a function bound to the module.
  • On the server side import.meta prints as hmr.importMeta. That getter reads location since fix import.meta.url in hmr  #21386 and throws ReferenceError: location is not defined. The dotted import.meta.require does not go through the getter now. Every other form (typeof import.meta, const { require } = import.meta, import.meta.env) still throws. This change does not repair the getter.
  • hmr.require(name) knows registered modules only. import.meta.require("node:path") in a route fails at the call with Failed to resolve dynamic import 'node:path'.

What does not change

Behaviour changes for hand-written code in output for node, and in cjs output for Bun

  • import.meta.require is the require of the output at run time. typeof gives "function" and a truthiness test passes. Under node on main it was undefined.
  • import.meta.require("x"), .resolve("x") and .main read require at run time. No string is resolved or bundled at build time.
  • An assignment to import.meta.require and a delete of it stay on import.meta. A call after such an assignment calls require.
  • ES-module output for node that has import.meta.require anywhere, also behind a test for Bun, starts with import { createRequire } from "node:module" and createRequire(import.meta.url), as for a require() there. A guarded call alone grows the output from 120 B to 220 B. A module that is loaded from a data: URL throws on that line (The argument 'filename' must be a file URL object).
  • --define import.meta.require=undefined gives the value of main, for the whole build.

Output that ran on main and throws now

Output that fails in another way now

Not in this change

Workaround on released versions

  • cjs output: --define import.meta.require=require. On main it gives the bytes that this change prints for the forms that Bun writes.
  • node output: --banner 'import.meta.require??=(await import("node:module")).createRequire(import.meta.url);'.
  • The default needs no diagnosis after a build that exits 0. A flag also reaches the browser files of a server build.

Measurements (release builds of merge base 9b4284f and of this branch, one toolchain, one machine)

  • size: text 107,318,912 to 107,319,680 (+768 B). Data and bss are equal. There is no new function. The largest deltas: get_ast +170 B (the sqlite arm), maybe_rewrite_property_access 2 x +118 B, run_from_thread_pool +109 B (the rule), NonLocalBinding::name +73 B.
  • For each import.meta.url access in maybe_rewrite_property_access (qemu slope, 200 against 400 sites): 85 to 87 instructions. The visitor around it: 220 to 220.
  • For each require("lit") site: visitor 382 to 382, printer 125 to 125. For each external require record in the printer: 107 to 107.
  • visit_expr_in_out on a bundle with no require and no import.meta: 168,508 against 168,508 guest instructions (JS), 1,522 against 1,522 (TS).
  • Expr 16 B, E::Special 8 B, the parser Options 392 B and P 4,056 B are unchanged. The new bool is in padding.
  • A 301-module project with no import.meta.require: mimalloc entries 63,800 / 63,783 / 63,756 on the merge base and 63,773 / 63,764 / 63,741 here.
  • Output with no import.meta.require: 0 of 155 bundles differ (52 npm packages, for bun, node and browser, 291.7 MB), and 0 of 199 --no-bundle outputs differ. Of 8 packages that have it, the 16 builds for Bun and for the browser are identical, and 7 of the 8 builds for node differ.
  • 30 forms that Bun writes x 13 cells of target, format and flag: 122 of 390 build and run (main 57). No cell that runs on main fails. The 65 new cells are node esm, node esm with --splitting, bun cjs, node cjs and --compile --bytecode, 13 each. The rest needs the reader.
  • Wall clock, 20,000-module build, 30 alternating runs: median 720 ms on the merge base and 704 ms here (p25 to p75: 573 to 840 and 598 to 772).

Tests

  • test/bundler/bundler_bun.test.ts: 54 new tests. 43 fail on main. The other 11 pin what must stay as it is: ES-module output for Bun, a build for the browser as esm and as cjs, iife, --define (two), --drop, a value that nothing uses, a name of import.meta under --minify-syntax, a build for Bun with a page, a #!/usr/bin/env bun entry with the default target.
  • import.meta.require is not a function error after target: "node" build #9416 with the real writer, and the tests of bundler: lower import.meta.require for non-bun targets #35662 with its dev-server expectation.
  • Five forms that Bun wrote for Bun (the helper, the helper in a shared chunk, the helper inside __esm, the helper beside a direct eval, the inline call of Bun 1.0.0 to 1.1.42) for node as esm and for bun and node as cjs. Three real writers for the same cells.
  • Node output runs under node and under Bun in 14 tests.
  • Hand-written code for node: a guarded call in a function, .resolve(), typeof, a truthiness test, a folded callee, a call by name under --reject-unresolved, each with and without minifySyntax. An assignment, a delete, --splitting with two entries, a #!/usr/bin/env bun entry as esm for node and as cjs.
  • A server build with a page, in three cells: the browser file stays as written and the server file is lowered.
  • The sqlite module as cjs, behind a dynamic import, and under --compile --bytecode.
  • test/bake/dev-and-prod.test.ts: one expectation changes, and a route pins hmr.require on the server side. test/bundler/transpiler/react-compiler.test.ts: the compiler keeps the node, and a join of it with module.exports keeps both.
  • Mutants: 23 mutants of the arm, the rule, the printer and the React Compiler arms, on an earlier tree of this branch. 21 failed a test. The two that survived were guards that no output shows: the exclusion of the runtime file, and an arm that dropped an unused value in a nested statement. Both are deleted. The rule changed after that run. Each of its conditions has a test that needs it: TargetBrowser-cjs, InBrowserFileOfServerBuild, InBunHashbangEntryAsCjs, TargetBun#9416 and BehindBunHashbangForNode.
  • All 120 test files of test/bundler/ and test/bake/ on the release builds of main and of this branch: no test fails only here. What fails is a 5 s limit on this machine: bundler_bytecode_portable (2) and native-plugin (1) on both builds.
  • On the debug build: the three files above, and bundler_splitting, bundler_cjs2esm, bundler_edgecase, bundler_minify, bundler_plugin, test/bake/dev/bundle and test/bake/dev/esm. Two tests hit a time limit of this machine there and pass on the release build: splitting/FoldingNeverMakesABundleWorse: graph 47 and edgecase/DeepImportDiamondDAG.

Earlier work

Self-review

  • The review of the first cut raised 23 concerns. 22 are addressed. Not taken: the repair of the dev server's importMeta getter. It throws for every form of import.meta on the server side, and the right url for a server-side module is a decision for that code. It is its own change.
  • Taken in another form: the node has no payload. The --splitting test has its two entries in one directory, because the other layout is the defect of the shared __require on main. Of the two import.meta.hot sequences the test pins the one after a call. The one after a read prints what js_parser: set the import.meta.hot call-drop flag only for a call target #36875 changes. There is no test for cjs output beside another member of import.meta: no host runs that output.
  • A second check of the final diff found that cjs output of a build for the browser was lowered, also for the module script of an HTML entry. Now a file for the browser stays as it is in every format.

no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/bundler/transpiler/react-compiler.test.ts, test/bake/dev-and-prod.test.ts

…JS and dev-server output

`bun build --target=bun` writes `require` as `var __require = import.meta.require;`.
A second build over such a file for node, or as CommonJS, left that line as
written: `TypeError: __require is not a function` under node, and
`SyntaxError: import.meta is only valid inside modules` as CommonJS.

The `EImportMeta` arm now returns `E::Special::ImportMetaRequire` for a file for
Bun or node in CommonJS output, in an ES module of a build for node, and on the
dev server. The printer spells the node like `ERequireCallTarget`: `require`,
the `__require` of the runtime, `hmr.require`. ES-module output for Bun, iife
output and files for the browser keep `import.meta.require`.

The module that the sqlite loader writes calls `require` in CommonJS output.

Co-authored-by: chloe caruso <git@paperclover.net>
@coderabbitai

coderabbitai Bot commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 18 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: oven-sh/bun/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 00b86c93-408b-417d-8b1b-c37d2828d7c3
📥 Commits

Reviewing files that changed from the base of the PR and between 0c864a0 and cfc0e92.

📒 Files selected for processing: 13
  • src/ast/e.rs
  • src/bundler/ParseTask.rs
  • src/bundler/transpiler.rs
  • src/js_parser/fold.rs
  • src/js_parser/p.rs
  • src/js_parser/parse/parse_entry.rs
  • src/js_parser/scan/scan_side_effects.rs
  • src/js_printer/lib.rs
  • src/react_compiler/hir/mod.rs
  • src/react_compiler/lowering/build_hir/expr.rs
  • test/bake/dev-and-prod.test.ts
  • test/bundler/bundler_bun.test.ts
  • test/bundler/transpiler/react-compiler.test.ts
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@robobun

robobun commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 5:05 AM PT - Oct 21st, 2026

❌ @robobun, your commit cfc0e92 has some failures in Build #129937 (All Failures)


🧪   To try this PR locally:

bunx bun-pr 46299

That installs a local version of the PR into your bun-46299 executable, so you can run:

bun-46299 --bun

@robobun

robobun commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

Status: ready for review.

Reproduced on main at 9b4284f with the two-step reproduction from the reopening comment of #9416: bun build --target=bun first.js --outfile=second.js, then bun build --target=node third.js --outfile=out.mjs && node out.mjs gives TypeError: __require is not a function. The same kind of library under an app built with --format=cjs --target=bun gives SyntaxError: import.meta is only valid inside modules. On this branch the first prints function and the second prints app: /. The commands are in the Notes of the description.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings marked 🟡 are optional suggestions and need no follow-up push.

Comment thread src/js_parser/fold.rs
Comment on lines +616 to +630
if name == b"require"
&& p.options.lower_import_meta_require
&& !p.method_call_must_be_replaced_with_undefined
&& !p.is_revisit_for_substitution
&& identifier_opts.assign_target() == js_ast::AssignTarget::None
&& !identifier_opts.is_delete_target()
{
if p.options.features.allow_runtime {
p.record_usage_of_runtime_require();
}
return Some(Expr {
data: js_ast::ExprData::ESpecial(E::Special::ImportMetaRequire),
loc: target.loc,
});
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Users who bundle for node code that detects Bun by testing import.meta.require now get a startup crash instead of the non-Bun branch. The arm at src/js_parser/fold.rs:616 lowers every non-assigned read, so import.meta.require ? import.meta.require("bun:sqlite") : null or typeof import.meta.require === "function" becomes a test of __require, which is always truthy in node output, and the guarded call then throws. Fix: keep the value undefined where the base did in positions that only test it (typeof, !, ? :, ??, ||, &&, == null) while still lowering calls, .resolve and plain value reads so const r = import.meta.require; r("x") keeps working. [also at: src/bundler/ParseTask.rs:2710 - Node-target bundles of code that feature-detects Bun with import.meta.require now take the Bun branch under node and throw, where on the base they fell back.]

Why this was flagged

A dependency or source file contains const sqlite = import.meta.require ? import.meta.require("bun:sqlite") : null; or if (typeof import.meta.require === "function") { ... Bun-only path ... } and is bundled with bun build --target=node (ESM or CJS) or --target=bun --format=cjs. src/bundler/ParseTask.rs:2702-2710 sets lower_import_meta_require for those outputs; src/js_parser/fold.rs:616-630 then replaces the read with E::Special::ImportMetaRequire for every position except assignment and delete targets, and src/js_printer/lib.rs:3423 prints it as __require or require. Under node the test is now true and the guarded __require("bun:sqlite") throws Cannot find module 'bun:sqlite' at module load. On the base branch the same output printed import.meta.require unchanged, which is undefined under node, so the test was false and the non-Bun branch ran. The PR's own test at test/bundler/bundler_bun.test.ts:443 (tested() expects "has require") records this change for an ordinary input.

Verification: src/js_parser/fold.rs:616-630 replaces every non-assigned, non-deleted read of import.meta.require with E::Special::ImportMetaRequire when lower_import_meta_require is set. On the base commit the EImportMeta arm left the access as written, so node evaluated import.meta.require to undefined and took the non-Bun branch; this is a regression that works on the base.

Comment thread src/js_printer/lib.rs
Comment on lines 3420 to +3429
true,
);
}
E::Special::ImportMetaRequire => {
self.print_expr(
Expr {
data: ExprData::ERequireCallTarget,
loc: expr.loc,
},
level,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 (optional) Dev-server client code that stores import.meta.require in a variable before calling it loses HMR dependency tracking, so edits to the required file now force a full page reload. The node prints as hmr.require (src/js_printer/lib.rs:3565), a prototype method whose this is the importer; called unbound, loadModuleSync gets importer === undefined and never records the edge (src/runtime/bake/hmr-module.ts:147, 307). On the base the client printed hmr.importMeta.require, which is this.require.bind(this) (hmr-module.ts:179). Fix: make the value safe to call detached for both require and import.meta.require, e.g. bind require per instance in the HMRModule constructor or print hmr.require.bind(hmr) when ERequireCallTarget is not the call target.

Why this was flagged

A dev-server client module contains const load = import.meta.require; const dep = load("./dep.js");. With output format InternalBakeDev, src/js_parser/fold.rs:616-629 returns E::Special::ImportMetaRequire, and src/js_printer/lib.rs:3420-3429 prints it through ERequireCallTarget, which for InternalBakeDev emits hmr.require (src/js_printer/lib.rs:3565-3567). HMRModule.prototype.require (src/runtime/bake/hmr-module.ts:145-147) calls loadModuleSync(id, true, this); invoked detached from load, this is undefined, so if (importer) mod.importers.add(importer) at hmr-module.ts:307-308 and 330-331 is skipped. When ./dep.js is edited, the hot-update walk at hmr-module.ts:676-679 sees importers.size === 0 without a self-accept, records a failure and does a full reload with the "root module that does not self-accept" message instead of propagating to the importer's import.meta.hot.accept. On the base branch the same source printed hmr.importMeta.require (src/js_printer/lib.rs:3282-3285), which is this.require.bind(this) at hmr-module.ts:179, so the importer edge was recorded.

Verification: HMRModule.prototype.require (hmr-module.ts:145-148) calls loadModuleSync(id, true, this); as a class method it runs in strict mode, so an unbound call passes importer === undefined. Every mod.importers.add(importer) site is guarded by if (importer) (hmr-module.ts:307, 330, 369, 418, 444, 481), so the call succeeds silently but records no edge.

Comment thread src/js_parser/fold.rs
// The printer spells this node as the `require` of the output. `--drop` of it needs the `E::Dot`. A revisit cannot record the use of the runtime's `__require`.
if name == b"require"
&& p.options.lower_import_meta_require
&& !p.method_call_must_be_replaced_with_undefined

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 (optional) Node-target builds of code that reads an import.meta.hot method without calling it leave the next import.meta.require unlowered, so it is undefined under node while the same expression a line earlier is a function. The guard at src/js_parser/fold.rs:618 reads method_call_must_be_replaced_with_undefined, which src/js_parser/fold.rs:782 and :816 set for every non-HMR import.meta.hot.<method> access and which only the next call visit clears (src/js_parser/visit/visit_expr.rs:2127). Fix: the --drop check must look at the current expression (e.g. compare the define hit or check the dot is the call target) instead of the sticky parser flag, so a stale flag from a sibling expression cannot suppress lowering.

Why this was flagged

A production (non-HMR) build for node contains, e.g., const dispose = import.meta.hot.dispose; const load = import.meta.require; (Vite-style modules read import.meta.hot methods as values). At src/js_parser/fold.rs:816 the hot method access sets p.method_call_must_be_replaced_with_undefined = true and returns undefined; the flag is only reset inside the next ECall visit at src/js_parser/visit/visit_expr.rs:2124-2127. The import.meta.require read that follows is not a call, so when it reaches the arm at src/js_parser/fold.rs:618 the flag is still set, the guard skips lowering and the dot is printed as import.meta.require, which is undefined under node. The dismissal only checked that drop cannot be defeated; it did not check that the guard's own action, suppressing the lowering, is the user-visible failure when the flag is stale. The author acknowledges this window in the PR notes but ships no test for it and bun/ImportMetaRequireAfterImportMetaHot only covers the call form. Remedy: gate on a per-expression drop decision rather than the sticky flag.

Verification: src/js_parser/fold.rs:782 and :816 set p.method_call_must_be_replaced_with_undefined = true unconditionally. src/js_parser/fold.rs:616-622 requires the flag clear, so an import.meta.require read after a non-call hot-method read is printed as written, undefined under node. On the base branch import.meta.require is never lowered for node, so this input already yields undefined there.

Comment thread src/bundler/ParseTask.rs
opts.lower_import_meta_require = match output_format {
options::Format::InternalBakeDev => true,
options::Format::Iife => false,
options::Format::Cjs => target.is_server_side(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟣 pre-existing, not blocking: pre-existing: users of bun build --format=cjs with no --target still get CommonJS output that contains import.meta.require, which is a SyntaxError in every CommonJS loader (node, webpack). The gate at src/bundler/ParseTask.rs:2705 is target.is_server_side(), so a browser-target CJS build is left unlowered. The stated reason for excluding the browser (a module script has no require) only holds for ESM output; a CJS module always has require and never has import.meta. Fix: lower import.meta.require to require for every CJS output regardless of target (Format::Cjs => true), keeping the HTML client files of a server build untouched, which already come through an ESM transpiler.
A small fix can ride a push you are already making; otherwise a short reply is enough.

Why this was flagged

A library or a dependency it bundles contains typeof Bun !== "undefined" ? import.meta.require("x") : null, or is a file that bun build --target=bun wrote (var __require = import.meta.require;). The user runs bun build --format=cjs entry.js with no --target, so target at src/bundler/ParseTask.rs:2479 is Target::Browser. The match at src/bundler/ParseTask.rs:2705 gives lower_import_meta_require = false, so the E::Dot arm at src/js_parser/fold.rs:616 does not fire and the output keeps import.meta.require. Loading that CommonJS file in node or any bundler throws SyntaxError: Cannot use 'import.meta' outside a module. The base branch produces the same broken output, so this is pre-existing; the PR lowers the same spelling for --target=bun and --target=node CJS output at the same line, and the printer path at src/js_printer/lib.rs:3559 already prints the bare global require for Format::Cjs, so the same lowering would be correct for browser-target CJS too.

Verification: bun build --format=cjs entry.js with no --target defaults to Target::Browser. src/bundler/ParseTask.rs:2702-2710 sets options::Format::Cjs => target.is_server_side(), which is false for Target::Browser, so the fold at src/js_parser/fold.rs:616-630 does not fire. Node's CommonJS loader rejects import.meta with a SyntaxError, so the CJS output fails to load at all.

Comment thread src/js_parser/fold.rs
}

// The printer spells this node as the `require` of the output. `--drop` of it needs the `E::Dot`. A revisit cannot record the use of the runtime's `__require`.
if name == b"require"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟣 pre-existing, not blocking: Users who build import.meta["require"](...) for node or as cjs get a bundle that works unminified and crashes with --minify-syntax. The new arm at src/js_parser/fold.rs:616 only runs from the EDot visit; with minify the EIndex path at src/js_parser/visit/visit_expr.rs:995-1016 rewrites the index to a dot and returns without running the fold, so the minified output keeps import.meta.require (undefined under node, SyntaxError in the @ bun-cjs wrapper) while the unminified output prints __require/require. Fix: lower every spelling that the lowering claims, e.g. run the import.meta fold (or re-visit the new EDot) after the EIndex->EDot shortcut so import.meta["require"], ["main"] and ["hot"] match the dotted form in both minify states.
A small fix can ride a push you are already making; otherwise a short reply is enough.

Why this was flagged

Trigger: source containing import.meta["require"] bundled with --target=node (esm or cjs) or --format=cjs --target=bun, with --minify-syntax or --minify. In visit_expr.rs:995-1016 the EIndex is turned into an E::Dot and the visitor returns at line 1016 without calling the property-access fold, so fold.rs:616-629 never sees it and no ESpecial::ImportMetaRequire is produced; record_usage_of_runtime_require is also not called. The same source without minify goes through visit_expr.rs:1023 onward and is lowered. Result: unminified node output runs, minified node output throws TypeError: import.meta.require is not a function, and minified cjs output is a SyntaxError inside the wrapper. On base both minify states were identical (both left as written), so the PR introduces a minify/non-minify divergence, the semantics-changing-minify bug class. Remedy: make the EIndex->EDot shortcut fall through into the same property-access fold the EDot path uses.

Verification: With minify, src/js_parser/visit/visit_expr.rs:995-1016 builds an E::Dot, assigns *e = dot; and returns; maybe_rewrite_property_access is never reached, so no ImportMetaRequire node is produced. Without minify, visit_expr.rs:1023-1039 reaches the fold at src/js_parser/fold.rs:597-630 and prints __require/require. No test uses import.meta["require"].

Comment thread src/js_parser/fold.rs
if name == b"require"
&& p.options.lower_import_meta_require
&& !p.method_call_must_be_replaced_with_undefined
&& !p.is_revisit_for_substitution

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟣 pre-existing, not blocking: Users bundling for node with --minify-syntax get undefined for import.meta.require reached through a local alias of import.meta, while the direct spelling in the same file prints __require. The guard !p.is_revisit_for_substitution at src/js_parser/fold.rs:619 skips the lowering when const m = import.meta; m.require || null is re-visited after inlining (src/js_parser/p.rs:2748-2754), and substitutions whose result is a call never re-run the fold at all (src/js_parser/p.rs:2755-2757), so import.meta.require(...) is emitted verbatim and throws under node. Fix: lower the alias form consistently with the direct form, and record the __require runtime usage at substitution time instead of relying on the revisit.
A small fix can ride a push you are already making; otherwise a short reply is enough.

Why this was flagged

Trigger: const m = import.meta; return m.require("x") or m.require || null in a file bundled with --target=node (esm or cjs) and --minify-syntax. The single-use-symbol substitution at p.rs:2735 replaces m with import.meta; when the result is EBinary/EUnary/EIf the revisit runs with is_revisit_for_substitution = true (p.rs:2749) and fold.rs:619 bails; when the result is an ECall no revisit happens (p.rs:2755-2757), so the fold never sees the new import.meta.require E::Dot. The printed output is import.meta.require(...), which is undefined under node (TypeError) and a SyntaxError inside the cjs wrapper, while a sibling direct import.meta.require("x") in the same file is lowered to __require("x"). Remedy: lower at substitution time or block the substitution for .require so both forms agree.

Verification: The new arm at src/js_parser/fold.rs:616-630 bails on !p.is_revisit_for_substitution (line 619); when the result is an ECall there is no revisit at all (p.rs:2755-2757), so the fold never sees the new import.meta.require. The aliased access prints import.meta.require while the direct spelling in the same file prints __require.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

import.meta.require is not a function error after target: "node" build

2 participants