js_parser: keep assignment and delete targets as references (string .length, import.meta, module.id, --define)

src/js_parser/fold.rs

@@ -349,13 +349,15 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
                             });
                         } else if p.options.bundle
                             && name == b"id"
+                            && !identifier_opts.is_delete_target()
                             && identifier_opts.assign_target() == js_ast::AssignTarget::None
                         {
                             // inline module.id
                             p.ignore_usage(p.module_ref);
                             return Some(p.new_expr(e_string_init(p.source.path.pretty), name_loc));
                         } else if p.options.bundle
                             && name == b"filename"
+                            && !identifier_opts.is_delete_target()
                             && identifier_opts.assign_target() == js_ast::AssignTarget::None
                         {
                             // inline module.filename
@@ -365,6 +367,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
                             );
                         } else if p.options.bundle
                             && name == b"path"
+                            && !identifier_opts.is_delete_target()
                             && identifier_opts.assign_target() == js_ast::AssignTarget::None
                         {
                             // inline module.path
@@ -443,7 +446,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 +499,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 +520,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
@@ -665,6 +675,13 @@ 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;
+                        // Only the disabled rewrites produce values; the enabled ones are `hmr.<name>` references.
+                        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 {

src/js_parser/visit/visit_expr.rs

@@ -249,8 +249,6 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
             p.symbols[result.r#ref.inner_index() as usize].set_has_been_assigned_to(true);
         }
 
-        let mut original_name: Option<&[u8]> = None;
-
         // Substitute user-specified defines for unbound symbols
         if p.symbols[e_.ref_.inner_index() as usize].kind == js_ast::symbol::Kind::Unbound
             && !result.is_inside_with_scope
@@ -274,8 +272,6 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
                         *e = newvalue;
                         return;
                     }
-
-                    original_name = def.original_name();
                 }
 
                 // Copy the side effect flags over in case this expression is unused
@@ -320,7 +316,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
         *e = p.handle_identifier(
             expr.loc,
             e_,
-            original_name,
+            None,
             IdentifierOpts::default()
                 .with_assign_target(in_.assign_target)
                 .with_is_delete_target(is_delete_target)
@@ -1051,8 +1047,11 @@ 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 {
+        // `[x][0] = v` and `delete [x][0]` act on the temporary, not on `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)
@@ -1218,6 +1217,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());
             }
             _ => {

src/jsc/RuntimeTranspilerCache.rs

@@ -55,7 +55,8 @@ bun_core::declare_scope!(cache, visible);
 /// offsets picked by a header byte) plus a body of tagged records with
 /// u8/u16/u32 ids and implied slots dropped, instead of fixed u32 arrays.
 /// Version 27: ModuleInfo string table holds Latin-1 / UTF-16 bodies, not WTF-8.
-const EXPECTED_VERSION: u32 = 27;
+/// Version 28: delete and assignment targets are no longer folded to a value (#21210).
+const EXPECTED_VERSION: u32 = 28;
 
 /// 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/bundler_cjs2esm.test.ts

@@ -509,6 +509,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, ...)`

test/bundler/esbuild/default.test.ts

@@ -4625,35 +4625,68 @@ describe.concurrent("bundler", () => {
       stdout: "1\n2",
     },
   });
-  // TODO: this doesnt warn in esbuild ???
-  // itBundled("default/DefineAssignWarning", {
-  //   // GENERATED
-  //   files: {
-  //     "/read.js": /* js */ `
-  //       console.log(
-  //         [a, b.c, b['c']],
-  //         [d, e.f, e['f']],
-  //         [g, h.i, h['i']],
-  //       )
-  //     `,
-  //     "/write.js": /* js */ `
-  //       console.log(
-  //         [a = 0, b.c = 0, b['c'] = 0],
-  //         [d = 0, e.f = 0, e['f'] = 0],
-  //         [g = 0, h.i = 0, h['i'] = 0],
-  //       )
-  //     `,
-  //   },
-  //   entryPoints: ["/read.js", "/write.js"],
-  //   define: {
-  //     a: "null",
-  //     "b.c": "null",
-  //     d: "ident",
-  //     "e.f": "ident",
-  //     g: "dot.chain",
-  //     "h.i": "dot.chain",
-  //   },
-  // });
+  // A define with a constant value (`a`) is only substituted where it is read;
+  // as an assignment target the identifier stays as written, since `null = 0`
+  // is a syntax error. esbuild also logs an "assign-to-define" warning there,
+  // bun does not. A define whose value is an identifier (`d`) is substituted
+  // in both positions.
+  itBundled("default/DefineAssignWarning", {
+    files: {
+      "/read.js": /* js */ `
+        capture(a);
+        capture(d);
+      `,
+      "/write.js": /* js */ `
+        capture(a = 1);
+        capture(a += 1);
+        capture(a++);
+        capture(--a);
+        capture([a] = [3]);
+        capture({ x: a } = { x: 4 });
+        capture(a ??= 5);
+        for (a in { 5: 0 });
+        for (a of [6]);
+        capture(d = 7);
+        console.log(a, globalThis.a, globalThis.ident);
+      `,
+    },
+    entryPoints: ["/read.js", "/write.js"],
+    define: {
+      a: "null",
+      d: "ident",
+    },
+    runtimeFiles: {
+      // The output is an ES module, so the globals it assigns to must exist.
+      "/test.js": /* js */ `
+        globalThis.capture = x => x;
+        globalThis.a = undefined;
+        globalThis.ident = undefined;
+        await import("./out/write.js");
+      `,
+    },
+    onAfterBundle(api) {
+      expect(api.captureFile("/out/read.js")).toEqual(["null", "ident"]);
+      expect(api.captureFile("/out/write.js")).toEqual([
+        "a = 1",
+        "a += 1",
+        "a++",
+        "--a",
+        "[a] = [3]",
+        "{ x: a } = { x: 4 }",
+        "a ??= 5",
+        "ident = 7",
+      ]);
+      const write = api.readFile("/out/write.js");
+      expect(write).toContain("for (a in { 5: 0 })");
+      expect(write).toContain("for (a of [6])");
+    },
+    run: {
+      file: "/test.js",
+      // The read of `a` is replaced with the define, the writes went to the
+      // real global.
+      stdout: "null 6 7",
+    },
+  });
   itBundled("default/KeepNamesTreeShaking", {
     todo: true, // TODO: Full keepNames implementation with Object.defineProperty
     files: {