Skip to content

bundler: require a bundled .node file from the chunk that holds it #47483

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.

Sign up for GitHub

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

Open
robobun wants to merge 1 commit into
base: main
Choose a base branch
from
Open

bundler: require a bundled .node file from the chunk that holds it #47483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/bundler/LinkerContext.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,16 @@ impl<'a> LinkerContext<'a> {
&& self.chunks_runtime_ref.is_valid()
}

/// Whether `chunk` makes the `require` for the bundled `.node` file `source_index` that it holds. The linker
/// prints the path of the file for that chunk, and the runtime's `__require` resolves from the chunk of the
/// runtime module. This is for Node.js: in a split build for Bun, the file calls `import.meta.require`.
pub(crate) fn makes_require_for_asset(&self, chunk: &Chunk, source_index: u32) -> bool {
self.parse_graph().input_files.items_loader()[source_index as usize] == Loader::Napi
&& !chunk
.files_with_parts_in_chunk
.contains(&Index::RUNTIME.get())
}
Comment thread
robobun marked this conversation as resolved.
Resolved

/// An `import()` — or a split `require()` — whose target is a
/// chunk entry point: the record is resolved at runtime against the
/// target's chunk instead of binding to a wrapper.
Expand Down Expand Up @@ -2369,6 +2379,7 @@ impl<'a> LinkerContext<'a> {
to_esm_ref: Ref,
to_commonjs_ref: Ref,
runtime_require_ref: Option<Ref>,
makes_require_for_asset: bool,
source_index: Index,
source: &Source,
module_info: Option<&mut crate::analyze_transpiled_module::ModuleInfo>,
Expand Down Expand Up @@ -2420,6 +2431,7 @@ impl<'a> LinkerContext<'a> {
Format::Cjs => None, // use unbounded global
_ => runtime_require_ref,
},
require_target_makes_require: makes_require_for_asset,
require_or_import_meta_for_source_callback: Some(
js_printer::RequireOrImportMetaCallback::init(self),
),
Expand Down
2 changes: 2 additions & 0 deletions src/bundler/ParseTask.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,8 @@ pub mod parse_worker {

/// `require("<unique key>")`. Unlike `import.meta.require`, the call target
/// prints per output format, so `--bytecode` (CommonJS) can compile it.
/// A `.node` file of a split build calls a `require` of its chunk: `generate_code_for_lazy_export`
/// for Bun, `LinkerContext::makes_require_for_asset` for Node.js.
fn require_embedded_asset(unique_key: &[u8]) -> Expr {
let import_path = Expr::init(
E::String {
Expand Down
2 changes: 2 additions & 0 deletions src/bundler/linker_context/generateCodeForFileInChunkJS.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ pub fn generate_code_for_file_in_chunk_js(
Ref::NONE,
Ref::NONE,
None,
false,
part_range.source_index,
source,
module_info,
Expand Down Expand Up @@ -942,6 +943,7 @@ pub fn generate_code_for_file_in_chunk_js(
to_esm_ref,
to_common_js_ref,
runtime_require_ref,
c.makes_require_for_asset(chunk, source_index as u32),
part_range.source_index,
source_ref,
module_info,
Expand Down
34 changes: 32 additions & 2 deletions src/bundler/linker_context/generateCodeForLazyExport.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ use bun_js_parser::js_lexer;

use crate::bun_css::properties::css_modules::Specifier as CssSpecifier;
use crate::bun_css::{BundlerStyleSheet, CssRef, CssRefTag};
use crate::{Index, IndexInt, LinkerContext};
use crate::options::Loader;
use crate::{Index, IndexInt, LinkerContext, WrapKind};
use bun_collections::DynamicBitSetUnmanaged as BitSet;

type SymbolList<'a> = bun_ast::symbol::List<'a>;
Expand Down Expand Up @@ -421,9 +422,28 @@ pub(crate) fn generate_code_for_lazy_export(

// `require(<asset>)` prints as the runtime's `__require` outside CommonJS
// output, so the part that holds the call must import it.
let calls_runtime_require = matches!(expr.data, ExprData::ECall(ref c)
let mut calls_runtime_require = matches!(expr.data, ExprData::ECall(ref c)
if matches!(c.target.data, ExprData::ERequireCallTarget))
&& this.options.output_format != crate::options::OutputFormat::Cjs;
if calls_runtime_require
&& requires_asset_from_import_meta(this, source_index)
&& let ExprData::ECall(mut call) = expr.data
{
let loc = call.target.loc;
call.target = Expr::init(
E::Dot {
target: Expr::init(E::ImportMeta {}, loc),
name: b"require".as_slice().into(),
name_loc: loc,
..Default::default()
},
loc,
);
Comment on lines +433 to +441

@claude claude Bot Oct 30, 2026

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 build a split Bun-target bundle with --define import.meta.require=... (or import.meta.url) lose their define for every bundled .node addon after merging. The target injected at src/bundler/linker_context/generateCodeForLazyExport.rs:433-441 is a fresh import.meta.require AST node built in the linker, after the parser has already applied defines, so it prints literally; on the base branch the call went through the runtime's __require, whose own import.meta.require the define had rewritten. Fix: make the injected addon call honour the build's defines, e.g. keep __require when import.meta.require is defined or run the define substitution over the injected expression. [also at: src/js_printer/lib.rs:3560 - Users who build a split Node.js or Bun bundle with a .node file and --define import.meta.url=... (or import.meta.require=...) get an addon that no longer loads after this merges.]

Why this was flagged

A user runs bun build --splitting --target bun --define import.meta.require=require entry.js so the output also runs on Node, and entry.js requires a .node addon that lands in the same directory as the runtime chunk. On the base branch the addon call prints __require(<path>) and the runtime module's __require is import.meta.require, which the parser-time define turned into require, so the addon loads. After this change requires_asset_from_import_meta (generateCodeForLazyExport.rs:589-593) is true and lines 433-441 replace the call target with E::Dot { target: E::ImportMeta, name: "require" }; defines are applied in the parser, not here, so the printer at src/js_printer/lib.rs:3278 emits the literal import.meta.require, which is undefined when the output runs on Node, and the addon fails to load. The PR description lists 10 of 208 checked builds that stop loading an addon because they redirect __require with --define; no guard in the diff keeps the old call when such a define is present.

Verification: Defines on import.meta.<x> are applied at parse time (src/js_parser/visit/visit_expr.rs:1358-1361). After this change, src/bundler/linker_context/generateCodeForLazyExport.rs:428-441 replaces the ERequireCallTarget with a freshly built E::Dot { target: E::ImportMeta, name: "require" } after parsing is done. The printer prints E::ImportMeta literally (src/js_printer/lib.rs:3273-3278), so the user's define no longer reaches it.

@robobun robobun Oct 30, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is true, and it is deliberate. The description states it under Downsides (10 of the 208 checked builds) and under "The rule, and what it decides" in the notes.

Main already prints a literal import.meta.require for the other request that the linker writes for a file that it emits: a split require() (#40519, src/js_printer/lib.rs:2934). A --define does not reach that call. On main at f7607a3:

$ printf 'const tool = () => require("./tool.js");\nconsole.log(tool().name);\n' > main.js
$ printf 'export const name = "tool";\n' > tool.js
$ bun build ./main.js --splitting --format esm --target bun --define 'import.meta.require=globalThis.hostRequire' --outdir out
$ grep -n 'tool-' out/main.js
8:  var tool = () => import.meta.require("./tool-h5n7x3et.js");

So on main, a build that replaces import.meta.require loads its chunks with the real one and its addons with the replacement. With this change, both use the real one.

The example in the comment does not run on main. --define import.meta.require=require prints var __require = require;. Node.js throws ReferenceError: require is not defined in ES module scope, and Bun throws ReferenceError: require is not defined. With this change, Bun finds the addon.

An earlier revision of this branch kept main's call when a define or a plugin changed the declaration of __require. I removed it: it was 61 lines, and no build that needs it is known. If a maintainer wants a define of import.meta.require or import.meta.url to keep main's call for the addon, I can add a check for those two keys.

// A module without a wrapper keeps its use of `__require`. It is that module's one use of the runtime
// module, and the chunk of the runtime module and the order of the chunks follow that use (#44495).
calls_runtime_require =
this.graph.meta.items_flags()[source_index as usize].wrap == WrapKind::None;
}

match exports_kind {
bun_ast::ExportsKind::Cjs => {
Expand Down Expand Up @@ -561,3 +581,13 @@ pub(crate) fn generate_code_for_lazy_export(

Ok(())
}

/// Whether the `require(<asset>)` of the bundled `.node` file `source_index` (`require_embedded_asset`) calls
/// `import.meta.require`, as a split `require()` does for Bun. The linker prints the path of the file for the
/// chunk that holds the call, and the runtime's `__require` resolves from the chunk of the runtime module.
/// For Node.js, see `LinkerContext::makes_require_for_asset`.
fn requires_asset_from_import_meta(this: &LinkerContext, source_index: IndexInt) -> bool {
this.graph.code_splitting
&& this.parse_graph().input_files.items_loader()[source_index as usize] == Loader::Napi
&& this.graph.ast.items_target()[source_index as usize].is_bun()
}
10 changes: 10 additions & 0 deletions src/js_printer/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1321,6 +1321,9 @@ pub struct Options<'a> {
/// `__preload`: when set, an `import()` of a chunk is printed as `(__preload(chunkId), import(path))`.
pub module_preload_ref: Ref,
pub require_ref: Option<Ref>,
/// `ERequireCallTarget` prints `<require_ref>("node:module").createRequire(import.meta.url)`: a `require`
/// that resolves from the file that holds the call, not from the file that declares `require_ref`.
pub require_target_makes_require: bool,
pub import_meta_ref: Ref,
pub hmr_ref: Ref,
pub indent: Indentation,
Expand Down Expand Up @@ -1403,6 +1406,7 @@ impl<'a> Default for Options<'a> {
to_esm_ref: Ref::NONE,
module_preload_ref: Ref::NONE,
require_ref: None,
require_target_makes_require: false,
import_meta_ref: Ref::NONE,
hmr_ref: Ref::NONE,
indent: Indentation::default(),
Expand Down Expand Up @@ -3549,6 +3553,12 @@ pub(crate) mod __gated_printer {

if let Some(require_ref) = self.options.require_ref {
self.print_symbol(require_ref);
if self.options.require_target_makes_require {
if let Some(mi) = self.module_info() {
mi.flags.contains_import_meta = true;
}
self.print(b"(\"node:module\").createRequire(import.meta.url)");
}
} else if self.options.module_type == bundle_opts::Format::InternalBakeDev {
self.print_symbol(self.options.hmr_ref);
self.print(b".require");
Expand Down
Loading
Loading