Skip to content
Open
Show file tree
Hide file tree
Changes from 6 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
24 changes: 18 additions & 6 deletions src/js_parser/fold.rs
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,10 @@
}
}
js_ast::ExprData::EString(str_) => {
if p.options.features.minify_syntax {
if p.options.features.minify_syntax
&& !identifier_opts.is_delete_target()
&& identifier_opts.assign_target() == js_ast::AssignTarget::None
{
// minify "long-string".length to 11
if name == b"length" {
if let Some(len) = e_string_javascript_length(&str_) {
Expand Down Expand Up @@ -492,11 +495,14 @@
}
}
js_ast::ExprData::EImportMeta(_) => {
if name == b"main" {
let can_inline = !identifier_opts.is_delete_target()
&& identifier_opts.assign_target() == js_ast::AssignTarget::None;
Comment thread
robobun marked this conversation as resolved.

if can_inline && name == b"main" {
return Some(p.value_for_import_meta_main(false, target.loc));
}

if name == b"hot" {
if can_inline && name == b"hot" {
return Some(Expr {
data: js_ast::ExprData::ESpecial(
if p.options.features.hot_module_reloading {
Expand All @@ -510,9 +516,10 @@
}

// Inline import.meta properties for Bake
if p.options.framework.is_some()
|| (p.options.bundle
&& p.options.output_format == js_parser::options::Format::Cjs)
if can_inline
&& (p.options.framework.is_some()
|| (p.options.bundle
&& p.options.output_format == js_parser::options::Format::Cjs))
{
if name == b"dir" || name == b"dirname" {
// Inline import.meta.dir
Expand Down Expand Up @@ -663,7 +670,12 @@
}
}
}
E::Special::HotEnabled | E::Special::HotDisabled => {
if identifier_opts.is_delete_target()
|| identifier_opts.assign_target() != js_ast::AssignTarget::None
{
return None;
}

Check failure on line 678 in src/js_parser/fold.rs

View check run for this annotation

Claude / Claude Code Review

HotEnabled guard regresses import.meta.hot.<name> assignment under dev server

The new guard fires for both `HotEnabled` and `HotDisabled`, but under the Bake dev server (`HotEnabled`) it turns previously-working `import.meta.hot.data = obj` into `hmr.indirectHot.data = obj`, which throws at runtime because `indirectHot` is a Proxy whose `set`/`get` traps unconditionally throw. The `delete undefined` / `undefined = fn` motivation only applies to `HotDisabled`; gate the early `return None` on `!enabled` (or `matches!(special, E::Special::HotDisabled)`) so the HMR-enabled ar
Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
let enabled = p.options.features.hot_module_reloading;
if name == b"data" {
return Some(if enabled {
Expand Down
9 changes: 8 additions & 1 deletion src/js_parser/visit/visit_expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1057,7 +1057,13 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
let target = e_.target.unwrap_inlined();
let index = e_.index.unwrap_inlined();

if p.options.features.minify_syntax {
// Folding a property reference to a value is unsafe where the
// reference itself is observed (delete result, assign target, call receiver).
Comment thread
robobun marked this conversation as resolved.
Outdated
if p.options.features.minify_syntax
&& !is_delete_target
&& !is_call_target
&& in_.assign_target == js_ast::AssignTarget::None
{
Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
if let Some(number) = index.data.as_e_number() {
if number.value() >= 0.0
&& number.value() < (usize::MAX as f64)
Expand Down Expand Up @@ -1218,6 +1224,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
}
}
Op::UnDelete => {
p.delete_target = e_.value.data;
Comment thread
robobun marked this conversation as resolved.
p.visit_expr_in_out(&mut e_.value, ExprIn::default());
Comment thread
robobun marked this conversation as resolved.
}
_ => {
Expand Down
3 changes: 2 additions & 1 deletion src/jsc/RuntimeTranspilerCache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ bun_core::declare_scope!(cache, visible);
/// bindings from the compiled bytecode after the module-loader rewrite, so the
/// record no longer carries them; blobs written in the old numbering must not
/// be read back.
const EXPECTED_VERSION: u32 = 24;
/// Version 25: `delete`/assign/call targets no longer fold under minify_syntax.
const EXPECTED_VERSION: u32 = 25;

/// 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
30 changes: 30 additions & 0 deletions test/bundler/bundler_cjs2esm.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,36 @@ describe("bundler", () => {
stdout: '[[{"xyz":456},456],[{"xyz":123},123],[{"xyz":456},456],[{"xyz":123},123]]',
},
});
itBundled("cjs2esm/DeleteExportsPropertyDeopt", {
files: {
"/entry.js": /* js */ `
import * as lib from './lib.js';
console.log(lib.a, lib.b);
`,
"/lib.js": /* js */ `
exports.a = 1;
exports.b = 2;
delete exports.a;
`,
},
cjs2esm: { unhandled: ["/lib.js"] },
run: { stdout: "undefined 2" },
});
itBundled("cjs2esm/DeleteModuleExportsPropertyDeopt", {
files: {
"/entry.js": /* js */ `
import * as lib from './lib.js';
console.log(lib.a, lib.b);
`,
"/lib.js": /* js */ `
module.exports.a = 1;
module.exports.b = 2;
delete module.exports.a;
`,
},
cjs2esm: { unhandled: ["/lib.js"] },
run: { stdout: "undefined 2" },
});
// https://github.com/oven-sh/bun/issues/4565
// `exports.x = ...` as the unbraced body of if/while/do/else must not be
// converted to `var $x = ...; export { $x as x };` because `export` is only
Expand Down
11 changes: 11 additions & 0 deletions test/bundler/transpiler/assign-to-import.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,17 @@ describe("assigning to an imported binding", () => {
`import * as ns from "./m.mjs"; const k = "x"; ns[k] = 5;\n`,
'Cannot assign to property on import "ns"',
],
["delete namespace property", `import * as ns from "./m.mjs"; delete ns.x;\n`, 'Cannot assign to import "x"'],
[
"delete string-index namespace property",
`import * as ns from "./m.mjs"; delete ns["x"];\n`,
'Cannot assign to import "x"',
],
[
"delete computed namespace property",
`import * as ns from "./m.mjs"; const k = "x"; delete ns[k];\n`,
'Cannot assign to property on import "ns"',
],
])("bun build still rejects it: %s", async (_name, entry, diagnostic) => {
using dir = tempDir("assign-to-import-build", {
"m.mjs": mod,
Expand Down
83 changes: 83 additions & 0 deletions test/bundler/transpiler/transpiler.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,89 @@ describe("Bun.Transpiler", () => {
it("works nested", () => {
ts.expectPrintedMin_('const a = ["hey"][0][0];', 'const a = "h"');
});
it("bails out when the index is a delete/assign/call target", () => {
// `a[n]` is a property reference; folding it to a value changes the
// result of `delete`, the effect of assignment, and the `this`
// binding of a call.
ts.expectPrintedMin_("x = delete [y][0]", "x = delete [y][0]");
ts.expectPrintedMin_("x = delete [y.z][0]", "x = delete [y.z][0]");
ts.expectPrintedMin_("x = delete { f: y }.f", "x = delete { f: y }.f");
ts.expectPrintedMin_("x = delete { f: y }['f']", "x = delete { f: y }.f");
ts.expectPrintedMin_('x = delete "foo"[2]', 'x = delete "foo"[2]');
ts.expectPrintedMin_('x = delete "foo".length', 'x = delete "foo".length');
ts.expectPrintedMin_("x = [y][0] = 5", "x = [y][0] = 5");
ts.expectPrintedMin_("x = [y][0] += 5", "x = [y][0] += 5");
ts.expectPrintedMin_("[y][0]++", "[y][0]++");
ts.expectPrintedMin_('x = "foo"[2] = 5', 'x = "foo"[2] = 5');
ts.expectPrintedMin_("x = [y.z][0]()", "x = [y.z][0]()");
ts.expectPrintedMin_("x = [y][0]()", "x = [y][0]()");
// Comma / `?:` / `??` / `||` / `&&` produce a value, not a reference;
// when a fold hoists the live arm up to the delete, the printer
// re-wraps it using WAS_ORIGINALLY_DELETE_OF_IDENTIFIER_OR_PROPERTY_ACCESS.
ts.expectPrinted_("x = delete (true ? a.b : 0)", "x = delete (0, a.b)");
ts.expectPrinted_("x = delete (true ? a : 0)", "x = delete (0, a)");
ts.expectPrintedMin_("x = delete (0, a.b)", "x = delete (0, a.b)");
ts.expectPrintedMin_("x = delete (null ?? a.b)", "x = delete (0, a.b)");
ts.expectPrintedMin_("x = delete (0 || a.b)", "x = delete (0, a.b)");
ts.expectPrintedMin_("x = delete (1 && a.b)", "x = delete (0, a.b)");
Comment thread
robobun marked this conversation as resolved.
// Still inlined outside those positions.
ts.expectPrintedMin_("x = [y][0]", "x = y");
ts.expectPrintedMin_('x = "foo"[2]', 'x = "o"');
ts.expectPrintedMin_('x = "foo".length', "x = 3");
ts.expectPrintedMin_("x = delete [y][0].z", "x = delete y.z");
ts.expectPrintedMin_("x = f([y][0])", "x = f(y)");
});
it("does not inline an enum member under delete", () => {
const pre = "enum E { A = 1 }\n";
const lastLine = out => out.trimEnd().split("\n").at(-1);
expect(lastLine(ts.parsed(pre + "x = delete E.A;", false))).toBe("x = delete E.A;");
expect(lastLine(ts.parsed(pre + 'x = delete E["A"];', false))).toBe('x = delete E["A"];');
expect(lastLine(ts.parsedMin(pre + "x = delete E.A;", false))).toBe("x = delete E.A;");
expect(lastLine(ts.parsedMin(pre + 'x = delete E["A"];', false))).toBe("x = delete E.A;");
// Still inlined when read.
expect(lastLine(ts.parsed(pre + "x = E.A;", false))).toBe("x = 1 /* A */;");
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
it("does not substitute --define for a delete target", () => {
// `user_undefined` is defined as `undefined` in the transpiler config above.
ts.expectPrintedMin_("x = delete user_undefined", "x = delete user_undefined");
// A later read is still substituted (delete_target is per-node, not per-symbol).
ts.expectPrintedMin_(
"x = delete user_undefined; y = user_undefined;",
"x = delete user_undefined;\ny = void 0;\n",
);
});
it("does not inline import.meta.<prop> under delete or assignment", () => {
// Inlining would produce `delete undefined` (strict-mode SyntaxError) or
// `undefined = 5` for the HotDisabled value.
ts.expectPrinted_("x = delete import.meta.hot", "x = delete import.meta.hot");
ts.expectPrinted_("x = delete import.meta.main", "x = delete import.meta.main");
ts.expectPrinted_("import.meta.hot = 5", "import.meta.hot = 5");
ts.expectPrinted_("import.meta.main = 5", "import.meta.main = 5");
ts.expectPrinted_("x = delete import.meta.hot.accept", "x = delete undefined.accept");
ts.expectPrinted_("import.meta.hot.accept = fn", "undefined.accept = fn");
// Reads are still inlined.
ts.expectPrinted_("x = import.meta.hot", "x = undefined");
});
it("preserves delete/assign/call-receiver semantics at runtime", async () => {
const src = `
var obj = { p: 1 };
var r1 = delete [obj.p][0];
var y = 1;
[y][0] = 5;
var k = { f: 7 };
var r2 = delete { f: k.f }.f;
var o = { m() { return this === o } };
var r3 = [o.m][0]();
var q = { p: 1 };
delete (0, q.p);
console.log(JSON.stringify([obj.p, r1, y, k.f, r2, r3, q.p]));
`;
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("");
expect(stdout).toBe("[1,true,1,7,true,false,1]\n");
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