diff --git a/src/bundler/LinkerContext.rs b/src/bundler/LinkerContext.rs index 4bda69e9a23b..e06d1283335d 100644 --- a/src/bundler/LinkerContext.rs +++ b/src/bundler/LinkerContext.rs @@ -2403,6 +2403,15 @@ impl<'a> LinkerContext<'a> { ..Default::default() }]; + debug_assert!( + flags.wrap != WrapKind::Cjs + || ast.commonjs_named_exports.count() == 0 + || ast + .flags + .intersects(AstFlags::USES_EXPORTS_REF | AstFlags::USES_MODULE_REF), + "the wrapper prints `exports.x` for a lifted export and has no `exports` parameter" + ); + let print_options = js_printer::Options { bundling: true, // TODO: IIFE diff --git a/src/bundler/linker_context/scanImportsAndExports.rs b/src/bundler/linker_context/scanImportsAndExports.rs index 73dee5b22d28..352527738e5a 100644 --- a/src/bundler/linker_context/scanImportsAndExports.rs +++ b/src/bundler/linker_context/scanImportsAndExports.rs @@ -111,6 +111,8 @@ pub(crate) fn scan_imports_and_exports( let exports_refs: *mut [Ref] = ast.exports_ref; let module_refs: *mut [Ref] = ast.module_ref; let wrapper_refs: *mut [Ref] = ast.wrapper_ref; + let commonjs_named_exports: *mut [bun_ast::ast_result::CommonJSNamedExports] = + ast.commonjs_named_exports; let parts_list: *mut [PartList] = ast.parts; // Element is a *mutable* nullable // pointer (`BundledAst.css: Option<*mut BundlerStyleSheet>`). @@ -621,9 +623,15 @@ pub(crate) fn scan_imports_and_exports( col!(flags)[source_index] = flag; } - let is_lifted_commonjs = col_ref!(ast_flags_list)[source_index] - .contains(AstFlags::COMMONJS_LIFTED_TO_ESM); - if is_lifted_commonjs && flag.wrap == WrapKind::Cjs { + let ast_flags = col_ref!(ast_flags_list)[source_index]; + let is_lifted_commonjs = ast_flags.contains(AstFlags::COMMONJS_LIFTED_TO_ESM); + let wrapper_has_exports = + ast_flags.intersects(AstFlags::USES_EXPORTS_REF | AstFlags::USES_MODULE_REF); + if flag.wrap == WrapKind::Cjs + && (is_lifted_commonjs + || (!wrapper_has_exports + && col_ref!(commonjs_named_exports)[source_index].count() > 0)) + { // The wrapper prints `exports.x = ...` again, so it takes `exports`. col!(ast_flags_list)[source_index].insert(AstFlags::USES_EXPORTS_REF); } else if is_lifted_commonjs && export_kind != ExportsKind::Cjs { diff --git a/test/bundler/bundler_cjs2esm.test.ts b/test/bundler/bundler_cjs2esm.test.ts index 57e31a43c888..5e90112e218b 100644 --- a/test/bundler/bundler_cjs2esm.test.ts +++ b/test/bundler/bundler_cjs2esm.test.ts @@ -1706,6 +1706,125 @@ describe("bundler", () => { stdout: "helped:in", }, }); + // A wrapped file prints its lifted `exports.x = ...` as written, so its wrapper takes `exports`. + // Here dead code or a removed statement makes the file CommonJS and counts no use of `exports`. + const wrappedWithNoUseOfExports: Record = { + "/dead-branch.cjs": `exports.a = 1; if (false) { console.log(this); }`, + "/dead-ternary.cjs": `exports.a = 1; exports.b = true ? 2 : this;`, + "/dead-or.cjs": `exports.a = 1; exports.b = 0 || 2 || this;`, + "/dead-and.cjs": `exports.a = 1; false && this.x;`, + "/dead-call.cjs": `exports.a = 1; function f() { return 1; } if (false) { f.call(this); }`, + "/removed-this.cjs": `exports.a = 1; this;`, + "/removed-arrow.cjs": `exports.a = 1; (() => this);`, + "/removed-delete.cjs": `exports.a = 1; (function () { delete exports.a; });`, + "/removed-delete-all.cjs": `exports.a = 1; (function () { delete module.exports; });`, + "/removed-call.cjs": `exports.a = 1; (function () { module.exports(); });`, + "/removed-delete-member.cjs": `exports.a = 1; (function () { delete module.exports.a; });`, + "/removed-store.cjs": `exports.a = 1; (function () { module.exports = {}; });`, + }; + // A counted use of `exports` already gives these wrappers the parameter. + const wrappedWithAUseOfExports: Record = { + "/live-this.cjs": `exports.a = 1; exports.b = this === exports;`, + "/live-typeof-this.cjs": `exports.a = 1; exports.b = typeof this;`, + "/later-use.cjs": `exports.a = 1; var self = 1 ? globalThis : this; exports.b = typeof self;`, + "/this-first.cjs": `this; exports.a = 1;`, + }; + // Dead code of these kinds does not make the file CommonJS. + const stillLifted: Record = { + "/dead-delete.cjs": `exports.a = 1; if (false) { delete exports.a; }`, + "/dead-store.cjs": `exports.a = 1; if (false) { module.exports = {}; }`, + }; + const importAs: Record string> = { + Default: (file, i) => `import f${i} from ".${file}"; const a${i} = f${i}.a;`, + Named: (file, i) => `import { a as a${i} } from ".${file}";`, + Star: (file, i) => `import * as f${i} from ".${file}"; const a${i} = f${i}.a;`, + Require: (file, i) => `const a${i} = require(".${file}").a;`, + }; + const readA = (files: string[], importLine: (file: string, i: number) => string) => + files.map(importLine).join("\n") + `\nconsole.log(${files.map((_, i) => `a${i}`).join(", ")});`; + for (const [form, importLine] of Object.entries(importAs)) { + const wrapped = Object.keys(wrappedWithNoUseOfExports); + itBundled(`cjs2esm/WrappedFileWithLiftedExportsTakesExports${form}Import`, { + files: { "/entry.js": readA(wrapped, importLine), ...wrappedWithNoUseOfExports }, + run: { stdout: wrapped.map(() => "1").join(" ") }, + }); + } + itBundled("cjs2esm/WrappedFileWithLiftedExportsTakesExports", { + files: { + "/entry.js": readA( + Object.keys({ ...wrappedWithNoUseOfExports, ...wrappedWithAUseOfExports, ...stillLifted }), + importAs.Default, + ), + ...wrappedWithNoUseOfExports, + ...wrappedWithAUseOfExports, + ...stillLifted, + }, + cjs2esm: { unhandled: Object.keys({ ...wrappedWithNoUseOfExports, ...wrappedWithAUseOfExports }) }, + onAfterBundle(api) { + api.expectFile("/out.js").not.toContain("__commonJS(function() {"); + }, + run: { stdout: Array(18).fill("1").join(" ") }, + }); + itBundled("cjs2esm/WrappedEntryPointWithLiftedExportsTakesExports", { + files: { + "/entry.cjs": `exports.a = 1; if (false) { console.log(this); } console.log("loaded");`, + }, + run: { stdout: "loaded" }, + }); + itBundled("cjs2esm/WrappedFileWithLiftedExportsTakesExportsMinifyIdentifiers", { + files: { + "/entry.js": readA(["/dead-branch.cjs", "/removed-this.cjs", "/removed-store.cjs"], importAs.Default), + ...wrappedWithNoUseOfExports, + }, + minifyIdentifiers: true, + run: { stdout: "1 1 1" }, + }); + itBundled("cjs2esm/WrappedFileWithLiftedExportsTakesExportsSplitting", { + files: { + "/entry.js": `const lib = await import("./dead-branch.cjs"); console.log(lib.default.a);`, + ...wrappedWithNoUseOfExports, + }, + outdir: "/out", + splitting: true, + run: { file: "/out/entry.js", stdout: "1" }, + }); + // The production define makes the branch dead. + itBundled("cjs2esm/ThisInDeadDevelopmentBranchKeepsExports", { + files: { + "/entry.js": `import lib from "./lib.cjs"; console.log(lib.a);`, + "/lib.cjs": /* js */ ` + exports.a = 1; + if (process.env.NODE_ENV !== "production") { + console.log("dev", this === exports); + } + `, + }, + define: { "process.env.NODE_ENV": '"production"' }, + run: { stdout: "1" }, + }); + // `process.browser` is `false` for these targets without a flag. + for (const target of ["bun", "node"] as const) { + itBundled(`cjs2esm/ThisInDeadProcessBrowserBranchKeepsExports_${target}`, { + files: { + "/entry.js": `import lib from "./lib.cjs"; console.log(lib.a, lib.a2);`, + "/lib.cjs": /* js */ ` + exports.a = 1; + if (process.browser) { this.a2 = 1; } + `, + }, + target, + run: { stdout: "1 undefined" }, + }); + } + // The linker wraps this file for the default import. No parser mark is involved. + itBundled("cjs2esm/LinkerWrappedFileWithLiftedExportsTakesExports", { + files: { + "/entry.js": `import React from "react"; console.log(React.a);`, + "/node_modules/react/index.js": `import { z } from "./z.js"; exports.a = z;`, + "/node_modules/react/z.js": `export const z = 7;`, + }, + run: { stdout: "7" }, + }); itBundled("cjs2esm/DefaultImportJsxClassicRuntime", { files: { "/entry.jsx": /* jsx */ ` diff --git a/test/bundler/esbuild/extra.test.ts b/test/bundler/esbuild/extra.test.ts index 7cd5621a0b63..bf5566b7f8be 100644 --- a/test/bundler/esbuild/extra.test.ts +++ b/test/bundler/esbuild/extra.test.ts @@ -611,7 +611,6 @@ describe("bundler", () => { run: true, }); itBundled("extra/DefineExports1", { - todo: true, files: { "in.js": `if (exports.x !== void 0) throw 'fail'; this;`, }, @@ -619,7 +618,6 @@ describe("bundler", () => { run: true, }); itBundled("extra/DefineExports2", { - todo: true, files: { "in.js": `if (exports.foo !== void 0) throw 'fail'; this;`, }, @@ -663,7 +661,6 @@ describe("bundler", () => { run: true, }); itBundled("extra/DefineExportsArray2", { - todo: true, files: { "in.js": `if (exports.foo !== void 0) throw 'fail'; this`, },