diff --git a/src/js_parser/p.rs b/src/js_parser/p.rs index 12d485079d93..1e48d4cf0095 100644 --- a/src/js_parser/p.rs +++ b/src/js_parser/p.rs @@ -763,6 +763,20 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O Stmt::alloc(t, loc) } + /// `parse_stmts_up_to` drops a module-level `"use strict"`, so script-like output re-emits it. + pub(crate) fn use_strict_directive(&self) -> Stmt { + debug_assert_eq!( + self.module_scope().strict_mode, + js_ast::StrictModeKind::ExplicitStrictMode + ); + self.s( + S::Directive { + value: b"use strict".into(), + }, + self.module_scope_directive_loc, + ) + } + pub(crate) fn load_name_from_ref(&self, r#ref: Ref) -> &'a [u8] { use js_ast::base::RefTag; match r#ref.tag() { @@ -8049,6 +8063,10 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O } } + // Goes in front of any other directive the file kept. The prologue order does not matter. + let preserve_strict_mode = wrap_mode == WrapMode::BunCommonjs + && self.module_scope().strict_mode == js_ast::StrictModeKind::ExplicitStrictMode; + if wrap_mode == WrapMode::BunCommonjs && !self.options.features.remove_cjs_module_wrapper { // This transforms the user's code into. // @@ -8122,12 +8140,6 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O total_stmts_count += part.stmts.len(); } - let preserve_strict_mode = self.module_scope().strict_mode - == js_ast::StrictModeKind::ExplicitStrictMode - && !(parts.len() > 0 - && parts[0].stmts.len() > 0 - && matches!(parts[0].stmts[0].data, js_ast::StmtData::SDirective(_))); - total_stmts_count += usize::from(preserve_strict_mode); // Stmt is not Default; fill with `Stmt::empty()`. @@ -8135,12 +8147,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O { let mut remaining_stmts = &mut stmts_to_copy[..]; if preserve_strict_mode { - remaining_stmts[0] = self.s( - S::Directive { - value: b"use strict".into(), - }, - self.module_scope_directive_loc, - ); + remaining_stmts[0] = self.use_strict_directive(); remaining_stmts = &mut remaining_stmts[1..]; } @@ -8183,6 +8190,16 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O } parts.truncate(1); parts[0].stmts = bun_ast::StoreSlice::new_mut(top_level_stmts); + } else if preserve_strict_mode { + // No wrapper (the eval entry point): the output runs as a classic script, so lead it. + let stmts = arena.alloc_slice_copy(&[self.use_strict_directive()]); + parts.insert( + 0, + js_ast::Part { + stmts: bun_ast::StoreSlice::new_mut(stmts), + ..Default::default() + }, + ); } // REPL mode transforms diff --git a/src/js_parser/parser.rs b/src/js_parser/parser.rs index 408deeeb45c0..2c8f88df87c9 100644 --- a/src/js_parser/parser.rs +++ b/src/js_parser/parser.rs @@ -355,7 +355,7 @@ pub mod Runtime { pub(crate) fn hash_for_runtime_transpiler(&self, hasher: &mut Wyhash) { debug_assert!(self.runtime_transpiler_cache.is_some()); - let bools: [bool; 17] = [ + let bools: [bool; 18] = [ self.top_level_await, self.auto_import_jsx, self.allow_runtime, @@ -373,6 +373,8 @@ pub mod Runtime { self.standard_decorators, self.lower_using, self.repl_mode, + // The eval entry point is printed without the CommonJS wrapper. + self.remove_cjs_module_wrapper, // note that we do not include .inject_jest_globals, as we bail out of the cache entirely if this is true ]; diff --git a/src/js_parser/repl_transforms.rs b/src/js_parser/repl_transforms.rs index 06fcfb00fd57..af8891978094 100644 --- a/src/js_parser/repl_transforms.rs +++ b/src/js_parser/repl_transforms.rs @@ -40,11 +40,8 @@ impl<'a, const TS: bool, const SCAN: bool> P<'a, TS, SCAN> { // A prologue "use strict" was consumed into module-scope strict mode and dropped, but // node's repl still evaluates the directive as a string expression. Reinject it at the // front, like the CJS wrapper's preserve_strict_mode. - let reinject_strict = self.module_scope().strict_mode - == js_ast::StrictModeKind::ExplicitStrictMode - && !(!parts.is_empty() - && !parts[0].stmts.is_empty() - && matches!(parts[0].stmts[0].data, StmtData::SDirective(_))); + let reinject_strict = + self.module_scope().strict_mode == js_ast::StrictModeKind::ExplicitStrictMode; total_stmts_count += usize::from(reinject_strict); if total_stmts_count == 0 { @@ -54,12 +51,7 @@ impl<'a, const TS: bool, const SCAN: bool> P<'a, TS, SCAN> { // Collect all statements into a single array let mut all_stmts = BumpVec::with_capacity_in(total_stmts_count, bump); if reinject_strict { - all_stmts.push(self.s( - S::Directive { - value: b"use strict".into(), - }, - self.module_scope_directive_loc, - )); + all_stmts.push(self.use_strict_directive()); } for part in parts.iter() { for stmt in part.stmts.iter() { diff --git a/src/jsc/RuntimeTranspilerCache.rs b/src/jsc/RuntimeTranspilerCache.rs index 597bcd6952aa..e4135f84054f 100644 --- a/src/jsc/RuntimeTranspilerCache.rs +++ b/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: "use strict" is kept for the eval entry point and in front of other directives. +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 diff --git a/test/bundler/transpiler/preserve-use-strict-cjs.test.ts b/test/bundler/transpiler/preserve-use-strict-cjs.test.ts index a948aab3b2b3..875a0710e444 100644 --- a/test/bundler/transpiler/preserve-use-strict-cjs.test.ts +++ b/test/bundler/transpiler/preserve-use-strict-cjs.test.ts @@ -1,5 +1,6 @@ import { expect, test } from "bun:test"; -import { bunRun } from "harness"; +import { readFileSync, rmSync } from "fs"; +import { bunEnv, bunExe, bunRun, isWindows, tempDir } from "harness"; import path from "path"; test.concurrent(`"use strict'; preserves strict mode in CJS`, async () => { @@ -9,3 +10,59 @@ test.concurrent(`"use strict'; preserves strict mode in CJS`, async () => { test.concurrent(`sloppy mode by default in CJS`, async () => { expect(await bunRun(path.join(import.meta.dir, "sloppy-mode-fixture.ts"))).toSpawn(); }); + +test.concurrent(`"use strict"; after another directive preserves strict mode in CJS`, async () => { + expect(await bunRun(path.join(import.meta.dir, "strict-mode-after-directive-fixture.cjs"))).toSpawn("strict"); +}); + +test.concurrent(`"use strict"; after another directive preserves strict mode with the inspector enabled`, async () => { + // With the inspector enabled the runtime transpiler does not minify syntax, + // so the "use client" directive stays in the output as the first statement. + // The transpiler used to skip "use strict" in that case, and the module ran + // in sloppy mode only while it was being debugged. + const socket = `/tmp/bun-use-strict-inspect-${process.pid}-${Date.now()}.sock`; + try { + await using proc = Bun.spawn({ + cmd: [bunExe(), path.join(import.meta.dir, "strict-mode-after-directive-fixture.cjs")], + env: { ...bunEnv, BUN_INSPECT: isWindows ? "127.0.0.1:0" : "ws+unix://" + socket }, + stdout: "pipe", + // The inspector prints its listening address here. + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).not.toContain("this is not undefined"); + expect(stdout).toBe("strict\n"); + expect(exitCode).toBe(0); + } finally { + rmSync(socket, { force: true }); + } +}); + +test.concurrent(`"use strict"; after another directive preserves strict mode under bun test --coverage`, async () => { + // --coverage turns syntax minification off for the whole process, the same + // way the inspector does, so a test suite used to see such modules in + // sloppy mode only when coverage was enabled. + using dir = tempDir("use-strict-coverage", { + "lib.cjs": readFileSync(path.join(import.meta.dir, "strict-mode-after-directive-fixture.cjs"), "utf8"), + "lib.test.ts": ` + import { expect, test } from "bun:test"; + test("lib is strict", () => { + expect(require("./lib.cjs")).toEqual({ FORCE_COMMON_JS: true }); + }); + `, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "test", "--coverage", "./lib.test.ts"], + cwd: String(dir), + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + // The fixture throws before it exports anything when it runs in sloppy mode. + expect(stderr).not.toContain("this is not undefined"); + expect(stderr).toContain("1 pass"); + // The test runner prints its banner to stdout too. + expect(stdout).toEndWith("strict\n"); + expect(exitCode).toBe(0); +}); diff --git a/test/bundler/transpiler/strict-mode-after-directive-fixture.cjs b/test/bundler/transpiler/strict-mode-after-directive-fixture.cjs new file mode 100644 index 000000000000..9ab6a8ffbbb0 --- /dev/null +++ b/test/bundler/transpiler/strict-mode-after-directive-fixture.cjs @@ -0,0 +1,15 @@ +"use client"; +"use strict"; + +function checkThis() { + if (this !== undefined) { + throw new Error("this is not undefined"); + } +} + +checkThis(); +console.log("strict"); + +module.exports = { + FORCE_COMMON_JS: true, +}; diff --git a/test/cli/run/run-eval.test.ts b/test/cli/run/run-eval.test.ts index 0c7462e3b5ec..fa4443b0570d 100644 --- a/test/cli/run/run-eval.test.ts +++ b/test/cli/run/run-eval.test.ts @@ -5,6 +5,22 @@ import { bunEnv, bunExe, isWindows, tempDir, tmpdirSync } from "harness"; import { tmpdir } from "os"; import { join, sep } from "path"; +// Prints whether the code around it runs in strict mode. A plain function call +// sees `this === undefined` only in strict mode, and assigning to an undeclared +// name throws only in strict mode. +const strictModeProbe = ` +var thisIsUndefined = (function () { return this === undefined; })(); +var undeclaredThrows = false; +try { + someUndeclaredNameForTheStrictModeProbe = 1; +} catch (e) { + undeclaredThrows = e instanceof ReferenceError; +} +console.log(JSON.stringify({ thisIsUndefined, undeclaredThrows })); +`; +const strictResult = JSON.stringify({ thisIsUndefined: true, undeclaredThrows: true }) + "\n"; +const sloppyResult = JSON.stringify({ thisIsUndefined: false, undeclaredThrows: false }) + "\n"; + for (const flag of ["-e", "--print"]) { describe(`bun ${flag}`, () => { test("it works", async () => { @@ -247,6 +263,11 @@ function group(run: (code: string) => SyncSubprocess<"pipe", "inherit">) { expect(stdout.toString("utf8")).toEqual(code + "\n"); } }); + + test('a leading "use strict" makes the script strict', async () => { + const { stdout } = run(`"use strict";` + strictModeProbe); + expect(stdout.toString("utf8")).toEqual(strictResult); + }); } describe("bun run - < file-path.js", () => { @@ -348,3 +369,103 @@ test("uncaught error from a CommonJS-sniffed stdin entry reports and exits 1", a expect(stdout).toBe(""); expect(exitCode).toBe(1); }); + +// The eval entry point (-e, -p, stdin) is transpiled as CommonJS without the +// module wrapper and then evaluated as a classic script. The transpiler consumes +// a module-level "use strict" while parsing, so it has to emit the directive +// again, or the script runs in sloppy mode. Node honors the directive. +describe.concurrent('"use strict" in the eval entry point', () => { + async function runBun(args: string[], cwd?: string) { + await using proc = Bun.spawn({ + cmd: [bunExe(), ...args], + env: bunEnv, + cwd, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout, stderr, exitCode }; + } + + test("bun -e honors a leading directive", async () => { + const { stdout, stderr, exitCode } = await runBun(["-e", `"use strict";` + strictModeProbe]); + expect(stderr).toBe(""); + expect(stdout).toBe(strictResult); + expect(exitCode).toBe(0); + }); + + test("bun -e stays sloppy without the directive", async () => { + // `module.exports` forces CommonJS. Without it the eval source is an ES + // module, which is always strict. + const { stdout, stderr, exitCode } = await runBun(["-e", `module.exports;` + strictModeProbe]); + expect(stderr).toBe(""); + expect(stdout).toBe(sloppyResult); + expect(exitCode).toBe(0); + }); + + test("bun -e is strict when the directive follows a comment", async () => { + const { stdout, stderr, exitCode } = await runBun(["-e", `// leading comment\n'use strict';` + strictModeProbe]); + expect(stderr).toBe(""); + expect(stdout).toBe(strictResult); + expect(exitCode).toBe(0); + }); + + test("bun -e is strict when another directive comes first", async () => { + const { stdout, stderr, exitCode } = await runBun(["-e", `"use client"; "use strict";` + strictModeProbe]); + expect(stderr).toBe(""); + expect(stdout).toBe(strictResult); + expect(exitCode).toBe(0); + }); + + test("bun -e as an ES module is strict", async () => { + // An import statement makes the eval source an ES module, which is strict + // with or without the directive. + const code = `"use strict"; import { ok } from "node:assert"; ok(true);` + strictModeProbe; + const { stdout, stderr, exitCode } = await runBun(["-e", code]); + expect(stderr).toBe(""); + expect(stdout).toBe(strictResult); + expect(exitCode).toBe(0); + }); + + test("bun -p honors a leading directive", async () => { + const { stdout, stderr, exitCode } = await runBun([ + "-p", + `"use strict"; (function () { return this === undefined; })()`, + ]); + expect(stderr).toBe(""); + expect(stdout).toBe("true\n"); + expect(exitCode).toBe(0); + }); + + test("bun -p is strict when another directive comes first", async () => { + // -p keeps the other directive as a statement (it disables dead code + // elimination), so "use strict" has to be emitted in front of it. + const { stdout, stderr, exitCode } = await runBun([ + "-p", + `"use client"; "use strict"; (function () { return this === undefined; })()`, + ]); + expect(stderr).toBe(""); + expect(stdout).toBe("true\n"); + expect(exitCode).toBe(0); + }); + + test("bun -p prints a lone directive like node does", async () => { + const { stdout, stderr, exitCode } = await runBun(["-p", `"use strict"`]); + expect(stderr).toBe(""); + expect(stdout).toBe("use strict\n"); + expect(exitCode).toBe(0); + }); + + test("a required CommonJS file is strict when another directive comes first", async () => { + // Same directive handling, but through the CommonJS module wrapper. The + // -p entry point disables dead code elimination for every module in the + // process, so "use client" survives as the first statement of the file. + using dir = tempDir("eval-use-strict", { + "strict-after-directive.cjs": `"use client";\n"use strict";\nmodule.exports = (function () { return this === undefined; })();\n`, + }); + const { stdout, stderr, exitCode } = await runBun(["-p", `require("./strict-after-directive.cjs")`], String(dir)); + expect(stderr).toBe(""); + expect(stdout).toBe("true\n"); + expect(exitCode).toBe(0); + }); +}); diff --git a/test/cli/run/transpiler-cache.test.ts b/test/cli/run/transpiler-cache.test.ts index c3e1c273234e..e0ce8e2e1b4c 100644 --- a/test/cli/run/transpiler-cache.test.ts +++ b/test/cli/run/transpiler-cache.test.ts @@ -124,6 +124,42 @@ describe("transpiler cache", () => { expect(await bunRun(join(temp_dir, "b.js"), env)).toSpawn("b"); expect(newCacheCount()).toBe(0); }); + test("the stdin entry point does not share entries with a file of the same contents", async () => { + // A CommonJS file is cached wrapped in the module function. The stdin (and + // -e) entry point is printed without that wrapper and evaluated as a plain + // script, so an entry written for the file must not be served to stdin and + // the other way around. Stdin is parsed with the tsx loader, so a .tsx file + // is the one that hashes to the same features. + const source = dummyFile(50 * 1024, "stdin", { code: `"cjs", typeof module` }); + writeFileSync(join(temp_dir, "a.tsx"), source); + + async function runStdin() { + await using proc = Bun.spawn({ + cmd: [bunExe(), "-"], + cwd: temp_dir, + env, + stdin: source, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(exitCode).toBe(0); + return stdout.trim(); + } + + expect(await bunRun(join(temp_dir, "a.tsx"), env)).toSpawn("cjs object"); + expect(newCacheCount()).toBe(1); + + // Same input hash, different features hash: the file's entry is replaced, + // not reused. If stdin were served the file's entry, it would evaluate the + // wrapper function without calling it and print nothing at all. + expect(await runStdin()).toBe("cjs object"); + expect(newCacheCount()).toBe(0); + expect(await runStdin()).toBe("cjs object"); + + expect(await bunRun(join(temp_dir, "a.tsx"), env)).toSpawn("cjs object"); + }); test("doing 50 buns at once does not crash", async () => { writeFileSync(join(temp_dir, "a.js"), dummyFile(50 * 1024, "1", "b")); writeFileSync(join(temp_dir, "b.js"), dummyFile(50 * 1024, "2", "b")); diff --git a/test/js/bun/repl/repl.test.ts b/test/js/bun/repl/repl.test.ts index 5947a8d30a3f..e51fefe57b04 100644 --- a/test/js/bun/repl/repl.test.ts +++ b/test/js/bun/repl/repl.test.ts @@ -797,6 +797,17 @@ describe.concurrent("Bun REPL", () => { expect(stderr).toBe(""); expect(exitCode).toBe(0); }); + + test('"use strict" still applies when another directive comes first', async () => { + const { stdout, exitCode } = await runRepl([ + `"use client"; "use strict"; (function () { return this === undefined; })()`, + ".exit", + ]); + const output = stripAnsi(stdout); + expect(output).toContain("true"); + expect(output).not.toContain("false"); + expect(exitCode).toBe(0); + }); }); describe("async evaluation", () => {