dev server: make each client ES module closure strict

src/bundler/linker_context/generateCodeForFileInChunkJS.rs

@@ -14,7 +14,7 @@ use crate::options::Format as OutputFormat;
 use crate::{Chunk, Index, LinkerContext, Part, PartRange, WrapKind};
 
 use bun_ast::StoreRef;
-use bun_ast::{B, Binding, E, Expr, G, Ref, S, Stmt};
+use bun_ast::{B, Binding, E, ExportsKind, Expr, G, Ref, S, Stmt, StrictModeKind, Target};
 use bun_js_parser::lexer as js_lexer;
 
 use super::convert_stmts_for_chunk::convert_stmts_for_chunk;
@@ -67,6 +67,28 @@ pub fn generate_code_for_file_in_chunk_js(
 
             let hmr_api_ref = ast.wrapper_ref;
 
+            // ES module code is strict, but the page runs a hot-update chunk as a classic script.
+            let has_es_module_semantics = ast.exports_kind == ExportsKind::Esm
+                || matches!(
+                    ast.module_scope.strict_mode,
+                    StrictModeKind::ImplicitStrictModeImport
+                        | StrictModeKind::ImplicitStrictModeExport
+                        | StrictModeKind::ImplicitStrictModeTopLevelAwait
+                );
+            if has_es_module_semantics
+                && ast.target == Target::Browser
+                && !ast.flags.contains(AstFlags::HAS_LAZY_EXPORT)
+            {
+                bun_core::handle_oom(stmts.inside_wrapper_prefix.append_non_dependency(
+                    Stmt::alloc(
+                        S::Directive {
+                            value: bun_ast::StoreStr::new(b"use strict"),
+                        },
+                        bun_ast::Loc::EMPTY,
+                    ),
+                ));
+            }
+
             for part in parts {
                 let part_stmts: &[Stmt] = part.stmts.slice();
                 if let Err(err) =

test/bake/dev/hot.test.ts

@@ -691,3 +691,161 @@ devTest("dev.write resolves only after the new module body has run", {
     expect(await c.js`globalThis.marker`).toBe("updated");
   },
 });
+
+// Sloppy code gives "object no throw made a global 2". Each file gets its own
+// `undeclaredName`, so one sloppy file cannot define the name for the next one.
+const strictModeProbe = (undeclaredName: string) => `(function () {
+  const thisOfPlainCall = (function () { return typeof this; })();
+  let frozenWrite;
+  try { Object.freeze({ x: 1 }).x = 2; frozenWrite = "no throw"; } catch (e) { frozenWrite = e.name; }
+  let undeclaredWrite;
+  try { ${undeclaredName} = 1; undeclaredWrite = "made a global"; } catch (e) { undeclaredWrite = e.name; }
+  const argumentAfterWrite = (function (a) { a = 2; return arguments[0]; })(1);
+  return [thisOfPlainCall, frozenWrite, undeclaredWrite, argumentAfterWrite].join(" ");
+})()`;
+
+devTest("a hot update runs ES module code in strict mode", {
+  files: {
+    "index.html": emptyHtmlFile({ scripts: ["index.ts"] }),
+    "index.ts": `
+      console.log("first load");
+      import.meta.hot.accept();
+    `,
+    "esm.ts": `export default ${strictModeProbe("undeclaredInEsm")};`,
+    "top-level-await.ts": `export default await Promise.resolve(${strictModeProbe("undeclaredInTopLevelAwait")});`,
+    "no-module-syntax.js": `globalThis.noModuleSyntax = ${strictModeProbe("undeclaredInNoModuleSyntax")};`,
+    // This file has ES module syntax, and the dev server prints it as CommonJS.
+    "hot-and-require.js": `
+      import.meta.hot.accept();
+      require("./empty.cjs");
+      globalThis.hotAndRequire = ${strictModeProbe("undeclaredInHotAndRequire")};
+    `,
+    "empty.cjs": `module.exports = {};`,
+  },
+  async test(dev) {
+    await using c = await dev.client("/");
+    await c.expectMessage("first load");
+
+    // The probes first run from the hot update. The browser runs that chunk as a
+    // classic script, where the page load used a module script.
+    await dev.write(
+      "index.ts",
+      `
+        import esm from "./esm";
+        import topLevelAwait from "./top-level-await";
+        import "./no-module-syntax.js";
+        import "./hot-and-require.js";
+        console.log({
+          savedModule: ${strictModeProbe("undeclaredInSavedModule")},
+          esm,
+          topLevelAwait,
+          noModuleSyntax: globalThis.noModuleSyntax,
+          hotAndRequire: globalThis.hotAndRequire,
+        });
+        import.meta.hot.accept();
+      `,
+    );
+    const strict = "undefined TypeError ReferenceError 1";
+    await c.expectMessage({
+      savedModule: strict,
+      esm: strict,
+      topLevelAwait: strict,
+      noModuleSyntax: strict,
+      hotAndRequire: strict,
+    });
+  },
+});
+
+// `import.meta` makes dep.js an ES module. `exports` makes the dev server print it as
+// CommonJS, so each save runs its strict closure again on the same `exports` object.
+devTest("an ES module that is printed as CommonJS updates in place on each save", {
+  files: {
+    "index.html": emptyHtmlFile({ scripts: ["index.ts"] }),
+    "index.ts": `
+      import * as dep from "./dep.js";
+      console.log("value=" + dep.value);
+      import.meta.hot.accept();
+    `,
+    "dep.js": `
+      import.meta.hot.dispose(() => {});
+      exports.value = 1;
+    `,
+  },
+  async test(dev) {
+    // A reload of the page or a console.error ends this client with an error.
+    await using c = await dev.client("/");
+    await c.expectMessage("value=1");
+    for (const value of [2, 3]) {
+      await dev.write(
+        "dep.js",
+        `
+          import.meta.hot.dispose(() => {});
+          exports.value = ${value};
+        `,
+      );
+      await c.expectMessage("value=" + value);
+    }
+  },
+});
+
+// tsc 6.0.2 output for `export { helper } from "./helper"; export const value = 1;`.
+// The dev server drops the file's own "use strict", so its closure is sloppy. It
+// must stay sloppy: on the second run `helper` is already a getter on `exports`,
+// and the write of `void 0` throws in strict code.
+const tscNamedReexport = (value: number) => `
+  "use strict";
+  Object.defineProperty(exports, "__esModule", { value: true });
+  exports.value = exports.helper = void 0;
+  var helper_1 = require("./helper");
+  Object.defineProperty(exports, "helper", { enumerable: true, get: function () { return helper_1.helper; } });
+  exports.value = ${value};
+`;
+
+devTest("a save of tsc CommonJS output with a named re-export updates in place", {
+  files: {
+    "index.html": emptyHtmlFile({ scripts: ["index.ts"] }),
+    "index.ts": `
+      import * as dep from "./lib/dep.js";
+      console.log("helper=" + dep.helper + " value=" + dep.value);
+      import.meta.hot.accept();
+    `,
+    "lib/dep.js": tscNamedReexport(1),
+    "lib/helper.js": `
+      "use strict";
+      Object.defineProperty(exports, "__esModule", { value: true });
+      exports.helper = void 0;
+      exports.helper = "h1";
+    `,
+  },
+  async test(dev) {
+    // A reload of the page or a console.error ends this client with an error.
+    await using c = await dev.client("/");
+    await c.expectMessage("helper=h1 value=1");
+    await dev.write("lib/dep.js", tscNamedReexport(2));
+    await c.expectMessage("helper=h1 value=2");
+  },
+});
+
+// An empty file is a lazy export. The printer looks for that statement first in
+// the closure, so an empty ES module gets no "use strict" before it.
+devTest("an empty ES module can get code and lose it again", {
+  files: {
+    "index.html": emptyHtmlFile({ scripts: ["index.ts"] }),
+    "index.ts": `
+      import * as dep from "./dep.mjs";
+      console.log("value=" + dep.value + ", has the key: " + ("value" in dep));
+      import.meta.hot.accept();
+    `,
+    "dep.mjs": ``,
+  },
+  async test(dev) {
+    await using c = await dev.client("/");
+    await c.expectMessage("value=undefined, has the key: false");
+    await dev.write("dep.mjs", `export const value = 1;`);
+    await c.expectMessage("value=1, has the key: true");
+    // One byte of white space, not zero bytes: the watcher on macOS does not see a
+    // save that only truncates the file.
+    await dev.write("dep.mjs", "\n", { dedent: false });
+    await c.expectMessage("value=undefined, has the key: false");
+  },
+});

test/bundler/bundler_loader.test.ts

@@ -750,4 +750,93 @@ describe("bundler", async () => {
       },
     });
   });
+
+  // In the dev server's module format each file is a closure. A browser runs a
+  // hot update as a classic script, so the closure of a client file with ES
+  // module semantics must carry its own "use strict".
+  describe("internal_bake_dev strict closures", () => {
+    const files = {
+      "/entry.ts": /* js */ `
+        import { x } from "./esm";
+        import "./no-module-syntax.js";
+        import topLevelAwait from "./top-level-await";
+        import "./import-meta-only";
+        import "./hot-and-require.js";
+        import viaModuleExports from "./import-and-module-exports.js";
+        import a from "./directive.cjs";
+        import b from "./no-directive.cjs";
+        import { helper } from "./tsc-output.cjs";
+        import data from "./data.json";
+        console.log(x, topLevelAwait, viaModuleExports, a, b, helper, data);
+      `,
+      "/esm.ts": `export const x = 1;`,
+      "/no-module-syntax.js": `console.log("no module syntax");`,
+      "/top-level-await.ts": `export default await Promise.resolve(1);`,
+      "/import-meta-only.ts": `if (import.meta.hot) import.meta.hot.accept();`,
+      "/hot-and-require.js": `import.meta.hot.accept(); console.log(require("./no-directive.cjs"));`,
+      "/import-and-module-exports.js": `import { x } from "./esm"; module.exports = x;`,
+      "/directive.cjs": `"use strict"; module.exports = 1;`,
+      "/no-directive.cjs": `module.exports = 2;`,
+      "/tsc-output.cjs": `"use strict";
+Object.defineProperty(exports, "__esModule", { value: true });
+exports.helper = void 0;
+var helper_1 = require("./no-directive.cjs");
+Object.defineProperty(exports, "helper", { enumerable: true, get: function () { return helper_1; } });
+`,
+      "/data.json": `{"a": 1}`,
+    };
+
+    /** Which printed form each module has, and the first statement of its closure. */
+    function closures(output: string) {
+      const result: Record<string, [form: "ES module" | "CommonJS", firstStatement: string]> = {};
+      for (const match of output.matchAll(/^  "([^"\n]+)"(\(|: \[)/gm)) {
+        const open = output.indexOf("{\n", match.index);
+        const firstStatement = output.slice(open + 2, output.indexOf("\n", open + 2)).trim();
+        result[match[1]] = [match[2] === "(" ? "CommonJS" : "ES module", firstStatement];
+      }
+      return result;
+    }
+
+    itBundled("bake-dev/strict-closures-browser", {
+      format: "internal_bake_dev",
+      target: "browser",
+      files,
+      onAfterBundle(api) {
+        const directive = `"use strict";`;
+        expect(closures(api.readFile("/out.js"))).toEqual({
+          "entry.ts": ["ES module", directive],
+          "esm.ts": ["ES module", directive],
+          "no-module-syntax.js": ["ES module", directive],
+          "top-level-await.ts": ["ES module", directive],
+          "import-meta-only.ts": ["ES module", directive],
+          // ES module syntax in a file that is printed as CommonJS.
+          "hot-and-require.js": ["CommonJS", directive],
+          "import-and-module-exports.js": ["CommonJS", directive],
+          // A CommonJS file stays as it is, with or without its own directive.
+          "directive.cjs": ["CommonJS", "module.exports = 1;"],
+          "no-directive.cjs": ["CommonJS", "module.exports = 2;"],
+          "tsc-output.cjs": ["CommonJS", `Object.defineProperty(exports, "__esModule", { value: true });`],
+          "data.json": ["CommonJS", "module.exports = { a: 1 };"],
+        });
+      },
+    });
+
+    // A server chunk that fails to parse ends the dev server process.
+    for (const target of ["bun", "node"] as const) {
+      itBundled(`bake-dev/strict-closures-${target}`, {
+        format: "internal_bake_dev",
+        target,
+        files,
+        onAfterBundle(api) {
+          const output = api.readFile("/out.js");
+          expect(Object.keys(closures(output)).sort()).toEqual(
+            Object.keys(files)
+              .map(file => file.slice(1))
+              .sort(),
+          );
+          expect(output).not.toContain(`"use strict"`);
+        },
+      });
+    }
+  });
 });