Skip to content
Open
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
3 changes: 2 additions & 1 deletion src/js_parser/p.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2179,7 +2179,8 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
) -> Expr {
let ref_ = ident.ref_;

if self.options.features.inlining {
// A property of the `with` object can shadow the const.
if self.options.features.inlining && !ident.must_keep_due_to_with_stmt() {
if let Some(replacement) = self.const_values.get(&ref_) {
let replacement = *replacement;
self.ignore_usage(ref_);
Expand Down
5 changes: 4 additions & 1 deletion src/js_parser/visit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1807,7 +1807,10 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
// encounter the constant because we haven't encountered the eval() yet.
// Inlined constants are not removed if they are in a top-level scope or
// if they are exported (which could be in a nested TypeScript namespace).
if p.const_values.count() > 0 {
if p.const_values.count() > 0
// A later case of the switch shares this scope, and its reads are not counted yet.
&& kind != StmtsKind::SwitchStmt
{
Comment thread
coderabbitai[bot] marked this conversation as resolved.
let items: &mut [Stmt] = stmts.as_mut_slice();
for stmt in items.iter_mut() {
match stmt.data {
Expand Down
5 changes: 5 additions & 0 deletions src/js_parser/visit/visit_expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,11 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
// Handle assigning to a constant
if in_.assign_target != js_ast::AssignTarget::None {
if p.symbols[result.r#ref.inner_index() as usize].kind == js_ast::symbol::Kind::Constant
// In a `with` body the target can be a property of the `with` object.
&& (!result.is_inside_with_scope
// `select_local_kind` can print the `const` as `let` or `var` in these cases.
|| p.options.bundle
|| p.will_wrap_module_in_try_catch_for_using)
{
// TODO: silence this for runtime transpiler
let r = js_lexer::range_of_identifier(p.source, expr.loc);
Expand Down
3 changes: 2 additions & 1 deletion src/jsc/RuntimeTranspilerCache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ bun_core::declare_scope!(cache, visible);
/// Version 28: the define table and `--drop` entries participate in the features hash.
/// Version 29: `new Array(x, ...spread)` is no longer folded into an array literal.
/// Version 30: String enum members are stored flat, so folds no longer append onto an inlined member.
const EXPECTED_VERSION: u32 = 30;
/// Version 31: A `const` read inside a `with` body is no longer replaced by its value.
const EXPECTED_VERSION: u32 = 31;

/// 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
40 changes: 40 additions & 0 deletions test/bundler/bundler_minify.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,46 @@ describe("bundler", () => {
},
run: { stdout: "top fn" },
});
// The `with` object can have a property with the name of a `const` from an
// enclosing scope. That property shadows the const, so a read inside the
// `with` body is not inlined. The `.cjs` names keep the code sloppy.
itBundled("minify/ConstReadInsideWithIsNotInlined", {
files: {
"/entry.cjs": /* js */ `
function f(o) { const x = "const"; with (o) { return x; } }
// The cases of a switch share one scope, and the read is in a later case.
function g(k, o) { switch (k) { case 1: const y = "const"; case 2: with (o) { return y; } } }
console.log(f({ x: "with" }), f({}), g(1, { y: "with" }), g(1, {}));
`,
},
minifySyntax: true,
format: "cjs",
outfile: "/out.cjs",
onAfterBundle(api) {
// The bundler prints the declarations as `let`.
api.expectFile("/out.cjs").toContain('x = "const"');
api.expectFile("/out.cjs").toContain("return x");
api.expectFile("/out.cjs").toContain('y = "const"');
api.expectFile("/out.cjs").toContain("return y");
},
run: { stdout: "with const with const" },
});
// The bundler can print `const` as `let` or `var`, which is only safe when
// nothing assigns to the name. So it still rejects the assignment, although
// the target can be a property of the `with` object.
itBundled("minify/ConstAssignInsideWithIsStillABundleError", {
files: {
"/entry.cjs": /* js */ `
function f(o) { const x = 1; with (o) { x = 5; } return o.x; }
console.log(f({ x: 2 }));
`,
},
format: "cjs",
outfile: "/out.cjs",
bundleErrors: {
"/entry.cjs": ['Cannot assign to "x" because it is a constant'],
},
});
itBundled("minify/TemplateStringFolding", {
files: {
"/entry.js": /* js */ `
Expand Down
104 changes: 104 additions & 0 deletions test/bundler/transpiler/runtime-transpiler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,110 @@ describe("with statement", () => {

expect(exitCode).toBe(0);
});

// The `with` object can have a property with the name of a `const` from an
// enclosing scope. Inside the `with` body that property shadows the const.
// Each fixture prints JSON, and each expected value is what Node prints.
// A `.cjs` file runs in sloppy mode, which `with` needs.
async function runSloppy(source: string) {
using dir = tempDir("with-shadows-const", { "index.cjs": source });
await using proc = Bun.spawn({
cmd: [bunExe(), "index.cjs"],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
return { stdout, stderr, exitCode };
}

test.concurrent("a read in the body is not replaced by the value of a shadowed const", async () => {
const { stdout, stderr, exitCode } = await runSloppy(/* js */ `
const top = "const";
function read(o) { const x = "const"; with (o) { return x; } }
function closure(o) { const x = "const"; with (o) { return (() => x)(); } }
function nested(a, b) { const x = "const"; with (a) { with (b) { return x; } } }
function outsideAndInside(o) { const x = "const"; const before = x; with (o) { return [before, x]; } }
function typeOf(o) { const x = "const"; with (o) { return typeof x; } }
function topLevel(o) { with (o) { return top; } }
// A const declared in the body's own block shadows the with object.
function declaredInBody(o) { with (o) { const x = "const"; return x; } }
// The cases of a switch share one scope, and the read is in a later case.
function laterCase(k, o) { switch (k) { case 1: const x = "const"; case 2: with (o) { return x; } } }
console.log(
JSON.stringify({
read: [read({ x: "with" }), read({})],
closure: [closure({ x: "with" }), closure({})],
nested: [nested({ x: "outer" }, {}), nested({ x: "outer" }, { x: "inner" }), nested({}, {})],
outsideAndInside: outsideAndInside({ x: "with" }),
typeOf: [typeOf({ x: 1 }), typeOf({})],
topLevel: [topLevel({ top: "with" }), topLevel({})],
declaredInBody: declaredInBody({ x: "with" }),
laterCase: [laterCase(1, { x: "with" }), laterCase(1, {})],
}),
);
`);

expect(stderr).toBe("");
expect(JSON.parse(stdout)).toEqual({
read: ["with", "const"],
closure: ["with", "const"],
nested: ["outer", "inner", "const"],
outsideAndInside: ["const", "with"],
typeOf: ["number", "string"],
topLevel: ["with", "const"],
declaredInBody: "const",
laterCase: ["with", "const"],
});
expect(exitCode).toBe(0);
});

test.concurrent("an assignment in the body can target the with object instead of a const", async () => {
const { stdout, stderr, exitCode } = await runSloppy(/* js */ `
function assign(o) { const x = 1; with (o) { x = 5; } return [o.x, x]; }
function compound(o) { const x = 1; with (o) { x += 5; } return [o.x, x]; }
function update(o) { const x = 1; with (o) { x++; } return [o.x, x]; }
function destructure(o) { const x = 1; with (o) { [x] = [7]; } return [o.x, x]; }
function forIn(o) { const x = 1; with (o) { for (x in { key: 0 }); } return [o.x, x]; }
function closure(o) { const x = 1; with (o) { (() => { x = 5; })(); } return [o.x, x]; }
function notLiteral(o) { const x = {}; with (o) { x = 5; } return [o.x, typeof x]; }
// Without the property, the assignment reaches the const and throws at run time.
function noProperty(o) { const x = 1; try { with (o) { x = 5; } } catch (e) { return [e.constructor.name, x]; } }
// https://github.com/oven-sh/bun/issues/13992, the fourth snippet
const obj = { obj: 2 };
with (obj) {
obj = 10;
}
console.log(
JSON.stringify({
assign: assign({ x: 2 }),
compound: compound({ x: 2 }),
update: update({ x: 2 }),
destructure: destructure({ x: 2 }),
forIn: forIn({ x: 2 }),
closure: closure({ x: 2 }),
notLiteral: notLiteral({ x: 2 }),
noProperty: noProperty({}),
obj,
}),
);
`);

expect(stderr).toBe("");
expect(JSON.parse(stdout)).toEqual({
assign: [5, 1],
compound: [7, 1],
update: [3, 1],
destructure: [7, 1],
forIn: ["key", 1],
closure: [5, 1],
notLiteral: [5, "object"],
noProperty: ["TypeError", 1],
obj: { obj: 10 },
});
expect(exitCode).toBe(0);
});
});

test("math.pow", () => {
Expand Down
86 changes: 86 additions & 0 deletions test/bundler/transpiler/transpiler.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4001,6 +4001,92 @@ console.log(foo, array);
);
});

// The `with` object can have a property with the name of the const. That
// property shadows the const, so the read must stay a read.
it("const inlining keeps a read inside a with statement", () => {
var transpiler = new Bun.Transpiler({
inline: true,
platform: "bun",
allowBunRuntime: false,
minify: { syntax: true },
});

function check(input, output) {
expect(transpiler.transformSync(input)).toBe(output);
}
hideFromStackTrace(check);

check(
"function f(o) { const x = 1; with (o) { return x; } }",
"function f(o) {\n const x = 1;\n with (o)\n return x;\n}\n",
);
check(
"function f(o) { const x = 1; with (o) { return typeof x; } }",
"function f(o) {\n const x = 1;\n with (o)\n return typeof x;\n}\n",
);
// A function created in the body captures the `with` object.
check(
"function f(o) { const x = 1; with (o) { return () => x; } }",
"function f(o) {\n const x = 1;\n with (o)\n return () => x;\n}\n",
);
check(
"function f(a, b) { const x = 1; with (a) { with (b) { return x; } } }",
"function f(a, b) {\n const x = 1;\n with (a)\n with (b)\n return x;\n}\n",
);
check("const x = 1; with (o) { console.log(x); }", "const x = 1;\nwith (o)\n console.log(x);\n");

// A read outside the body is still inlined. The read inside keeps the declaration.
check(
"function f(o) { const x = 1; g(x); with (o) { return x; } }",
"function f(o) {\n const x = 1;\n g(1);\n with (o)\n return x;\n}\n",
);
// The cases of a switch share one scope. A read in a later case keeps the declaration.
check(
"function f(k, o) { switch (k) { case 1: const x = 1; case 2: with (o) { return x; } } }",
"function f(k, o) {\n switch (k) {\n case 1:\n const x = 1;\n case 2:\n with (o)\n return x;\n }\n}\n",
);
// The `with` object expression is evaluated outside the body.
check("function f() { const x = 1; with (x) { return y; } }", "function f() {\n with (1)\n return y;\n}\n");
// A const declared in the body's own block shadows the `with` object.
check("function f(o) { with (o) { const x = 1; return x; } }", "function f(o) {\n with (o)\n return 1;\n}\n");
});

// The target can be a property of the `with` object, so the assignment is
// not a const assignment error and the name is not replaced by the value.
it("allows an assignment to the name of a const inside a with statement", () => {
var transpiler = new Bun.Transpiler({
inline: true,
platform: "bun",
allowBunRuntime: false,
minify: { syntax: true },
});

expect(transpiler.transformSync("function f(o) { const x = 1; with (o) { x = 5; } }")).toBe(
"function f(o) {\n const x = 1;\n with (o)\n x = 5;\n}\n",
);
expect(transpiler.transformSync("function f(o) { const x = {}; with (o) { x++; } }")).toBe(
"function f(o) {\n const x = {};\n with (o)\n x++;\n}\n",
);

// Outside the body the assignment is still an error.
expect(() => transpiler.transformSync("function f(o) { const x = 1; with (o) {} x = 5; }")).toThrow(
'Cannot assign to "x" because it is a constant',
);
expect(() => transpiler.transformSync("function f(o) { const x = {}; with (o) {} x++; }")).toThrow(
'This assignment will throw because "x" is a constant',
);

// A lowered top-level `using` prints each top-level `const` as `var`. Nothing
// would stop the assignment then, so the error stays.
const source = "using r = null; const x = 1; function f(o) { with (o) { x = 5; } }";
expect(() => new Bun.Transpiler({ target: "browser" }).transformSync(source)).toThrow(
'This assignment will throw because "x" is a constant',
);
expect(new Bun.Transpiler({ target: "bun" }).transformSync(source)).toBe(
"let r = null;\nconst x = 1;\nfunction f(o) {\n with (o)\n x = 5;\n}\n",
);
});

it("constant folding scopes", () => {
var transpiler = new Bun.Transpiler({
inline: true,
Expand Down
Loading