Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
9 changes: 9 additions & 0 deletions src/ast/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,15 @@ impl Expr {
// https://github.com/oven-sh/bun/issues/2594
Data::ESpread(_) => false,
Data::EMissing(_) => false,
// Inlining `a?.b` as the target of a surrounding member/call
// expression can splice two unrelated optional chains together:
// `[[a?.b]][0]?.[0].c` would become `a?.b.c`, which short-circuits
// past `.c` instead of throwing. The printer can only insert the
// `(a?.b)` wrapper when the parent's own optional_chain is None,
// which we cannot observe here, so bail out conservatively.
Comment thread
robobun marked this conversation as resolved.
Outdated
Data::EDot(e) => e.optional_chain.is_none(),
Data::EIndex(e) => e.optional_chain.is_none(),
Data::ECall(e) => e.optional_chain.is_none(),
_ => true,
}
}
Expand Down
42 changes: 27 additions & 15 deletions src/js_parser/visit/visit_expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1057,7 +1057,14 @@
let target = e_.target.unwrap_inlined();
let index = e_.index.unwrap_inlined();

if p.options.features.minify_syntax {
// These folds replace an index expression (a Reference into a temporary
// string/array) with a plain value. That is only sound when the parent
// uses the value, not the Reference: `delete [x][0]` and `[x][0] = v`
// act on the temporary, and inlining would redirect them at `x`.
Comment thread
robobun marked this conversation as resolved.
Outdated
if p.options.features.minify_syntax
&& !is_delete_target

Check failure on line 1065 in src/js_parser/visit/visit_expr.rs

View check run for this annotation

Claude / Claude Code Review

!is_delete_target guard at :1065 is dead code — delete [obj.p][0] still folds

The `!is_delete_target` guard is dead code — `p.delete_target` is never set to the operand in the `Op::UnDelete` arm (visit_expr.rs:1232-1234), so `is_delete_target` at :888 is always `false` and `delete [obj.p][0]` still folds to `delete obj.p`, contradicting the comment above. Add `p.delete_target = e_.value.data;` before the `visit_expr_in_out` call in `Op::UnDelete` (matching esbuild), and add `ts.expectPrintedMin_("delete [obj.p][0]", "delete [obj.p][0]")` plus a `delete` case in the runtim
Comment thread
robobun marked this conversation as resolved.
Outdated
&& in_.assign_target == js_ast::AssignTarget::None
{
if let Some(number) = index.data.as_e_number() {
if number.value() >= 0.0
&& number.value() < (usize::MAX as f64)
Expand Down Expand Up @@ -1085,29 +1092,34 @@
}
}
} else if let Some(array) = target.data.as_e_array() {
let int: usize = number.value() as usize;
// [x][0] -> x
if array.items.len_u32() == 1 && number.value() == 0.0 {
let inlined = *array.items.at(0);
if inlined.can_be_inlined_from_property_access() {
*e = inlined;
return;
}
}

// ['a', 'b', 'c'][1] -> 'b'
let int: usize = number.value() as usize;
if int < array.items.len_u32() as usize
let inlined = if array.items.len_u32() == 1 && int == 0 {
Some(*array.items.at(0))
} else if int < array.items.len_u32() as usize
&& p.expr_can_be_removed_if_unused(&target)
{
let inlined = *array.items.at(int);
Some(*array.items.at(int))
} else {
None
};
if let Some(inlined) = inlined {
// ['a', , 'c'][1] -> undefined
if matches!(inlined.data, Data::EMissing(..)) {
*e = p.new_expr(E::Undefined {}, inlined.loc);
return;
}
debug_assert!(inlined.can_be_inlined_from_property_access());
*e = inlined;
return;
if inlined.can_be_inlined_from_property_access() {
// "[obj.m][0]()" => "(0, obj.m)()"
*e = if is_call_target && inlined.has_value_for_this_in_call() {
p.new_expr(E::Number::new(0.0), expr.loc)
.join_with_comma(inlined)
} else {
inlined
};
return;
}

Check notice on line 1122 in src/js_parser/visit/visit_expr.rs

View check run for this annotation

Claude / Claude Code Review

Tagged-template tag position still rebinds this ([obj.m][0]`x` → obj.m`x`)

Pre-existing, fourth sibling of the call/delete/assign contexts the second commit guards here: `[obj.m][0]`tpl`` still folds to `obj.m`tpl``, rebinding `this` from the temp array to `obj` (Node prints `false` for `var obj={m(){return this===obj}}; [obj.m][0]`x``; the folded form prints `true`). `is_call_target` derives from `p.call_target` which only `e_call` sets (:1854); `e_template` (:695-696) visits the tag via plain `p.visit_expr()`, and `is_template_tag` is a commented-out TODO at :1044/:1
Comment thread
claude[bot] marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
}
}
}
Expand Down
105 changes: 105 additions & 0 deletions test/bundler/transpiler/transpiler.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,111 @@ describe("Bun.Transpiler", () => {
it("works nested", () => {
ts.expectPrintedMin_('const a = ["hey"][0][0];', 'const a = "h"');
});
it("bails out when the array item is an optional chain", () => {
// Folding `[a?.b][0]` to `a?.b` is unsafe when the result lands as the
// target of a surrounding optional-chain continuation: the two chains
// would be spliced into one. `[[a?.b]][0]?.[0].c` must not become
// `a?.b.c`, which short-circuits to `undefined` for `a == null` instead
// of throwing on the trailing `.c`.
ts.expectPrintedMin_("x = [[a?.b]][0]?.[0].c", "x = [a?.b]?.[0].c");
ts.expectPrintedMin_("x = [[a?.b]][0]?.[0]()", "x = [a?.b]?.[0]()");
ts.expectPrintedMin_("x = [[a?.b]][0]?.[0][c]", "x = [a?.b]?.[0][c]");
ts.expectPrintedMin_("x = ({ f: [a?.b] }).f?.[0].c", "x = [a?.b]?.[0].c");
ts.expectPrintedMin_("x = [[a?.[b]]][0]?.[0].c", "x = [a?.[b]]?.[0].c");
ts.expectPrintedMin_("x = [[a?.()]][0]?.[0].c", "x = [a?.()]?.[0].c");
// Continuation (not Start) on the inlined item's outermost node:
ts.expectPrintedMin_("x = [[a?.b.c]][0]?.[0].d", "x = [a?.b.c]?.[0].d");
ts.expectPrintedMin_("x = [[a?.b[c]]][0]?.[0].d", "x = [a?.b[c]]?.[0].d");
ts.expectPrintedMin_("x = [[a?.b()]][0]?.[0].d", "x = [a?.b()]?.[0].d");
// Multi-item path (expr_can_be_removed_if_unused): a @__PURE__ optional
// call is removable, so the second fold arm sees it.
ts.expectPrintedMin_("x = [[0, /* @__PURE__ */ a?.()]][0]?.[1].c", "x = [0, a?.()]?.[1].c");
ts.expectPrintedMin_("x = [0, /* @__PURE__ */ a?.()][1]", "x = [0, a?.()][1]");

// The outer `?.` on an array literal is dropped at parse time, so these
// reach the fold with `optional_chain == None` on the index and the
// printer adds the `(a?.b)` wrapper itself. Keep bailing on the fold so
// the wrapper isn't load-bearing.
ts.expectPrintedMin_("x = [a?.b][0]", "x = [a?.b][0]");
ts.expectPrintedMin_("x = [a?.b]?.[0]", "x = [a?.b][0]");
ts.expectPrintedMin_("x = [a?.b][0].c", "x = [a?.b][0].c");
ts.expectPrintedMin_("x = [a?.b]?.[0].c", "x = [a?.b][0].c");
ts.expectPrintedMin_("x = [a?.b][0]()", "x = [a?.b][0]()");

// Same bailout protects LHS / delete / new / tagged-template positions
// from becoming `a?.b = v` / `delete a?.b` / `new a?.b()`.
ts.expectPrintedMin_("[a?.b][0] = v", "[a?.b][0] = v");
ts.expectPrintedMin_("delete [a?.b][0]", "delete [a?.b][0]");
ts.expectPrintedMin_("x = new [a?.b][0]()", "x = new [a?.b][0]");
ts.expectPrintedMin_("[a?.b][0]`x`", "[a?.b][0]`x`");

// Non-chain items are still inlined.
ts.expectPrintedMin_("x = [[y]][0]?.[0].c", "x = y.c");
ts.expectPrintedMin_("x = [a.b][0].c", "x = a.b.c");
ts.expectPrintedMin_("x = [(a?.b)][0]", "x = [a?.b][0]");
});
it("bails out or strips `this` when the index is a call/assignment target", () => {
// `[obj.m][0]()` calls through a Reference into the temporary array,
// so `this` is the array; inlining to `obj.m()` would bind `this` to
// `obj`. Match the sibling folds and emit `(0, obj.m)()`.
ts.expectPrintedMin_("x = [obj.m][0]()", "x = (0, obj.m)()");
ts.expectPrintedMin_("x = [obj[m]][0]()", "x = (0, obj[m])()");
ts.expectPrintedMin_("x = [obj.m][0]", "x = obj.m");
ts.expectPrintedMin_("x = [y][0]()", "x = y()");
ts.expectPrintedMin_("x = [() => y][0]()", "x = (() => y)()");

// `[x][0] = v` writes into the temporary, not `x`. Same for `"s"[n]`.
ts.expectPrintedMin_("[obj.p][0] = 5", "[obj.p][0] = 5");
ts.expectPrintedMin_("[obj.p][0] += 5", "[obj.p][0] += 5");
ts.expectPrintedMin_("[obj.p][0]++", "[obj.p][0]++");
ts.expectPrintedMin_("[x][0] = 1", "[x][0] = 1");
ts.expectPrintedMin_("[,][0] = 1", "[,][0] = 1");
ts.expectPrintedMin_('"foo"[2] = 1', '"foo"[2] = 1');
ts.expectPrintedMin_('["a", "b"][1] = 1', '["a", "b"][1] = 1');
ts.expectPrintedMin_("({ a: [obj.p][0] } = {})", "({ a: [obj.p][0] } = {})");
});
it("preserves runtime semantics when inlining from a literal index", async () => {
const src = `
var a = null;
function check(label, fn, expected) {
var got;
try { got = "=> " + fn(); } catch (e) { got = e.constructor.name; }
console.log(label + ": " + (got === expected ? "ok" : got + " (want " + expected + ")"));
}
check("chain .c", () => [[a?.b]][0]?.[0].c, "TypeError");
check("chain ()", () => [[a?.b]][0]?.[0](), "TypeError");
check("chain [0]", () => [[a?.b]][0]?.[0][0], "TypeError");
check("chain obj", () => ({ f: [a?.b] }).f?.[0].c, "TypeError");
check("chain flat", () => [a?.b][0].c, "TypeError");
check("chain ?.[", () => [a?.b]?.[0].c, "TypeError");
check("chain cont", () => [[a?.b.c]][0]?.[0].d, "TypeError");
check("chain pure", () => [[0, /* @__PURE__ */ a?.()]][0]?.[1].c, "TypeError");
var obj = { n: "obj", m() { return this === obj; } };
check("this", () => [obj.m][0](), "=> false");
var o2 = { p: 1 };
check("assign", () => ([o2.p][0] = 5, o2.p), "=> 1");
`;
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", src],
env: bunEnv,
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toBe("");
Comment thread
robobun marked this conversation as resolved.
expect(stdout.trim().split("\n")).toEqual([
"chain .c: ok",
"chain (): ok",
"chain [0]: ok",
"chain obj: ok",
"chain flat: ok",
"chain ?.[: ok",
"chain cont: ok",
"chain pure: ok",
"this: ok",
"assign: ok",
]);
expect(exitCode).toBe(0);
});
it("bails out on optional-chain index into enum", () => {
const pre = "enum Foo { A }\nenum Bar { 'a-b' = 1 }\n";
const lastLine = out => out.trimEnd().split("\n").at(-1);
Expand Down
Loading