diff --git a/src/js_parser/fold.rs b/src/js_parser/fold.rs index 911e1e76de30..91d382b03168 100644 --- a/src/js_parser/fold.rs +++ b/src/js_parser/fold.rs @@ -443,7 +443,10 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O } } js_ast::ExprData::EString(str_) => { - if p.options.features.minify_syntax { + if p.options.features.minify_syntax + && !identifier_opts.is_delete_target() + && identifier_opts.assign_target() == js_ast::AssignTarget::None + { // minify "long-string".length to 11 if name == b"length" { if let Some(len) = e_string_javascript_length(&str_) { @@ -493,11 +496,14 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O } } js_ast::ExprData::EImportMeta(_) => { - if name == b"main" { + let can_inline = !identifier_opts.is_delete_target() + && identifier_opts.assign_target() == js_ast::AssignTarget::None; + + if can_inline && name == b"main" { return Some(p.value_for_import_meta_main(false, target.loc)); } - if name == b"hot" { + if can_inline && name == b"hot" { return Some(Expr { data: js_ast::ExprData::ESpecial( if p.options.features.hot_module_reloading { @@ -511,9 +517,10 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O } // Inline import.meta properties for Bake - if p.options.framework.is_some() - || (p.options.bundle - && p.options.output_format == js_parser::options::Format::Cjs) + if can_inline + && (p.options.framework.is_some() + || (p.options.bundle + && p.options.output_format == js_parser::options::Format::Cjs)) { if name == b"dir" || name == b"dirname" { // Inline import.meta.dir @@ -666,6 +673,15 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O } E::Special::HotEnabled | E::Special::HotDisabled => { let enabled = p.options.features.hot_module_reloading; + // !enabled rewrites produce values (undefined/{}), so keep the + // reference under delete/assign. enabled rewrites produce hmr. + // refs and must run (HotEnabled prints as throwing `hmr.indirectHot`). + if !enabled + && (identifier_opts.is_delete_target() + || identifier_opts.assign_target() != js_ast::AssignTarget::None) + { + return None; + } if name == b"data" { return Some(if enabled { Expr { diff --git a/src/js_parser/visit/visit_expr.rs b/src/js_parser/visit/visit_expr.rs index 260a937e154b..5b0304106ba0 100644 --- a/src/js_parser/visit/visit_expr.rs +++ b/src/js_parser/visit/visit_expr.rs @@ -1057,8 +1057,13 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O let target = e_.target.unwrap_inlined(); let index = e_.index.unwrap_inlined(); - // `[x][0] = v` writes into the temporary, not `x`. - if p.options.features.minify_syntax && in_.assign_target == js_ast::AssignTarget::None { + // Folding a property reference to a value is unsafe where the + // reference itself is observed (delete result, assign target; the call + // receiver case is handled inside via `(0, x)`). + if p.options.features.minify_syntax + && !is_delete_target + && in_.assign_target == js_ast::AssignTarget::None + { if let Some(number) = index.data.as_e_number() { if number.value() >= 0.0 && number.value() < (usize::MAX as f64) @@ -1224,6 +1229,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O } } Op::UnDelete => { + p.delete_target = e_.value.data; p.visit_expr_in_out(&mut e_.value, ExprIn::default()); } _ => { diff --git a/src/jsc/RuntimeTranspilerCache.rs b/src/jsc/RuntimeTranspilerCache.rs index 2eee3816433a..ffbfee190187 100644 --- a/src/jsc/RuntimeTranspilerCache.rs +++ b/src/jsc/RuntimeTranspilerCache.rs @@ -51,7 +51,8 @@ bun_core::declare_scope!(cache, visible); /// Version 25: Every ModuleInfo record carries a trailing FetchParameters slot /// so ImportEntry/ExportEntry/StarExportEntry moduleRequestType matches JSC's /// after WebKit 90b2ecf79ae3 keyed m_loadedModules on (specifier, type). -const EXPECTED_VERSION: u32 = 25; +/// Version 26: `delete`/assign/call targets no longer fold under minify_syntax. +const EXPECTED_VERSION: u32 = 26; /// 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 diff --git a/test/bundler/bundler_cjs2esm.test.ts b/test/bundler/bundler_cjs2esm.test.ts index c0860af1dfec..d8fa0a8c351c 100644 --- a/test/bundler/bundler_cjs2esm.test.ts +++ b/test/bundler/bundler_cjs2esm.test.ts @@ -392,6 +392,36 @@ describe("bundler", () => { stdout: '[[{"xyz":456},456],[{"xyz":123},123],[{"xyz":456},456],[{"xyz":123},123]]', }, }); + itBundled("cjs2esm/DeleteExportsPropertyDeopt", { + files: { + "/entry.js": /* js */ ` + import * as lib from './lib.js'; + console.log(lib.a, lib.b); + `, + "/lib.js": /* js */ ` + exports.a = 1; + exports.b = 2; + delete exports.a; + `, + }, + cjs2esm: { unhandled: ["/lib.js"] }, + run: { stdout: "undefined 2" }, + }); + itBundled("cjs2esm/DeleteModuleExportsPropertyDeopt", { + files: { + "/entry.js": /* js */ ` + import * as lib from './lib.js'; + console.log(lib.a, lib.b); + `, + "/lib.js": /* js */ ` + module.exports.a = 1; + module.exports.b = 2; + delete module.exports.a; + `, + }, + cjs2esm: { unhandled: ["/lib.js"] }, + run: { stdout: "undefined 2" }, + }); // `delete (null ?? exports.a)` evaluates to a value, so the result is `true` // with no effect on the property. Under cjs2esm, `exports.a` is rewritten to // an `ECommonjsExportIdentifier`; the printer has to re-wrap it as `(0, ...)` diff --git a/test/bundler/transpiler/assign-to-import.test.ts b/test/bundler/transpiler/assign-to-import.test.ts index 655ec7f1c997..80e255ff75ba 100644 --- a/test/bundler/transpiler/assign-to-import.test.ts +++ b/test/bundler/transpiler/assign-to-import.test.ts @@ -81,6 +81,17 @@ describe("assigning to an imported binding", () => { `import * as ns from "./m.mjs"; const k = "x"; ns[k] = 5;\n`, 'Cannot assign to property on import "ns"', ], + ["delete namespace property", `import * as ns from "./m.mjs"; delete ns.x;\n`, 'Cannot assign to import "x"'], + [ + "delete string-index namespace property", + `import * as ns from "./m.mjs"; delete ns["x"];\n`, + 'Cannot assign to import "x"', + ], + [ + "delete computed namespace property", + `import * as ns from "./m.mjs"; const k = "x"; delete ns[k];\n`, + 'Cannot assign to property on import "ns"', + ], ])("bun build still rejects it: %s", async (_name, entry, diagnostic) => { using dir = tempDir("assign-to-import-build", { "m.mjs": mod, diff --git a/test/bundler/transpiler/transpiler.test.js b/test/bundler/transpiler/transpiler.test.js index f19b5896a37d..cdf37c0200e5 100644 --- a/test/bundler/transpiler/transpiler.test.js +++ b/test/bundler/transpiler/transpiler.test.js @@ -139,6 +139,75 @@ describe("Bun.Transpiler", () => { it("works nested", () => { ts.expectPrintedMin_('const a = ["hey"][0][0];', 'const a = "h"'); }); + it("bails out when the index is a delete target", () => { + // `a[n]` is a property reference; folding it to a value changes the + // result of `delete`. + ts.expectPrintedMin_("x = delete [y][0]", "x = delete [y][0]"); + ts.expectPrintedMin_("x = delete [y.z][0]", "x = delete [y.z][0]"); + ts.expectPrintedMin_("x = delete { f: y }.f", "x = delete { f: y }.f"); + ts.expectPrintedMin_("x = delete { f: y }['f']", "x = delete { f: y }.f"); + ts.expectPrintedMin_('x = delete "foo"[2]', 'x = delete "foo"[2]'); + ts.expectPrintedMin_('x = delete "foo".length', 'x = delete "foo".length'); + // Comma / `?:` / `??` / `||` / `&&` produce a value, not a reference; + // when a fold hoists the live arm up to the delete, the printer + // re-wraps it using WAS_ORIGINALLY_DELETE_OF_IDENTIFIER_OR_PROPERTY_ACCESS. + ts.expectPrinted_("x = delete (true ? a.b : 0)", "x = delete (0, a.b)"); + ts.expectPrinted_("x = delete (true ? a : 0)", "x = delete (0, a)"); + ts.expectPrintedMin_("x = delete (0, a.b)", "x = delete (0, a.b)"); + ts.expectPrintedMin_("x = delete (null ?? a.b)", "x = delete (0, a.b)"); + ts.expectPrintedMin_("x = delete (0 || a.b)", "x = delete (0, a.b)"); + ts.expectPrintedMin_("x = delete (1 && a.b)", "x = delete (0, a.b)"); + // Still inlined outside those positions. + ts.expectPrintedMin_('x = "foo".length', "x = 3"); + ts.expectPrintedMin_("x = delete [y][0].z", "x = delete y.z"); + }); + it("does not inline an enum member under delete", () => { + const pre = "enum E { A = 1 }\n"; + const lastLine = out => out.trimEnd().split("\n").at(-1); + expect(lastLine(ts.parsed(pre + "x = delete E.A;", false))).toBe("x = delete E.A;"); + expect(lastLine(ts.parsed(pre + 'x = delete E["A"];', false))).toBe('x = delete E["A"];'); + expect(lastLine(ts.parsedMin(pre + "x = delete E.A;", false))).toBe("x = delete E.A;"); + expect(lastLine(ts.parsedMin(pre + 'x = delete E["A"];', false))).toBe("x = delete E.A;"); + // Still inlined when read. + expect(lastLine(ts.parsed(pre + "x = E.A;", false))).toBe("x = 1 /* A */;"); + }); + it("does not substitute --define for a delete target", () => { + // `user_undefined` is defined as `undefined` in the transpiler config above. + ts.expectPrintedMin_("x = delete user_undefined", "x = delete user_undefined"); + // A later read is still substituted (delete_target is per-node, not per-symbol). + ts.expectPrintedMin_( + "x = delete user_undefined; y = user_undefined;", + "x = delete user_undefined;\ny = void 0;\n", + ); + }); + it("does not inline import.meta. under delete or assignment", () => { + // Inlining would produce `delete undefined` (strict-mode SyntaxError) or + // `undefined = 5` for the HotDisabled value. + ts.expectPrinted_("x = delete import.meta.hot", "x = delete import.meta.hot"); + ts.expectPrinted_("x = delete import.meta.main", "x = delete import.meta.main"); + ts.expectPrinted_("import.meta.hot = 5", "import.meta.hot = 5"); + ts.expectPrinted_("import.meta.main = 5", "import.meta.main = 5"); + ts.expectPrinted_("x = delete import.meta.hot.accept", "x = delete undefined.accept"); + ts.expectPrinted_("import.meta.hot.accept = fn", "undefined.accept = fn"); + // Reads are still inlined. + ts.expectPrinted_("x = import.meta.hot", "x = undefined"); + }); + it("preserves delete semantics at runtime", async () => { + const src = ` + var obj = { p: 1 }; + var r1 = delete [obj.p][0]; + var k = { f: 7 }; + var r2 = delete { f: k.f }.f; + var q = { p: 1 }; + delete (0, q.p); + console.log(JSON.stringify([obj.p, r1, k.f, r2, q.p])); + `; + await using proc = Bun.spawn({ cmd: [bunExe(), "-e", src], env: bunEnv, stderr: "pipe" }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe("[1,true,7,true,1]\n"); + expect(exitCode).toBe(0); + }); it("bails out when the array item is an optional chain", () => { // Folding `[a?.b][0]` to `a?.b` is unsafe when the result lands as the // target of a surrounding optional-chain continuation: the two chains