Skip to content

fix(bundler): import.meta.url and esm wrapper fixes - #23803

Merged
Jarred-Sumner merged 7 commits into
mainfrom
dylan/import-meta-url-and-module-type
Oct 19, 2025
Merged

Jarred-Sumner merged 7 commits into
mainfrom
dylan/import-meta-url-and-module-type

Conversation

@dylan-conway

@dylan-conway dylan-conway commented Oct 18, 2025 •

Copy link
Copy Markdown
Member

What does this PR do?

Fixes printing import.meta.url and others with --bytecode. Fixes #14954.

Fixes printing __toESM when output module format is CJS and input module format is ESM.

The key change is that __toESM's isNodeMode parameter now depends on the input module type (whether the importing file uses ESM syntax like import/export) rather than the output format. This matches Node.js ESM behavior where importing CommonJS from .mjs files always wraps the entire module.exports object as the default export, ignoring __esModule markers.

How did you verify your code works?

Added comprehensive test suite in test/bundler/bundler_cjs.test.ts with 23 tests covering:

Core Behaviors:

  • ✅ Files using import syntax always get isNodeMode=1, which ignores __esModule markers and wraps the entire CJS module as default
  • ✅ This matches Node.js ESM semantics for importing CJS from .mjs files
  • ✅ Different CJS export patterns (exports.x, module.exports = ..., functions, primitives)
  • ✅ Named, default, and namespace (import *) imports
  • ✅ Different targets (node, browser, bun) - all behave the same
  • ✅ Different output formats (esm, cjs) - format doesn't affect the behavior
  • ✅ .mjs files re-exporting from .cjs
  • ✅ Deep re-export chains
  • ✅ Edge cases (non-boolean __esModule, __esModule=false, etc.)

Test Results:

  • With this PR's changes: All 23 tests pass ✅
  • Without this PR (system bun): 22 pass, 1 fails (the one testing that __esModule is ignored with import syntax + CJS format)

The failing test with system bun demonstrates the bug being fixed: currently, format=cjs with import syntax still respects __esModule, but it should ignore it (matching Node.js behavior).

@robobun

robobun commented Oct 18, 2025 •

Copy link
Copy Markdown
Collaborator
Updated 8:00 PM PT - Oct 18th, 2025

❌ Your commit 4cf9c30d has 3 failures in Build #29613 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 23803

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

bun-23803 --bun

@coderabbitai

coderabbitai Bot commented Oct 18, 2025 •

Copy link
Copy Markdown
Contributor

Walkthrough

Adds ExportsKind.toModuleType(), propagates module type into JS codegen/printer options, changes printer wrapping logic to consult the propagated input module type, expands import.meta inlining for bundle+CJS outputs, and updates/adds bundler tests (including a new CJS interop suite).

Changes

Cohort / File(s) Change Summary
ExportsKind → ModuleType
src/ast.zig
Added pub fn toModuleType(self: @This()) bun.options.ModuleType mapping export kinds (none, cjs, esm_with_dynamic_fallback, esm_with_dynamic_fallback_from_cjs, esm) to ModuleType.
Import.meta inlining
src/ast/maybe.zig
Expanded condition to allow inlining import.meta properties when p.options.bundle == true and p.options.output_format == .cjs, in addition to the existing p.options.framework != null case.
Module type propagation into codegen
src/bundler/.../LinkerContext.zig
Initialize js_printer.Options with .input_module_type = ast.exports_kind.toModuleType(), propagating the module type into JS printing options.
Printer options and interop wrapping logic
src/js_printer.zig
Added public field input_module_type: options.ModuleType = .unknown to pub const Options; changed require/import -> ESM wrapping check to use p.options.input_module_type == .esm instead of module_type.isESM().
Tests — expectations update
test/bundler/bundler_npm.test.ts
Adjusted expected out/entry.js size from 221726 to 221720 (6-byte reduction).
Tests — new CJS interop suite
test/bundler/bundler_cjs.test.ts
Added comprehensive tests (~23 cases) validating CommonJS→ESM interop and __toESM wrapping across various export shapes, __esModule edge cases, re-exports, targets, and output formats.

Suggested reviewers

  • Jarred-Sumner
  • nektro

Pre-merge checks

✅ Passed checks (4 passed)
Check name Status Explanation
Title Check ✅ Passed The PR title "fix(bundler): import.meta.url and esm wrapper fixes" directly and specifically addresses the two main changes in the changeset. The changes expand import.meta property handling for bundling scenarios and modify the __toESM wrapper behavior to depend on input module type rather than output format. The title is concise, avoids vague terminology, and clearly communicates the primary improvements from a developer's perspective.
Linked Issues Check ✅ Passed The PR directly addresses the primary requirement from issue #14954 by expanding import.meta property handling in src/ast/maybe.zig to support bundling scenarios, which resolves the bytecode generation failure when using import.meta.dir with the --bytecode flag. Additionally, the PR fixes the secondary objective regarding __toESM wrapper behavior when output format is CJS and input format is ESM by implementing isNodeMode logic based on input module type. The comprehensive test suite with 23 tests validates both fixes and demonstrates that all tests pass with the PR changes.
Out of Scope Changes Check ✅ Passed All changes in this PR are directly aligned with the stated objectives. The modifications to src/ast.zig, src/ast/maybe.zig, src/bundler/LinkerContext.zig, and src/js_printer.zig implement the core fixes for import.meta.url handling and input module type tracking. The new test suite comprehensively validates these fixes, and the 6-byte reduction in bundler_npm.test.ts is a natural consequence of the other changes. No unrelated or tangential modifications are present.
Description Check ✅ Passed The PR description fully follows the required template with both sections properly completed. The "What does this PR do?" section provides clear, detailed explanations of the two key fixes: import.meta.url handling with --bytecode and the isNodeMode logic change to depend on input module type. The "How did you verify your code works?" section comprehensively documents the 23-test suite with specific coverage areas and test results, providing sufficient verification evidence.

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

@coderabbitai coderabbitai 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.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/js_printer.zig (2)

1715-1721: LGTM: using input module type to gate __toESM flag fixes CJS-out + ESM-in.

This resolves the previous coupling to output format. Add targeted tests (internal/external module; importer ESM vs CJS).


1715-1721: Add , 1 flag to external require() path when wrapping with __toESM in ESM mode.

The internal path (lines 1715–1718) correctly passes , 1 to __toESM when input_module_type == .esm, but the external path (lines 1771–1772) omits it. Both paths must be aligned.

Apply the fix at lines 1771–1772:

                if (wrap_with_to_esm) {
+                   if (p.options.input_module_type == .esm) {
+                       p.print(",");
+                       p.printSpace();
+                       p.print("1");
+                   }
                    p.print(")");
                }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 0a92d64 and 3996f71.

📒 Files selected for processing (4)
  • src/ast.zig (1 hunks)
  • src/ast/maybe.zig (1 hunks)
  • src/bundler/LinkerContext.zig (1 hunks)
  • src/js_printer.zig (2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)

**/*.zig: Declare the extern C symbol in Zig and export a Zig-friendly alias for use
Wrap the Bun____toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue

Files:

  • src/ast.zig
  • src/ast/maybe.zig
  • src/js_printer.zig
  • src/bundler/LinkerContext.zig
src/**/*.zig

📄 CodeRabbit inference engine (CLAUDE.md)

In Zig code, manage memory carefully and use defer for cleanup of allocations/resources

When adding debug logs in Zig, create a scoped logger and log via Bun APIs: const log = bun.Output.scoped(.${SCOPE}, .hidden); then log("...", .{})

src/**/*.zig: Use private fields in Zig with the # prefix (e.g., struct { #foo: u32 };)
Prefer decl literals in Zig (e.g., const decl: Decl = .{ .binding = 0, .value = 0 };)
Prefer placing @import statements at the bottom of the Zig file (formatter may reorder automatically)
Prefer @import("bun") rather than @import("root").bun or @import("../bun.zig")

Files:

  • src/ast.zig
  • src/ast/maybe.zig
  • src/js_printer.zig
  • src/bundler/LinkerContext.zig
src/**/js_*.zig

📄 CodeRabbit inference engine (.cursor/rules/registering-bun-modules.mdc)

src/**/js_*.zig: Implement JavaScript bindings in a Zig file named with a js_ prefix (e.g., js_smtp.zig, js_your_feature.zig)
Handle reference counting correctly with ref()/deref() in JS-facing Zig code
Always implement proper cleanup in deinit() and finalize() for JS-exposed types

Files:

  • src/js_printer.zig
src/{**/js_*.zig,bun.js/api/**/*.zig}

📄 CodeRabbit inference engine (.cursor/rules/registering-bun-modules.mdc)

Use bun.JSError!JSValue for proper error propagation in JS-exposed Zig functions

Files:

  • src/js_printer.zig
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Format
🔇 Additional comments (2)
src/ast/maybe.zig (1)

413-431: The review comment raises valid concerns. An absolute path on Windows filesystem C:\Documents and Settings\ should be encoded using u3 notation file:///C:/Documents and Settings/ to maximize compatibility, and Unicode characters outside of the ASCII range must be UTF-8 encoded, and those UTF-8 encodings must be percent-encoded. Windows UNC filenames (such as \server\folder\data.xml) are described in RFC 8089, Appendix E as "non-standard", with two representations: file://server/folder/data.xml or file:////server/folder/data.xml.

Based on the codebase analysis, dedicated test coverage for these edge cases (Windows paths, UNC, non-ASCII filenames) is not evident in existing test files. The original review comment's suggestions remain actionable and should be addressed.

Verify that jsc.URL.fileURLFromString correctly handles Windows paths (drive letters, UNC), and non-ASCII filenames; add tests for import.meta properties with these edge cases.

The code change itself is sound—defer on bunstr is correct, logic change is safe—but verification and tests are recommended before merge.

src/bundler/LinkerContext.zig (1)

1327-1327: LGTM! Integration verified and correct.

The toModuleType() method at src/ast.zig:349 correctly maps ExportsKind variants to ModuleType values (none→unknown, cjs→cjs, esm variants→esm). The input_module_type field exists in printer Options (src/js_printer.zig:417) with the correct type, and is properly used at line 1715 to detect ESM input for __toESM wrapper generation. The integration chain is complete and correct.

Comment thread src/ast.zig
Comment on lines +349 to +359
pub fn toModuleType(self: @This()) bun.options.ModuleType {
return switch (self) {
.none => .unknown,
.cjs => .cjs,

.esm_with_dynamic_fallback,
.esm_with_dynamic_fallback_from_cjs,
.esm,
=> .esm,
};
}

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.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Deriving module type from export kind can misclassify ESM modules with no exports.

Mapping .none -> .unknown risks under-detecting ESM “importer” files that have only imports (or just import.meta) and no exports. In such cases input_module_type would be .unknown, and the printer would omit the , 1 flag to __toESM, breaking CJS→ESM interop for those importers.

  • Suggest: derive module type from a stronger signal (e.g., “is ESM syntax present” / module kind) rather than only exports. If Linker/AST already tracks an “is_esm_module” or similar, prefer that. Otherwise, augment the derivation to consider presence of import/export syntax or parser/module mode.

🏁 Script executed:

#!/bin/bash
# Find where ExportsKind.none is assigned and how ESM-ness is determined
rg -nP 'exports_kind\s*=\s*\.none|ExportsKind\.none' -g 'src/**/*.zig' -C3

# Locate any existing "is ESM" signal you could use instead of exports-only
rg -nP '\bis_esm\b|\bmodule_kind\b|\bmodule_type\b' -g 'src/**/*.zig' -C3

Length of output: 64485



🏁 Script executed:

#!/bin/bash
# Find ExportsKind enum definition and its variants
rg -nP 'pub\s+(const\s+)?ExportsKind\s*=|enum.*ExportsKind|ExportsKind\s*\{' -g 'src/**/*.zig' -A10

# Find the toModuleType method implementation
rg -nP 'fn\s+toModuleType|pub\s+fn\s+toModuleType' -g 'src/**/*.zig' -A8

# Check Parser structure for has_es_module_syntax usage
rg -nP '\bhas_es_module_syntax\b' -g 'src/**/*.zig' -C2

Length of output: 5379


Output:

src/bundler/LinkerContext.zig-1327-            .input_module_type = ast.exports_kind.toModuleType(),
src/bundler/LinkerContext.zig-1328-            .module_type = c.options.output_format,
src/bundler/LinkerContext.zig-1329-            .print_dce_annotations = c.options.emit_dce_annotations,
src/ast/P.zig-147-    esm_export_keyword: logger.Range = logger.Range.None,
src/ast/P.zig-148-    esm_import_keyword: logger.Range = logger.Range.None,
src/ast/P.zig-149-    latest_return_had_semicolon: bool = false,
src/ast/P.zig-1911-        // if we had an import statement, set a flag
src/ast/P.zig-1911-        if (p.esm_import_keyword.isEmpty()) {
src/ast/P.zig-1913-            p.esm_import_keyword = p.tokens.items[p.token_index].range;
src/ast/P.zig-1917-            p.esm_import_keyword = p.tokens.items[p.token_index].range;
src/ast/P.zig-1919-            if (!p.esm_export_keyword.isEmpty())

🏁 Script executed:

#!/bin/bash
# Where is toModuleType() actually called?
rg -nP '\.toModuleType\(\)' -g 'src/**/*.zig' -B2 -A2

# Check if esm_import_keyword is accessible in Ast struct
rg -nP 'esm_import_keyword|esm_export_keyword' -g 'src/**/*.zig' -C1

Length of output: 6299


🏁 Script executed:

#!/bin/bash
# Where is toModuleType() actually called?
rg -nP '\.toModuleType\(\)' -g 'src/**/*.zig' -B2 -A2

# Check if esm_import_keyword is accessible in Ast struct
rg -nP 'esm_import_keyword|esm_export_keyword' -g 'src/**/*.zig' -C1

Length of output: 6299


🏁 Script executed:

#!/bin/bash
# Find Ast struct definition (not just Ast.zig file)
rg -nP 'pub\s+struct\s+Ast\s*\{|pub\s+const\s+Ast\s*=\s*struct' -g 'src/**/*.zig' -A30

# Check if has_es_module_syntax is stored in Ast
rg -nP '\bhas_es_module_syntax\b' -g 'src/ast/Ast.zig' -C3

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Search for js_ast.Ast definition
fd -e zig -x grep -l "pub.*struct.*Ast" {} \; | head -20

# Look at what's returned from the parser
rg -nP 'pub\s+fn.*parse.*Ast|fn.*return.*Ast' -g 'src/ast/Parser.zig' -A5

# Find where import_keyword and export_keyword are used in Ast context
rg -nP '\.import_keyword|\.export_keyword' -g 'src/**/*.zig' -B2 -A1

Length of output: 1087


🏁 Script executed:

#!/bin/bash
# Find the function/code around P.zig:6519 to see what struct it returns
sed -n '6500,6550p' src/ast/P.zig

# Find Ast struct definition - look for "pub const Ast" or similar in ast.zig
sed -n '1,100p' src/ast/Ast.zig | grep -A 80 'pub const Ast'

Length of output: 2907


🏁 Script executed:

#!/bin/bash
# Look at full Ast struct definition
rg -nP 'pub const Ast = struct' -g 'src/ast/Ast.zig' -A 50

# Check all exports_kind assignments in Parser.zig
rg -nP 'exports_kind\s*=' -g 'src/ast/Parser.zig' -B2 -A1

Length of output: 3685


🏁 Script executed:

#!/bin/bash
# Check where p.options.module_type gets set and if it's ever non-.unknown
rg -nP 'opts\.module_type\s*=' -g 'src/**/*.zig' -B1 -A1 | head -40

# Verify: what does has_es_module_syntax include?
sed -n '2007,2015p' src/ast/P.zig

Length of output: 2027


Fix the exports_kind derivation to detect ESM-only importers.

The bug is real: files with only ESM imports (no exports, no top-level await) have exports_kind=.none, which maps to module_type=.unknown via toModuleType(). This occurs when module_type is preset and the fix path at Parser.zig:1080 is skipped.

The fix should use the existing has_es_module_syntax signal (which correctly includes import keyword detection per P.zig:2014) instead of checking only exports and await. Change Parser.zig line 1017–1018 from:

} else if (p.esm_export_keyword.len > 0 or p.top_level_await_keyword.len > 0) {
    exports_kind = .esm;

to:

} else if (p.has_es_module_syntax) {
    exports_kind = .esm;

This ensures ESM-only importers are classified as .esm rather than .unknown, preserving the CJS→ESM interop flag.

🤖 Prompt for AI Agents
In Parser.zig around lines 1017–1018 (and note the fix path skipped at ~1080),
the exports_kind derivation only checks esm_export_keyword or
top_level_await_keyword which misses files that only have ESM imports; change
the conditional to check p.has_es_module_syntax instead so exports_kind is set
to .esm for import-only ESM files, preserving correct module_type mapping and
CJS→ESM interop behavior.

Comment thread src/js_printer.zig
Jarred-Sumner and others added 3 commits October 18, 2025 17:09
This adds test/bundler/bundler_cjs.test.ts with 23 tests covering the
__toESM helper's behavior when converting CommonJS to ESM. The tests
document how the isNodeMode parameter (now based on input_module_type)
affects default export handling.

Key behaviors tested:
- Files using import syntax always get isNodeMode=1, which ignores
  __esModule markers and wraps the entire CJS module as default
- This matches Node.js ESM behavior for importing CJS from .mjs
- Tests cover different targets (node/bun/browser), formats (esm/cjs),
  file extensions (.mjs/.cjs), and edge cases

All 23 tests pass.

@coderabbitai coderabbitai 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.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 98ef8cd and 3eca45d.

📒 Files selected for processing (1)
  • test/bundler/bundler_cjs.test.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
test/**

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

Place all tests under the test/ directory

Files:

  • test/bundler/bundler_cjs.test.ts
test/bundler/**/*

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

Place bundler/transpiler/CSS/bun build tests under test/bundler/

Files:

  • test/bundler/bundler_cjs.test.ts
test/**/*.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

test/**/*.{js,ts}: Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Use shared utilities from test/harness.ts where applicable

Files:

  • test/bundler/bundler_cjs.test.ts
test/**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

test/**/*.test.{ts,tsx}: Test files must live under test/ and end with .test.ts or .test.tsx
In tests, always use port: 0; do not hardcode ports or roll your own random port
Prefer normalizeBunSnapshot for snapshotting test output instead of asserting raw strings
Do not write tests that assert absence of crashes (e.g., 'no panic' or 'no uncaught exception')
Use Bun’s Jest-compatible runner (import { test, expect } from "bun:test") for tests
Avoid shell commands like find or grep in tests; use Bun’s Glob and built-in tools instead
Prefer running tests via bun bd test and use provided harness utilities (bunEnv, bunExe, tempDir)
Use Bun.spawn with proper stdio handling and await proc.exited in process-spawning tests

Files:

  • test/bundler/bundler_cjs.test.ts
test/bundler/**

📄 CodeRabbit inference engine (CLAUDE.md)

Place bundler/transpiler tests under test/bundler/

Files:

  • test/bundler/bundler_cjs.test.ts
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}

📄 CodeRabbit inference engine (test/CLAUDE.md)

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Use bun:test for files ending with *.test.{ts,js,jsx,tsx,mjs,cjs}
Prefer concurrent tests (test.concurrent/describe.concurrent) over sequential when feasible
Organize tests with describe blocks to group related tests
Use utilities like describe.each, toMatchSnapshot, and lifecycle hooks (beforeAll, beforeEach, afterEach) and track resources for cleanup

Files:

  • test/bundler/bundler_cjs.test.ts
test/**/*.{ts,tsx,js,jsx,mjs,cjs}

📄 CodeRabbit inference engine (test/CLAUDE.md)

For large/repetitive strings, use Buffer.alloc(count, fill).toString() instead of "A".repeat(count)

Files:

  • test/bundler/bundler_cjs.test.ts
🧠 Learnings (3)
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/esm.test.ts : esm.test.ts should cover ESM feature behavior in development mode

Applied to files:

  • test/bundler/bundler_cjs.test.ts
📚 Learning: 2025-10-04T09:52:49.414Z
Learnt from: CR
PR: oven-sh/bun#0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-10-04T09:52:49.414Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Do not use ESM import syntax; write modules as CommonJS with export default { ... }

Applied to files:

  • test/bundler/bundler_cjs.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/bundle.test.ts : bundle.test.ts should contain DevServer-specific bundling tests

Applied to files:

  • test/bundler/bundler_cjs.test.ts
🧬 Code graph analysis (1)
test/bundler/bundler_cjs.test.ts (1)
test/bundler/expectBundled.ts (1)
  • itBundled (1734-1768)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Format
🔇 Additional comments (2)
test/bundler/bundler_cjs.test.ts (2)

1-3: Good placement and harness usage.

Lives under test/bundler/, uses bun:test + itBundled as per repo conventions.


4-20: Add import.meta tests with --bytecode using the established itBundled() pattern.

The review comment correctly identifies a gap: no tests verify import.meta.url, import.meta.dir, etc. when building with --bytecode. This is confirmed by:

  1. Git history shows commit "cjs import.meta.url fix, esm __toESM wrapper fix", validating the PR objective.
  2. Bytecode tests exist (e.g., HelloWorldBytecode) but contain no import.meta usage; import.meta tests exist (pathToFileURLWorks) but lack bytecode.
  3. Search confirms no tests combine import.meta + bytecode.

However, the suggested test approach diverges from the codebase pattern. Instead of raw Bun.spawn() CLI invocations, bundler tests use the itBundled() framework (see bundler_compile.test.ts lines 323–379 for the ReactSSR+bytecode example). A more consistent approach would add the import.meta test to bundler_compile.test.ts using:

itBundled("compile/ImportMetaBytecode", {
  compile: true,
  bytecode: true,
  files: {
    "/entry.ts": `
      console.log("url=" + import.meta.url);
      console.log("dir=" + import.meta.dir);
    `,
  },
  run: {
    stdout: /* assert expected output */,
    env: { BUN_JSC_verboseDiskCache: "1" },
  },
});

This aligns with established test patterns and leverages the framework's build/run infrastructure.

Comment thread test/bundler/bundler_cjs.test.ts
Comment thread test/bundler/bundler_cjs.test.ts Outdated
- Remove invalid test that had two entry files but only one outfile
  and no assertions (was redundant with tests 10/11)
- Add targeted test for input=ESM, output=CJS to directly cover the
  __toESM wrapper print fix
- Add doc comment for input_module_type field in js_printer.zig
  explaining it represents the importing file's module type and
  controls interop helper behavior

All 23 tests still pass with debug build.
With system bun: 22 pass, 1 fails (expected - demonstrates the fix).

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/js_printer.zig (1)

1744-1776: Fix missing second argument to __toESM for external require() when importing file is ESM.

The code at lines 1716–1723 correctly adds a second argument (1) to __toESM() when input_module_type == .esm for internal modules. However, the external require() branch at lines 1773–1775 lacks this check and unconditionally closes the __toESM() call without the second argument, creating inconsistent interop behavior.

Apply this fix:

                if (wrap_with_to_esm) {
+                   if (p.options.input_module_type == .esm) {
+                       p.print(",");
+                       p.printSpace();
+                       p.print("1");
+                   }
                    p.print(")");
                }
                 return;
♻️ Duplicate comments (1)
src/js_printer.zig (1)

417-419: Doc added for input_module_type — looks good.

Comment clearly states purpose and relation to __toESM semantics.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 3eca45d and 1ee7f0a.

📒 Files selected for processing (2)
  • src/js_printer.zig (2 hunks)
  • test/bundler/bundler_cjs.test.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
test/**

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

Place all tests under the test/ directory

Files:

  • test/bundler/bundler_cjs.test.ts
test/bundler/**/*

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

Place bundler/transpiler/CSS/bun build tests under test/bundler/

Files:

  • test/bundler/bundler_cjs.test.ts
test/**/*.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

test/**/*.{js,ts}: Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Use shared utilities from test/harness.ts where applicable

Files:

  • test/bundler/bundler_cjs.test.ts
test/**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

test/**/*.test.{ts,tsx}: Test files must live under test/ and end with .test.ts or .test.tsx
In tests, always use port: 0; do not hardcode ports or roll your own random port
Prefer normalizeBunSnapshot for snapshotting test output instead of asserting raw strings
Do not write tests that assert absence of crashes (e.g., 'no panic' or 'no uncaught exception')
Use Bun’s Jest-compatible runner (import { test, expect } from "bun:test") for tests
Avoid shell commands like find or grep in tests; use Bun’s Glob and built-in tools instead
Prefer running tests via bun bd test and use provided harness utilities (bunEnv, bunExe, tempDir)
Use Bun.spawn with proper stdio handling and await proc.exited in process-spawning tests

Files:

  • test/bundler/bundler_cjs.test.ts
test/bundler/**

📄 CodeRabbit inference engine (CLAUDE.md)

Place bundler/transpiler tests under test/bundler/

Files:

  • test/bundler/bundler_cjs.test.ts
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}

📄 CodeRabbit inference engine (test/CLAUDE.md)

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Use bun:test for files ending with *.test.{ts,js,jsx,tsx,mjs,cjs}
Prefer concurrent tests (test.concurrent/describe.concurrent) over sequential when feasible
Organize tests with describe blocks to group related tests
Use utilities like describe.each, toMatchSnapshot, and lifecycle hooks (beforeAll, beforeEach, afterEach) and track resources for cleanup

Files:

  • test/bundler/bundler_cjs.test.ts
test/**/*.{ts,tsx,js,jsx,mjs,cjs}

📄 CodeRabbit inference engine (test/CLAUDE.md)

For large/repetitive strings, use Buffer.alloc(count, fill).toString() instead of "A".repeat(count)

Files:

  • test/bundler/bundler_cjs.test.ts
**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)

**/*.zig: Declare the extern C symbol in Zig and export a Zig-friendly alias for use
Wrap the Bun____toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue

Files:

  • src/js_printer.zig
src/**/js_*.zig

📄 CodeRabbit inference engine (.cursor/rules/registering-bun-modules.mdc)

src/**/js_*.zig: Implement JavaScript bindings in a Zig file named with a js_ prefix (e.g., js_smtp.zig, js_your_feature.zig)
Handle reference counting correctly with ref()/deref() in JS-facing Zig code
Always implement proper cleanup in deinit() and finalize() for JS-exposed types

Files:

  • src/js_printer.zig
src/{**/js_*.zig,bun.js/api/**/*.zig}

📄 CodeRabbit inference engine (.cursor/rules/registering-bun-modules.mdc)

Use bun.JSError!JSValue for proper error propagation in JS-exposed Zig functions

Files:

  • src/js_printer.zig
src/**/*.zig

📄 CodeRabbit inference engine (CLAUDE.md)

In Zig code, manage memory carefully and use defer for cleanup of allocations/resources

When adding debug logs in Zig, create a scoped logger and log via Bun APIs: const log = bun.Output.scoped(.${SCOPE}, .hidden); then log("...", .{})

src/**/*.zig: Use private fields in Zig with the # prefix (e.g., struct { #foo: u32 };)
Prefer decl literals in Zig (e.g., const decl: Decl = .{ .binding = 0, .value = 0 };)
Prefer placing @import statements at the bottom of the Zig file (formatter may reorder automatically)
Prefer @import("bun") rather than @import("root").bun or @import("../bun.zig")

Files:

  • src/js_printer.zig
🧠 Learnings (3)
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/esm.test.ts : esm.test.ts should cover ESM feature behavior in development mode

Applied to files:

  • test/bundler/bundler_cjs.test.ts
📚 Learning: 2025-10-04T09:52:49.414Z
Learnt from: CR
PR: oven-sh/bun#0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-10-04T09:52:49.414Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Do not use ESM import syntax; write modules as CommonJS with export default { ... }

Applied to files:

  • test/bundler/bundler_cjs.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/bundle.test.ts : bundle.test.ts should contain DevServer-specific bundling tests

Applied to files:

  • test/bundler/bundler_cjs.test.ts
🧬 Code graph analysis (1)
test/bundler/bundler_cjs.test.ts (1)
test/bundler/expectBundled.ts (1)
  • itBundled (1734-1768)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Format
🔇 Additional comments (1)
test/bundler/bundler_cjs.test.ts (1)

429-447: Great: added direct “input=ESM, output=CJS” coverage for __toESM wrapper.

This precisely validates the printer change in CJS output.

Comment on lines +21 to +26
describe("bundler", () => {
// ============================================================================
// Tests with ESM syntax (import statements)
// These all use isNodeMode=1, which IGNORES __esModule
// ============================================================================

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.

🧹 Nitpick | 🔵 Trivial

Add one case exercising the external require() path.

Mark a dependency as external so the printer emits require("pkg") wrapped by __toESM, then assert runtime shape matches the internal/bundled case. This will guard the external branch symmetry.

Example sketch:

itBundled("cjs/__toESM_external_pkg", {
  files: {
    "/entry.js": `
      import def, { named } from "pkg";
      console.log(JSON.stringify({ def, named }));
    `,
    // Provide runtime package so the test can run:
    "/node_modules/pkg/package.json": `{ "name":"pkg","main":"index.cjs" }`,
    "/node_modules/pkg/index.cjs": `
      exports.named = "n";
      exports.default = { v: 1 };
    `,
  },
  external: ["pkg"],
  format: "cjs",
  run: { stdout: '{"def":{"v":1},"named":"n"}' },
});
🤖 Prompt for AI Agents
In test/bundler/bundler_cjs.test.ts around lines 21-26 add a new itBundled test
that exercises the external require() path: mark "pkg" as external so the
bundler emits require("pkg") wrapped by __toESM, provide a minimal runtime
package under /node_modules/pkg (package.json with main pointing to an index.cjs
and index.cjs exporting named and default values), set format to "cjs" and
external: ["pkg"], and assert the process run stdout equals the same JSON shape
as the internal/bundled case (e.g. '{"def":{"v":1},"named":"n"}') to verify
symmetry between external and internal branches.

The previous test was importing from an ESM file, which doesn't
exercise the __esModule marker bug. Changed to import from a CJS
file with __esModule marker, using both default and named imports.

Test results:
- With debug build (fix): 23 pass ✅
- With system bun (old): 21 pass, 2 fail ✅
  - Test 11: cjs/__toESM_format_cjs_with_import
  - Test 21: cjs/__toESM_input_esm_output_cjs_wrapper_print

Both failing tests demonstrate the bug: system bun respects __esModule
even with import syntax + cjs format, but the fix correctly ignores it.

@coderabbitai coderabbitai 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.

Actionable comments posted: 4

♻️ Duplicate comments (1)
test/bundler/bundler_cjs.test.ts (1)

21-26: Add one case exercising the external require() branch for symmetry.

This suite doesn’t cover the external path (require("pkg") + __toESM). Add a targeted case to ensure parity with bundled path. (Duplicate of earlier request.)

Apply by appending this test block:

+  // Test X: external require() path — ensure __toESM wrapper symmetry with bundled case
+  itBundled("cjs/__toESM_external_pkg", {
+    files: {
+      "/entry.js": /* js */ `
+        import def, { named } from "pkg";
+        console.log(JSON.stringify({ def, named }));
+      `,
+      // Provide runtime package so the test can run:
+      "/node_modules/pkg/package.json": `{ "name":"pkg","main":"index.cjs" }`,
+      "/node_modules/pkg/index.cjs": /* js */ `
+        exports.named = "n";
+        exports.default = { v: 1 };
+      `,
+    },
+    external: ["pkg"],
+    format: "cjs",
+    run: { stdout: '{"def":{"v":1},"named":"n"}' },
+  });
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 1ee7f0a and 4cf9c30.

📒 Files selected for processing (1)
  • test/bundler/bundler_cjs.test.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
test/**

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

Place all tests under the test/ directory

Files:

  • test/bundler/bundler_cjs.test.ts
test/bundler/**/*

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

Place bundler/transpiler/CSS/bun build tests under test/bundler/

Files:

  • test/bundler/bundler_cjs.test.ts
test/**/*.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

test/**/*.{js,ts}: Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Use shared utilities from test/harness.ts where applicable

Files:

  • test/bundler/bundler_cjs.test.ts
test/**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

test/**/*.test.{ts,tsx}: Test files must live under test/ and end with .test.ts or .test.tsx
In tests, always use port: 0; do not hardcode ports or roll your own random port
Prefer normalizeBunSnapshot for snapshotting test output instead of asserting raw strings
Do not write tests that assert absence of crashes (e.g., 'no panic' or 'no uncaught exception')
Use Bun’s Jest-compatible runner (import { test, expect } from "bun:test") for tests
Avoid shell commands like find or grep in tests; use Bun’s Glob and built-in tools instead
Prefer running tests via bun bd test and use provided harness utilities (bunEnv, bunExe, tempDir)
Use Bun.spawn with proper stdio handling and await proc.exited in process-spawning tests

Files:

  • test/bundler/bundler_cjs.test.ts
test/bundler/**

📄 CodeRabbit inference engine (CLAUDE.md)

Place bundler/transpiler tests under test/bundler/

Files:

  • test/bundler/bundler_cjs.test.ts
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}

📄 CodeRabbit inference engine (test/CLAUDE.md)

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Use bun:test for files ending with *.test.{ts,js,jsx,tsx,mjs,cjs}
Prefer concurrent tests (test.concurrent/describe.concurrent) over sequential when feasible
Organize tests with describe blocks to group related tests
Use utilities like describe.each, toMatchSnapshot, and lifecycle hooks (beforeAll, beforeEach, afterEach) and track resources for cleanup

Files:

  • test/bundler/bundler_cjs.test.ts
test/**/*.{ts,tsx,js,jsx,mjs,cjs}

📄 CodeRabbit inference engine (test/CLAUDE.md)

For large/repetitive strings, use Buffer.alloc(count, fill).toString() instead of "A".repeat(count)

Files:

  • test/bundler/bundler_cjs.test.ts
🧠 Learnings (3)
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/esm.test.ts : esm.test.ts should cover ESM feature behavior in development mode

Applied to files:

  • test/bundler/bundler_cjs.test.ts
📚 Learning: 2025-10-04T09:52:49.414Z
Learnt from: CR
PR: oven-sh/bun#0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-10-04T09:52:49.414Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Do not use ESM import syntax; write modules as CommonJS with export default { ... }

Applied to files:

  • test/bundler/bundler_cjs.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/bundle.test.ts : bundle.test.ts should contain DevServer-specific bundling tests

Applied to files:

  • test/bundler/bundler_cjs.test.ts
🧬 Code graph analysis (1)
test/bundler/bundler_cjs.test.ts (1)
test/bundler/expectBundled.ts (1)
  • itBundled (1734-1768)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Format

Comment on lines +4 to +20
// Tests for CommonJS <> ESM interop, specifically the __toESM helper behavior.
//
// The key insight from the code change:
// - `input_module_type` is set based on the AST's exports_kind (whether the importing
// file uses ESM syntax like import/export or CJS syntax like require/module.exports)
// - When a file uses ESM syntax (import/export), isNodeMode = 1
// - When a file uses CJS syntax (require), __toESM is not used at all
//
// This means:
// - Any file using `import` will always get isNodeMode=1, which IGNORES __esModule
// and always wraps the CJS module as the default export
// - This matches Node.js ESM behavior where importing CJS from .mjs always wraps
// the entire exports object as the default
//
// The __esModule marker is only respected in non-bundled scenarios or when using
// actual CommonJS require() syntax.

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.

🧹 Nitpick | 🔵 Trivial

Request: add a dedicated --bytecode import.meta test to validate the other PR objective.

This file exercises __toESM well, but the PR also fixes import.meta.* under bun build --bytecode. Please add a separate test (e.g., test/bundler/bundler_import_meta_bytecode.test.ts) that spawns bun build --bytecode --compile --minify and verifies runtime output. As per coding guidelines, use Bun.spawn, await proc.exited, and harness utilities.

Sketch:

import { test, expect } from "bun:test";
import { bunExe, bunEnv, tempDir, normalizeBunSnapshot } from "../harness";
import { join } from "path";
import { writeFile } from "fs/promises";

test("bytecode/import.meta.url compiles and prints", async () => {
  const dir = tempDir();
  await writeFile(join(dir, "meta.ts"), `
    console.log(JSON.stringify({ url: import.meta.url, dir: import.meta.dir, file: import.meta.file }));
  `);
  const build = Bun.spawn({
    cmd: [bunExe(), "build", "--bytecode", "--compile", "--minify", "meta.ts", "--outfile", "meta"],
    cwd: dir,
    env: bunEnv(),
    stdio: ["ignore", "pipe", "pipe"],
  });
  await build.exited;
  expect(build.exitCode).toBe(0);

  const exe = process.platform === "win32" ? "meta.exe" : "meta";
  const run = Bun.spawn({ cmd: [join(dir, exe)], cwd: dir, stdio: ["ignore", "pipe", "pipe"] });
  await run.exited;
  const out = (await new Response(run.stdout).text()).trim();
  expect(normalizeBunSnapshot(out)).toMatchSnapshot();
});

As per coding guidelines.

🤖 Prompt for AI Agents
In test/bundler/bundler_cjs.test.ts around lines 4 to 20: add a new test file
test/bundler/bundler_import_meta_bytecode.test.ts that spawns Bun to build a
small module using --bytecode --compile --minify and then executes the produced
binary to verify import.meta.* values; create a temp dir, write a meta.ts that
logs JSON of import.meta.url/dir/file, invoke Bun.spawn to run bun build with
proper cwd/env/stdio, await build.exited and assert exitCode === 0, then run the
produced binary (handle Windows exe name), await run.exited, read run.stdout,
normalize with normalizeBunSnapshot and expect it to matchSnapshot; follow
existing harness utilities (bunExe, bunEnv, tempDir, normalizeBunSnapshot) and
testing style (import from "bun:test"), using async/await and Bun.spawn with
stdio ["ignore","pipe","pipe"] for both build and run.

Comment on lines +127 to +129
// Namespace import only gets the CJS exports as-is, no default wrapper
stdout: '{"foo":"foo","bar":"bar"}',
},

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.

⚠️ Potential issue | 🟠 Major

Fix namespace import expectation: default should be present (align with Test 17).

Namespace/star import should expose default plus enumerable CJS keys under Node-mode wrapping. Current expectation contradicts Test 17’s namespace shape.

Apply this diff:

-    run: {
-      // Namespace import only gets the CJS exports as-is, no default wrapper
-      stdout: '{"foo":"foo","bar":"bar"}',
-    },
+    run: {
+      // Namespace import returns wrapper: default + enumerable properties
+      stdout: '{"default":{"foo":"foo","bar":"bar"},"foo":"foo","bar":"bar"}',
+    },
🤖 Prompt for AI Agents
In test/bundler/bundler_cjs.test.ts around lines 127-129, the namespace import
expectation omits the Node-mode wrapper's default property; update the expected
stdout to include the default key (wrapping the original CJS exports) alongside
the enumerable keys. Change the expectation to include "default":{...} so the
JSON becomes something like
'{"default":{"foo":"foo","bar":"bar"},"foo":"foo","bar":"bar"}'.

Comment on lines +137 to +189
// Test 7: target=node
itBundled("cjs/__toESM_target_node", {
files: {
"/entry.js": /* js */ `
import lib from './lib.cjs';
console.log(JSON.stringify(lib));
`,
"/lib.cjs": /* js */ `
exports.x = 1;
exports.y = 2;
`,
},
target: "node",
run: {
stdout: '{"x":1,"y":2}',
},
});

// Test 8: target=browser
itBundled("cjs/__toESM_target_browser", {
files: {
"/entry.js": /* js */ `
import lib from './lib.cjs';
console.log(JSON.stringify(lib));
`,
"/lib.cjs": /* js */ `
exports.x = 1;
exports.y = 2;
`,
},
target: "browser",
run: {
stdout: '{"x":1,"y":2}',
},
});

// Test 9: target=bun
itBundled("cjs/__toESM_target_bun", {
files: {
"/entry.js": /* js */ `
import lib from './lib.cjs';
console.log(JSON.stringify(lib));
`,
"/lib.cjs": /* js */ `
exports.x = 1;
exports.y = 2;
`,
},
target: "bun",
run: {
stdout: '{"x":1,"y":2}',
},
});

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.

🧹 Nitpick | 🔵 Trivial

Optional: dedupe target variants via a tiny generator to reduce boilerplate.

Replace the three near-identical target tests with a loop that generates them.

Example:

-  // Test 7/8/9...
-  itBundled("cjs/__toESM_target_node", { /* ... */ target: "node", run:{ stdout:'{"x":1,"y":2}' } });
-  itBundled("cjs/__toESM_target_browser", { /* ... */ target: "browser", run:{ stdout:'{"x":1,"y":2}' } });
-  itBundled("cjs/__toESM_target_bun", { /* ... */ target: "bun", run:{ stdout:'{"x":1,"y":2}' } });
+  for (const target of ["node", "browser", "bun"] as const) {
+    itBundled(`cjs/__toESM_target_${target}`, {
+      files: {
+        "/entry.js": `import lib from './lib.cjs'; console.log(JSON.stringify(lib));`,
+        "/lib.cjs": `exports.x = 1; exports.y = 2;`,
+      },
+      target,
+      run: { stdout: '{"x":1,"y":2}' },
+    });
+  }
🤖 Prompt for AI Agents
In test/bundler/bundler_cjs.test.ts around lines 137 to 189, there are three
nearly identical tests only differing by the target ("node", "browser", "bun");
replace them with a small loop or array.map that iterates over the target
variants and calls itBundled for each to remove duplication. Keep the same
files, run expectations and test names (e.g., append the target to the test
name) so behavior is unchanged, and ensure the loop is executed at
file-evaluation time so the test harness registers each generated test.

Comment on lines +467 to +469
// Star import gets the exports as-is, no wrapper
stdout: '{"named":"named","default":"default","__esModule":true}',
},

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.

⚠️ Potential issue | 🟠 Major

Star import with __esModule: ensure wrapper shape/order matches other tests.

Keep star-import behavior consistent with Test 17 (default first, then keys). Also remove “no wrapper” comment.

Apply this diff:

-    run: {
-      // Star import gets the exports as-is, no wrapper
-      stdout: '{"named":"named","default":"default","__esModule":true}',
-    },
+    run: {
+      // Star import returns wrapper: default + enumerable properties
+      stdout: '{"default":"default","named":"named","__esModule":true}',
+    },
🤖 Prompt for AI Agents
In test/bundler/bundler_cjs.test.ts around lines 467-469, the expected stdout
for the star-import case is ordered incorrectly and the inline comment is
outdated; change the expected JSON string to have "default" first then other
keys (e.g. '{"default":"default","named":"named","__esModule":true}') to match
Test 17’s shape/order, and remove the “no wrapper” comment on the preceding
line.

@Jarred-Sumner
Jarred-Sumner merged commit de4a5a0 into main Oct 19, 2025
64 of 65 checks passed
@Jarred-Sumner
Jarred-Sumner deleted the dylan/import-meta-url-and-module-type branch October 19, 2025 03:49
@coderabbitai coderabbitai Bot mentioned this pull request Jan 23, 2026
Jarred-Sumner pushed a commit that referenced this pull request Sep 2, 2026
…1150)

### Problem
- `bun build` returns the whole `module.exports` for a default import of
a CommonJS module that sets `__esModule`, when the importer is a `.ts`,
`.tsx` or `.js` file. `bun run`, esbuild and Rolldown return
`exports.default` for the same files. They use Node's interop (the whole
`module.exports`) only for a `.mjs`, `.mts` or `"type": "module"`
importer. TypeScript and Babel emit that marker, so this breaks the
default import of many packages (styled-components, react-bootstrap).
Found while bundling Outline: a React invalid hook call at boot.
- Cause: `print_code_for_file_in_chunk_js`
(`src/bundler/LinkerContext.rs:2290`) derived the printer's
`input_module_type` from `ast.exports_kind`. Any file with ESM syntax
counted as ESM, so every `__toESM` call got `isNodeMode = 1`, which
ignores `__esModule`.
- A second source of the same bug: the resolver set `module_type` from
the matched `"import"` or `"require"` export condition. A "fake ESM"
file in a package without `"type"`, reached through `"exports": {
"import": ... }`, was ESM to the printer too. Fixes #7709. Fixes #18615.

### Fix
- `BundledAst` carries the resolver's `module_type` (the extension, else
the nearest package.json `"type"`). The linker passes that to the
printer, so `isNodeMode = 1` only for an ESM-by-type importer. The
external `require()` path now prints `, 1` under the same rule, as
esbuild does.
- The resolver no longer turns an export condition into a module type,
and `.mjs`/`.mts`/`.cjs`/`.cts` now win over any package.json `"type"`.
Node decides a file's format from its extension and the nearest
package.json only. A task that bypasses the resolver (a plugin result)
falls back to the extension.
- Correct because it is esbuild's rule (`ModuleTypeData`, set from the
extension or the enclosing package.json) and Node's. Checked against
esbuild 0.21.5 for every importer kind below.
- Verified: `test/bundler/bundler_cjs.test.ts` (45 tests, 17 fail on the
released bun). Also `test/bundler/esbuild/*`, `bundler_cjs2esm`,
`bundler_splitting`, `bundler_npm`, `bundler_edgecase`,
`bundler_regressions`, `bundler_barrel`, `transpiler/*`,
`test/js/bun/resolve/*`, `cli/run/run-cjs`.

### Background
- `__toESM(mod, isNodeMode)` is the runtime helper that builds the ESM
view of a CommonJS module. With `isNodeMode = 0` it honors `__esModule`:
`default` is `mod.default`. With `isNodeMode = 1` it copies Node:
`default` is `mod` itself.
- `ExportsKind` is what the parser found in a file (`import`/`export`
syntax versus `exports`/`module` use). `ModuleType` is what the file
system says: the extension or package.json `"type"`. Node's interop
follows the second, never the first.
- Supersedes #35656, which found the export condition part.

<details><summary>Notes</summary>

Repro from the report (1.3.13, 1.4.0, canary):

```
# dep.cjs: Object.defineProperty(exports, "__esModule", { value: true }); exports.default = function () {}; exports.named = 1;
# entry.ts: import d, { named } from "./dep.cjs"; console.log(typeof d, named);
bun entry.ts                                       # function 1
bun build entry.ts --outfile=out.js && bun out.js  # object 1   (before this change)
esbuild entry.ts --bundle --format=esm | node --input-type=module   # function 1
```

esbuild 0.21.5 output for each importer, matched by the new tests:
`.ts`: `__toESM(require_dep())`. `.mts`: `__toESM(require_dep(), 1)`.
`"type": "module"`: `, 1`. `"type": "commonjs"`: no `, 1`. External CJS
in CJS output: `__toESM(require("ext"))` for `.ts`,
`__toESM(require("ext"), 1)` for `.mjs`. Dynamic `import()` of a bundled
or split CJS module: the same rule.

History: #23803 (Oct 2025) moved `isNodeMode` from the output format to
`exports_kind`. Before that, the default `--format=esm` always used
`isNodeMode = 1`, so `bun build` never honored `__esModule` for `.ts`
importers.

Test expectation updates for the dropped `, 1` (2 bytes minified, 3
bytes otherwise): `EmitInvalidSourceMap2` mapping column, `npm/ReactSSR`
mapping columns and file size (6 bytes: three `, 1`), and the
`BundledReactPreservesImportRefs` snapshot. Six tests in
`bundler_cjs.test.ts` asserted the old behavior for a `.js` entry and
now assert `exports.default`. `regression/issue/03844` still sees `, 1`:
the importer there is `ws/wrapper.mjs`.

Resolver change, what else it touches: `module_type` is also the
parser's hint for a file with no `import`/`export` and no
`exports`/`module` use. Such a file reached through an `"import"`
condition was ESM by the condition. It now follows the `Unknown` rules
(ESM unless it uses `require`, `__dirname` or `__filename`). A `.cjs`
target of an `"import"` condition is now CommonJS.

Known gap, unchanged here: the resolver's `enclosing_package_json` skips
a package.json without a `"name"` (#229). A nameless `{ "type": "module"
}` above the importer does not make it ESM for this decision, except
when it sits next to a file resolved through an exports map
(`handle_esm_resolution` reads the file's own directory). Node and
esbuild use the nearest package.json whatever its name.

Unrelated, found on the way: `bun build
test/regression/issue/03844/03844.fixture.ts` from the repo root fails
with `EISDIR reading file: "test"` and trips `assertion failed:
crate::is_absolute(self.text)` in a debug build. Handed off separately.
</details>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Using import.meta.dir when compiling with --bytecode flag throws error

3 participants