Substitute top-level this with undefined in ES modules

src/js_parser/p.rs

@@ -6032,7 +6032,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
                 // compile time using expression substitution here.
                 return Some(Expr {
                     loc,
-                    data: null_value_expr(),
+                    data: js_ast::ExprData::EUndefined(E::Undefined {}),
                 });
             } else {
                 // In a CommonJS module, "this" is supposed to be the same as "exports".
@@ -10275,10 +10275,6 @@ pub(crate) fn null_expr_data() -> js_ast::ExprData {
 pub(crate) fn null_stmt_data() -> js_ast::StmtData {
     js_ast::StmtData::SEmpty(S::Empty {})
 }
-#[inline]
-pub(crate) fn null_value_expr() -> js_ast::ExprData {
-    js_ast::ExprData::ENull(E::Null {})
-}
 
 /// `require()` of an ES module returns a copy of the namespace with
 /// `__esModule` set, and reads `default` off `module.exports`: neither name is

src/js_parser/parse/parse_entry.rs

@@ -265,6 +265,9 @@ impl<'a> Options<'a> {
             hasher.update(b"udfcf=0");
         }
 
+        // `exports_kind` falls back to `module_type` when the content does not decide.
+        hasher.update(&[self.module_type as u8]);
+
         self.features.hash_for_runtime_transpiler(hasher);
     }
 

src/jsc/RuntimeTranspilerCache.rs

@@ -57,7 +57,8 @@ bun_core::declare_scope!(cache, visible);
 /// Version 27: ModuleInfo string table holds Latin-1 / UTF-16 bodies, not WTF-8.
 /// Version 28: the define table and `--drop` entries participate in the features hash.
 /// Version 29: `new Array(x, ...spread)` is no longer folded into an array literal.
-const EXPECTED_VERSION: u32 = 29;
+/// Version 30: ESM top-level `this` is `undefined`, not `null`; `module_type` is in the features hash.
+const EXPECTED_VERSION: u32 = 30;
 
 /// 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

test/bundler/esbuild/default.test.ts

@@ -5313,7 +5313,7 @@ describe.concurrent("bundler", () => {
       "/node_modules/pkg/file2.js": `export default [this, this]`,
     },
     run: {
-      stdout: "[ null, null ] [ null, null ]",
+      stdout: "[ undefined, undefined ] [ undefined, undefined ]",
     },
   });
   itBundled("default/QuotedProperty", {

test/bundler/transpiler/runtime-transpiler.test.ts

@@ -252,3 +252,48 @@ describe("unterminated string literals in large files", () => {
     expect(exitCode).toBe(1);
   });
 });
+
+// https://github.com/oven-sh/bun/issues/32167
+describe.concurrent("top-level this", () => {
+  const files = {
+    // the issue's repro: an import-only entry point, `this` captured by an arrow
+    "import-only.js": `import { EventEmitter } from "node:events";
+const emitter = new EventEmitter();
+emitter.on("event", () => {
+  console.log(typeof this, this === undefined);
+});
+emitter.emit("event");
+`,
+    "export-only.mjs": `export {};\nconsole.log(typeof this, this === undefined);\n`,
+    // an imported (not entry point) ES module goes through the async transpiler path
+    "imports-esm.js": `import { kind, isUndefined } from "./dep.mjs";\nconsole.log(kind, isUndefined);\n`,
+    "dep.mjs": `export const kind = typeof this;\nexport const isUndefined = this === undefined;\n`,
+    // CommonJS keeps `this === module.exports`, also when imported from an ES module
+    // in a "type": "module" package
+    "package.json": `{ "type": "module" }`,
+    "cjs.cjs": `console.log(typeof this, this === module.exports);\n`,
+    "imports-cjs.js": `import dep from "./dep.cjs";\nconsole.log(dep.kind, dep.isExports);\n`,
+    "dep.cjs": `var self = this;\nmodule.exports = { kind: typeof self, isExports: self === exports };\n`,
+  };
+
+  for (const [file, expected] of [
+    ["import-only.js", "undefined true\n"],
+    ["export-only.mjs", "undefined true\n"],
+    ["imports-esm.js", "undefined true\n"],
+    ["cjs.cjs", "object true\n"],
+    ["imports-cjs.js", "object true\n"],
+  ]) {
+    test(file, async () => {
+      using dir = tempDir("top-level-this", files);
+      await using proc = Bun.spawn({
+        cmd: [bunExe(), file],
+        env: bunEnv,
+        cwd: String(dir),
+        stderr: "pipe",
+      });
+      const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+      expect({ stdout, stderr }).toEqual({ stdout: expected, stderr: "" });
+      expect(exitCode).toBe(0);
+    });
+  }
+});

test/bundler/transpiler/transpiler.test.js

@@ -3722,6 +3722,22 @@ class Foo {
     );
   });
 
+  // https://github.com/oven-sh/bun/issues/32167
+  it("top-level this is undefined in an ES module and exports in CommonJS", () => {
+    expectPrinted_("export {}; console.log(this)", "export {};\nconsole.log(undefined)");
+    expectPrinted_('import "foo"; console.log(this)', 'import"foo";\nconsole.log(undefined)');
+    expectPrinted_(
+      "export {}; console.log(typeof this, this === undefined)",
+      'export {};\nconsole.log("undefined", true)',
+    );
+    expectPrinted_("export {}; const f = () => [this]", "export {};\nconst f = () => [undefined]");
+    // bare `delete undefined` is a SyntaxError in strict mode code
+    expectPrinted_("export {}; delete this", "export {};\ndelete (0, undefined)");
+    expectPrinted_("export {}; function f() { return this }", "export {};\nfunction f() {\n  return this;\n}");
+
+    expectPrinted_("console.log(this)", "console.log(exports)");
+  });
+
   it("declarations named eval or arguments, and reserved words, in strict mode", () => {
     expectParseError(
       '"use strict"; var arguments = 1',

test/cli/run/transpiler-cache.test.ts

@@ -123,6 +123,27 @@ describe("transpiler cache", () => {
     expect(await bunRun(join(temp_dir, "main.js"), env)).toSpawn(expected);
     expect(!existsSync(cache_dir)).toBeTrue();
   });
+  test("byte-identical files with different package.json types do not share entries", async () => {
+    // A file with no import/export and no module/exports references is an ES
+    // module (strict) under "type": "module" and CommonJS (sloppy) under
+    // "type": "commonjs", so the package.json "type" must be part of the cache key.
+    const data = dummyFile(50 * 1024, "1", {
+      code: `(() => { try { undeclaredVariable = 1; return "sloppy"; } catch { return "strict"; } })()`,
+    });
+    mkdirSync(join(temp_dir, "esm"));
+    mkdirSync(join(temp_dir, "cjs"));
+    writeFileSync(join(temp_dir, "esm", "package.json"), '{ "type": "module" }');
+    writeFileSync(join(temp_dir, "cjs", "package.json"), '{ "type": "commonjs" }');
+    writeFileSync(join(temp_dir, "esm", "a.js"), data);
+    writeFileSync(join(temp_dir, "cjs", "a.js"), data);
+
+    expect(await bunRun(join(temp_dir, "esm", "a.js"), env)).toSpawn("strict");
+    expect(newCacheCount()).toBe(1);
+    // Same bytes in a CommonJS package must not be served the cached ES module entry.
+    expect(await bunRun(join(temp_dir, "cjs", "a.js"), env)).toSpawn("sloppy");
+    // And the reverse direction, now that the CommonJS entry was written last.
+    expect(await bunRun(join(temp_dir, "esm", "a.js"), env)).toSpawn("strict");
+  });
   test("it is indeed content addressable", async () => {
     writeFileSync(join(temp_dir, "a.js"), dummyFile(50 * 1024, "1", "b"));
     expect(await bunRun(join(temp_dir, "a.js"), env)).toSpawn("b");