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
7 changes: 6 additions & 1 deletion src/js_parser/ast/P.zig
Original file line number Diff line number Diff line change
Expand Up @@ -6506,10 +6506,15 @@ pub fn NewParser_(
total_stmts_count += part.stmts.len;
}

// Only skip re-emitting "use strict" if the first output
// statement IS already "use strict". Other directives (e.g.
// "use client") do not satisfy the strict-mode contract and
// would otherwise let the wrapper IIFE run sloppy.
const preserve_strict_mode = p.module_scope.strict_mode == .explicit_strict_mode and
!(parts.items.len > 0 and
parts.items[0].stmts.len > 0 and
parts.items[0].stmts[0].data == .s_directive);
parts.items[0].stmts[0].data == .s_directive and
strings.eqlComptime(parts.items[0].stmts[0].data.s_directive.value, "use strict"));

total_stmts_count += @as(usize, @intCast(@intFromBool(preserve_strict_mode)));

Expand Down
25 changes: 19 additions & 6 deletions src/js_parser/ast/parse.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1184,23 +1184,36 @@ pub fn Parse(
// Parse one or more directives at the beginning
if (isDirectivePrologue) {
isDirectivePrologue = false;
// ES2015 §14.1.1: the directive prologue is the leading
// sequence of ExpressionStatements consisting of a single
// StringLiteral — ONLY at the top of a FunctionBody or
// the top-level Script/Module. Block-scope string
// literals are ordinary expressions (dead code in most
// cases) and must not be promoted to S.Directive.
const scope_kind = p.current_scope.kind;
const is_directive_scope = scope_kind == .entry or scope_kind == .function_body;
switch (stmt.data) {
.s_expr => |expr| {
switch (expr.value.data) {
.e_string => |str| {
if (!str.prefer_template) {
if (!str.prefer_template and is_directive_scope) {
isDirectivePrologue = true;

if (str.eqlComptime("use strict")) {
skip = true;
// Track "use strict" directives
const is_strict = str.eqlComptime("use strict");
if (is_strict) {
p.current_scope.strict_mode = .explicit_strict_mode;
if (p.current_scope == p.module_scope)
p.module_scope_directive_loc = stmt.loc;
}
if (is_strict and p.current_scope == p.module_scope) {
// Module-level "use strict" is dropped; the CJS
// wrapper re-emits it so the IIFE runs in strict mode.
skip = true;
p.module_scope_directive_loc = stmt.loc;
} else if (str.eqlComptime("use asm")) {
skip = true;
stmt.data = Prefill.Data.SEmpty;
} else {
// Preserve other directives (including function-level
// "use strict" — no later pass restores it).
stmt = Stmt.alloc(S.Directive, S.Directive{
.value = str.slice(p.allocator),
}, stmt.loc);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Expand Down
3 changes: 0 additions & 3 deletions src/js_parser/ast/visit.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1201,9 +1201,6 @@ pub fn Visit(
switch (stmt.data) {
.s_empty => continue,

Comment thread
robobun marked this conversation as resolved.
// skip directives for now
.s_directive => continue,

.s_local => |local| {
Comment thread
robobun marked this conversation as resolved.
// Merge adjacent local statements
if (output.items.len > 0) {
Expand Down
6 changes: 3 additions & 3 deletions test/bundler/bundler_npm.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,17 @@ describe("bundler", () => {
"../entry.tsx",
],
mappings: [
["react.development.js:524:'getContextName'", "1:5567:Y1"],
["react.development.js:524:'getContextName'", "1:5580:Y1"],
["react.development.js:2495:'actScopeDepth'", "23:4082:GJ++"],
["react.development.js:696:''Component'", '1:7629:\'Component "%s"'],
["react.development.js:696:''Component'", '1:7642:\'Component "%s"'],
["entry.tsx:6:'\"Content-Type\"'", '100:18808:"Content-Type"'],
["entry.tsx:11:'<html>'", "100:19062:void"],
["entry.tsx:23:'await'", "100:19161:await"],
],
},
},
expectExactFilesize: {
"out/entry.js": 221895,
"out/entry.js": 221947,
},
run: {
stdout: "<!DOCTYPE html><html><body><h1>Hello World</h1><p>This is an example.</p></body></html>",
Expand Down
23 changes: 23 additions & 0 deletions test/regression/issue/29533-fn.fixture.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
exports.isES5 = (function () {
"use strict";
return this === undefined;
})();

exports.typeofThis = (function () {
"use strict";
return typeof this;
}).call("hello");

exports.mode = (function () {
"use strict";
// Unique sentinel so an earlier test can't poison the result by
// leaking a reachable global named `undeclared`.
delete globalThis.__issue29533_sentinel__;
try {
__issue29533_sentinel__ = 1;
delete globalThis.__issue29533_sentinel__;
return "sloppy";
} catch (e) {
return "strict";
}
})();
Comment thread
coderabbitai[bot] marked this conversation as resolved.
4 changes: 4 additions & 0 deletions test/regression/issue/29533-module.fixture.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
"use strict";
exports.typeofThis = (function () {
return typeof this;
}).call("hello");
132 changes: 132 additions & 0 deletions test/regression/issue/29533.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
// https://github.com/oven-sh/bun/issues/29533

import { expect, test } from "bun:test";
import { bunEnv, bunExe, tempDir } from "harness";

test("function-body 'use strict' is preserved in CJS", () => {
const m = require("./29533-fn.fixture.cjs");
expect(m.isES5).toBe(true);
expect(m.typeofThis).toBe("string");
expect(m.mode).toBe("strict");
});

test("module-level 'use strict' still enforces strict mode in CJS", () => {
// The CJS wrapper re-emits module-level "use strict".
const m = require("./29533-module.fixture.cjs");
expect(m.typeofThis).toBe("string");
});
Comment thread
claude[bot] marked this conversation as resolved.

// Subprocesses inherit the parent's bun — under `bun bd` (ASAN debug) that
// binary prints a JSC warning to stderr unconditionally. Assert stderr has
// no "error:" rather than toBe("") so the warning doesn't fail the test.
// (panics/ASSERTION FAILED already produce a non-zero exit code, which the
// expect(exitCode).toBe(0) at each test callsite catches.)
function expectNoStderrErrors(stderr: string) {
expect(stderr).not.toMatch(/^error:/im);
}
Comment thread
robobun marked this conversation as resolved.

test.concurrent("function-body 'use strict' preserved in .js with package type=commonjs", async () => {
// Same body shape as the fixture, but a .js file in a directory whose
// package.json declares "commonjs" — exercises the CJS classifier path for
// the extension the original reproduction hit (bluebird's index.js).
using dir = tempDir("issue-29533-js", {
"package.json": JSON.stringify({ type: "commonjs" }),
"entry.js": `
var isES5 = (function () {
"use strict";
return this === undefined;
})();
var mode = (function () {
"use strict";
try {
__issue29533_spawn_sentinel__ = 1;
return "sloppy";
} catch (e) {
return "strict";
}
})();
console.log(isES5 + " " + mode);
`,
});

await using proc = Bun.spawn({
cmd: [bunExe(), "run", "entry.js"],
cwd: String(dir),
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});

const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

expectNoStderrErrors(stderr);
expect(stdout.trim()).toBe("true strict");
expect(exitCode).toBe(0);
Comment thread
robobun marked this conversation as resolved.
});

test.concurrent("non-strict module-level directive doesn't suppress CJS 'use strict' re-emission", async () => {
// Regression guard: the CJS wrapper in P.zig skips re-emitting "use strict"
// when the module already starts with one. That check must look at the
// directive VALUE, not just the tag — otherwise a module that starts with
// e.g. "use client" (or any other custom directive) followed by "use
// strict" would run sloppy.
using dir = tempDir("issue-29533-use-client", {
"entry.cjs": `
"use client";
"use strict";
var isStrict = (function () {
return this === undefined;
}).call(undefined);
module.exports.isStrict = isStrict;
console.log(isStrict);
`,
});

await using proc = Bun.spawn({
cmd: [bunExe(), "run", "entry.cjs"],
cwd: String(dir),
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});

const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

expectNoStderrErrors(stderr);
expect(stdout.trim()).toBe("true");
expect(exitCode).toBe(0);
});

test.concurrent("block-scope string literals are not treated as directives", async () => {
// ES2015 §14.1.1 restricts the directive prologue to the leading
// ExpressionStatements of a FunctionBody or Script/Module. Block-scope
// string expressions (dead code) must remain ordinary expressions so
// DCE can still drop them under minify_syntax. Bundle with --minify and
// confirm the block-scope string is gone.
using dir = tempDir("issue-29533-block-dir", {
"entry.js": `
function foo() {
if (true) {
"__block_scope_should_be_dropped__";
console.log("hello");
}
}
foo();
`,
});

await using proc = Bun.spawn({
cmd: [bunExe(), "build", "--minify", "--target=node", "entry.js"],
cwd: String(dir),
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});

const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

expectNoStderrErrors(stderr);
expect(stdout).not.toContain("__block_scope_should_be_dropped__");
expect(stdout).toContain('"hello"');
expect(exitCode).toBe(0);
});
Loading