Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 29 additions & 12 deletions src/js_parser/p.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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.
//
Expand Down Expand Up @@ -8122,25 +8140,14 @@ 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()`.
let stmts_to_copy = arena.alloc_slice_fill_with(total_stmts_count, |_| Stmt::empty());
{
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..];
}

Expand Down Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion src/js_parser/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
];

Expand Down
14 changes: 3 additions & 11 deletions src/js_parser/repl_transforms.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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() {
Expand Down
3 changes: 2 additions & 1 deletion src/jsc/RuntimeTranspilerCache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
59 changes: 58 additions & 1 deletion test/bundler/transpiler/preserve-use-strict-cjs.test.ts
Original file line number Diff line number Diff line change
@@ -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 () => {
Expand All @@ -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"],
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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);
});
15 changes: 15 additions & 0 deletions test/bundler/transpiler/strict-mode-after-directive-fixture.cjs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

121 changes: 121 additions & 0 deletions test/cli/run/run-eval.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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);
});
});
36 changes: 36 additions & 0 deletions test/cli/run/transpiler-cache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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"));
Expand Down
Loading
Loading