Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 4 additions & 1 deletion src/js_parser/p.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5610,7 +5610,10 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
if self.options.repl_mode {
return None;
}
if self.has_es_module_syntax && self.commonjs_named_exports.count() == 0 {
if (self.has_es_module_syntax
|| (self.options.module_type == options::ModuleType::Esm && !self.options.bundle))
&& self.commonjs_named_exports.count() == 0
{
// In an ES6 module, "this" is supposed to be undefined. Instead of
// doing this at runtime using "fn.call(undefined)", we do it at
// compile time using expression substitution here.
Expand Down
14 changes: 13 additions & 1 deletion src/js_parser/parse/parse_entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,10 @@ impl<'a> Options<'a> {
hasher.update(b"no_dce");
}

// `module_type` changes transpiled output for byte-identical sources:
// exports_kind and the top-level `this` substitution both depend on it.
hasher.update(&[self.module_type as u8]);

self.features.hash_for_runtime_transpiler(hasher);
}

Expand Down Expand Up @@ -1624,7 +1628,15 @@ impl<'a> Parser<'a> {

if p.is_deoptimized_commonjs() {
exports_kind = js_ast::ExportsKind::Cjs;
} else if p.esm_export_keyword.len > 0 || p.top_level_await_keyword.len > 0 {
} else if p.esm_export_keyword.len > 0
|| p.top_level_await_keyword.len > 0
// .mjs / .mts / "type":"module" are authoritative at runtime: a bare
// reference to `module`/`exports` (e.g. `typeof module`) must not flip
// the file to CJS. Bundler excluded: its resolver can return Esm for
// CJS files (nameless nested "type":"commonjs", exports-map "import"
// key), so the content sniff below is still needed there.
|| (p.options.module_type == options::ModuleType::Esm && !p.options.bundle)
{
exports_kind = js_ast::ExportsKind::Esm;
Comment thread
robobun marked this conversation as resolved.
Resolved
} else if uses_exports_ref || uses_module_ref || p.has_top_level_return || p.has_with_scope
{
Expand Down
4 changes: 3 additions & 1 deletion src/jsc/RuntimeTranspilerCache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ bun_core::declare_scope!(cache, visible);
/// Version 22: Serialize `has_tla` in the cached ESM record flags byte. Entries
/// written before #30888 carried `has_tla=false` for every module; the cache-HIT
/// path reinstates the bug for any previously-cached TLA module (#30887).
const EXPECTED_VERSION: u32 = 22;
/// Version 23: .mjs/.mts/"type":"module" is authoritative over the CommonJS
/// content sniff; `module_type` is now part of the features hash.
const EXPECTED_VERSION: u32 = 23;

/// Source files smaller than this are not written to / read from the on-disk
/// transpiler cache. Originally 50 KiB, which excluded almost every file in a
Expand Down
9 changes: 4 additions & 5 deletions src/jsc/RuntimeTranspilerStore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@ impl RuntimeTranspilerStore {
path: &Fs::Path<'_>,
referrer: String,
loader: Loader,
module_type: ModuleType,
package_json: Option<&PackageJSON>,
) -> *mut c_void {
// The path text is heap-duplicated here and freed in `reset_for_pool` via
Expand Down Expand Up @@ -352,6 +353,7 @@ impl RuntimeTranspilerStore {
vm,
log: bun_ast::Log::init(),
loader,
module_type,
promise: StrongOptional::create(JSValue::from_cell(promise), global_object),
poll_ref: KeepAlive::default(),
fetcher: Fetcher::File,
Expand Down Expand Up @@ -401,6 +403,7 @@ pub struct TranspilerJob {
pub non_threadsafe_input_specifier: OwnedString,
pub non_threadsafe_referrer: OwnedString,
pub loader: Loader,
pub module_type: ModuleType,
pub promise: StrongOptional,
// Note: struct is stored in a HiveArray and crosses to a worker thread;
// raw pointers/BackRefs are used (BACKREF — VM owns the
Expand Down Expand Up @@ -816,11 +819,7 @@ impl TranspilerJob {
&& vm_main_hash == hash
&& strings::eql_long(vm_main, path.text, false);

let module_type: ModuleType = match this_tag {
ResolvedSourceTag::PackageJsonTypeCommonjs => ModuleType::Cjs,
ResolvedSourceTag::PackageJsonTypeModule => ModuleType::Esm,
_ => ModuleType::Unknown,
};
let module_type = self.module_type;

let mut parse_options = ParseOptions {
arena: &arena,
Expand Down
4 changes: 3 additions & 1 deletion src/resolver/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1571,7 +1571,9 @@ impl<'a> Resolver<'a> {
primary: Fs::Path::init_with_namespace(import_path, b"node"),
secondary: None,
},
module_type: options::ModuleType::Esm,
// Embedded code can be CJS (the react-refresh fallback)
// or ESM (bun-framework-react/*.tsx); let the parser decide.
module_type: options::ModuleType::Unknown,
primary_side_effects_data: SideEffects::NoSideEffectsPureData,
flags: ResultFlags::default(),
..Default::default()
Expand Down
45 changes: 36 additions & 9 deletions src/runtime/jsc_hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3852,6 +3852,9 @@ struct LoaderResult<'a> {
specifier: &'a [u8],
/// Always `None` for non-JS-like loaders (not needed there).
package_json: Option<&'a bun_resolver::package_json::PackageJSON>,
/// `"type"` from the nearest ancestor package.json, including nameless ones
/// that `enclosing_package_json` skips. `Unknown` for non-JS-like loaders.
nearest_module_type: ModuleType,
}

/// `options.getLoaderAndVirtualSource` — high-tier body.
Expand Down Expand Up @@ -3975,15 +3978,40 @@ unsafe fn get_loader_and_virtual_source<'a>(
// package.json sniff for `.js`/`.ts` module-type.
let dir = path.name().dir;
let is_js_like = loader.map(|l| l.is_java_script_like()).unwrap_or(true);
let package_json = if is_js_like && bun_paths::is_absolute(dir) {
let (package_json, nearest_module_type) = if is_js_like && bun_paths::is_absolute(dir) {
// SAFETY: per fn contract — `transpiler.resolver` is a value field of
// the VM; `read_dir_info` is re-entrant on the JS thread.
match unsafe { (*jsc_vm).transpiler.resolver.read_dir_info(dir) } {
Ok(Some(dir_info)) => dir_info.package_json().or(dir_info.enclosing_package_json),
_ => None,
Ok(Some(dir_info)) => {
// `enclosing_package_json` skips nameless package.json files, but
// Node's "type" determination uses the nearest one regardless of
// name (e.g. `parse5/dist/cjs/package.json` = {"type":"commonjs"}).
// Per Node's LOOKUP_PACKAGE_SCOPE the walk stops at a directory
// named "node_modules" (format defaults to commonjs).
let mut walk = Some(dir_info);
let nearest_module_type = loop {
match walk {
Some(di) => {
if di.is_node_modules() {
break ModuleType::Unknown;
}
match di.package_json() {
Some(pj) => break pj.module_type,
None => walk = di.get_parent(),
}
}
None => break ModuleType::Unknown,
}
};
Comment thread
robobun marked this conversation as resolved.
Resolved
(
dir_info.package_json().or(dir_info.enclosing_package_json),
nearest_module_type,
)
}
_ => (None, ModuleType::Unknown),
}
} else {
None
(None, ModuleType::Unknown)
};

Ok(LoaderResult {
Expand All @@ -3993,6 +4021,7 @@ unsafe fn get_loader_and_virtual_source<'a>(
is_main,
specifier,
package_json,
nearest_module_type,
})
}

Expand Down Expand Up @@ -4212,11 +4241,8 @@ unsafe fn transpile_file(
}
// regex /\.[jt]s$/
if ext.len() == b".ts".len() && (ext == b".js" || ext == b".ts") {
// Use the package.json module type if it exists.
break 'brk lr
.package_json
.map(|pkg| pkg.module_type)
.unwrap_or(ModuleType::Unknown);
// Use the nearest package.json's "type" (including nameless ones).
break 'brk lr.nearest_module_type;
}
// For JSX/TSX and other extensions, let the file contents decide.
ModuleType::Unknown
Expand Down Expand Up @@ -4272,6 +4298,7 @@ unsafe fn transpile_file(
&lr.path,
(*referrer).dupe_ref(),
concurrent_loader,
module_type,
lr.package_json,
)
};
Expand Down
28 changes: 28 additions & 0 deletions test/bundler/bundler_edgecase.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2771,6 +2771,34 @@ describe("bundler", () => {
expect(out).not.toContain("require_foo\u2014bar");
},
});
itBundled("edgecase/NamelessNestedTypeCommonjsUnderTypeModule", {
files: {
"/entry.js": `import a from "pkg"; console.log(a.hello, a.viaThis);`,
"/node_modules/pkg/package.json": `{"name":"pkg","type":"module","exports":{"default":"./dist/cjs/index.js"}}`,
"/node_modules/pkg/dist/cjs/package.json": `{"type":"commonjs"}`,
"/node_modules/pkg/dist/cjs/index.js": `
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
Object.defineProperty(exports, "hello", { value: "nested-cjs" });
this.viaThis = "this-is-exports";
`,
},
target: "node",
run: { stdout: "nested-cjs this-is-exports" },
});
itBundled("edgecase/ExportsMapImportConditionPointsAtCjs", {
files: {
"/entry.js": `import b from "pkg"; console.log(b.hello);`,
"/node_modules/pkg/package.json": `{"name":"pkg","type":"commonjs","exports":{"import":"./lib/index.js","require":"./lib/index.js"}}`,
"/node_modules/pkg/lib/index.js": `
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
Object.defineProperty(exports, "hello", { value: "import-cjs" });
`,
},
target: "node",
run: { stdout: "import-cjs" },
});
});

for (const backend of ["api", "cli"] as const) {
Expand Down
120 changes: 119 additions & 1 deletion test/cli/run/run-cjs.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { describe, expect, test } from "bun:test";
import { mkdirSync } from "fs";
import { bunEnv, bunExe, tmpdirSync } from "harness";
import { bunEnv, bunExe, tempDir, tmpdirSync } from "harness";
import { join } from "path";

describe.concurrent("run-cjs", () => {
Expand All @@ -18,3 +18,121 @@ describe.concurrent("run-cjs", () => {
expect(stdout).toEqual("hello world\n");
});
});

describe.concurrent("module type from .mjs / package.json is authoritative over module/exports references", () => {
const guardBody = `import path from "node:path";
console.log(JSON.stringify({ m: typeof module, e: typeof exports, sep: typeof path.sep }));
`;
const bareBody = `console.log(JSON.stringify({ m: typeof module, e: typeof exports, thisIsCjs: typeof this === "object" && this != null }));
`;

async function run(cwd: string, entry: string) {
await using proc = Bun.spawn({
cmd: [bunExe(), entry],
cwd,
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
// stderr only appears in the assertion diff when stdout is empty (crash/parse error).
const out = stdout.trim()
? stdout
.trim()
.split("\n")
.map(l => JSON.parse(l))
: [{ crashed: stderr }];
return { out, exitCode };
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

const esmGuard = { m: "undefined", e: "undefined", sep: "string" };
const esmBare = { m: "undefined", e: "undefined", thisIsCjs: false };
const cjsBare = { m: "object", e: "object", thisIsCjs: true };

test(".mjs with import + typeof module runs as ESM", async () => {
using dir = tempDir("mjs-guard", { "guard.mjs": guardBody });
expect(await run(String(dir), "guard.mjs")).toEqual({ out: [esmGuard], exitCode: 0 });
});

test(".mts with import + typeof module runs as ESM", async () => {
using dir = tempDir("mts-guard", { "guard.mts": guardBody });
expect(await run(String(dir), "guard.mts")).toEqual({ out: [esmGuard], exitCode: 0 });
});

test('"type":"module" .js with import + typeof module runs as ESM', async () => {
using dir = tempDir("tm-guard", {
"package.json": `{"type":"module"}`,
"guard.js": guardBody,
});
expect(await run(String(dir), "guard.js")).toEqual({ out: [esmGuard], exitCode: 0 });
});

test(".mjs with no import/export + typeof exports runs as ESM", async () => {
using dir = tempDir("mjs-bare", { "bare.mjs": bareBody });
expect(await run(String(dir), "bare.mjs")).toEqual({ out: [esmBare], exitCode: 0 });
});

test('"type":"module" .js with no import/export + typeof module runs as ESM', async () => {
using dir = tempDir("tm-bare", {
"package.json": `{"type":"module"}`,
"bare.js": bareBody,
});
expect(await run(String(dir), "bare.js")).toEqual({ out: [esmBare], exitCode: 0 });
});

test(".mjs loaded via dynamic import() runs as ESM", async () => {
using dir = tempDir("mjs-dyn", {
"guard.mjs": guardBody,
"bare.mjs": bareBody,
"entry.mjs": `await import("./guard.mjs");\nawait import("./bare.mjs");\n`,
});
expect(await run(String(dir), "entry.mjs")).toEqual({ out: [esmGuard, esmBare], exitCode: 0 });
});

test('.mjs inside "type":"commonjs" package still runs as ESM (extension wins)', async () => {
using dir = tempDir("mjs-in-cjs", {
"package.json": `{"type":"commonjs"}`,
"guard.mjs": guardBody,
"entry.js": `import("./guard.mjs");\n`,
});
expect(await run(String(dir), "entry.js")).toEqual({ out: [esmGuard], exitCode: 0 });
});

test('.js under node_modules with no package.json does not inherit outer "type":"module"', async () => {
using dir = tempDir("nm-no-pkg", {
"package.json": `{"name":"proj","type":"module"}`,
"node_modules/foo/index.js": `module.exports = { m: typeof module, e: typeof exports, thisIsCjs: true };\n`,
"entry.mjs": `import x from "foo"; console.log(JSON.stringify(x));\n`,
});
expect(await run(String(dir), "entry.mjs")).toEqual({ out: [cjsBare], exitCode: 0 });
});

test('nameless nested {"type":"commonjs"} overrides an outer {"type":"module"}', async () => {
using dir = tempDir("nested-cjs", {
"package.json": `{"name":"outer","type":"module"}`,
"dist/cjs/package.json": `{"type":"commonjs"}`,
"dist/cjs/inner/index.js": `Object.defineProperty(exports, "ok", { value: true });\n` + bareBody,
"entry.mjs": `await import("./dist/cjs/inner/index.js");\n`,
});
expect(await run(String(dir), "entry.mjs")).toEqual({ out: [cjsBare], exitCode: 0 });
});

test('.cjs inside "type":"module" package still runs as CJS (extension wins)', async () => {
using dir = tempDir("cjs-in-esm", {
"package.json": `{"type":"module"}`,
"mod.cjs": bareBody,
"entry.mjs": `await import("./mod.cjs");\n`,
});
expect(await run(String(dir), "entry.mjs")).toEqual({ out: [cjsBare], exitCode: 0 });
});

test("ambiguous .js with typeof module still runs as CJS", async () => {
using dir = tempDir("js-sniff", { "sniff.js": bareBody });
expect(await run(String(dir), "sniff.js")).toEqual({ out: [cjsBare], exitCode: 0 });
});

test(".cjs with typeof module still runs as CJS", async () => {
using dir = tempDir("cjs-sniff", { "sniff.cjs": bareBody });
expect(await run(String(dir), "sniff.cjs")).toEqual({ out: [cjsBare], exitCode: 0 });
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
});
1 change: 0 additions & 1 deletion test/js/third_party/es-module-lexer/package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
{
"name": "es-module-lexer-test",
"module": "index.ts",
"type": "module",
"devDependencies": {
"@types/bun": "latest"
},
Expand Down
Loading