From c8931d0f654a2a91bb6f21a8b3275c7acccaf5c1 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 18 Oct 2026 04:06:50 +0000 Subject: [PATCH 1/3] js_parser: copy object and array defines per use, and copy the list of a lone JSX spread object A use of an object or array define got the node of the define table. The automatic JSX runtime took the property list of a lone spread object, so one `
` left the define empty for every other use, file and thread. DefineData::value_for_use now returns a copy of a string, object or array value, and the value field is private. The JSX collapse copies the list of the spread object, which a const that holds a macro result can share with other uses. --- src/ast/expr.rs | 81 +++++++++++++++++++++ src/ast/g.rs | 15 ++++ src/bundler/defines.rs | 68 +++++------------- src/bundler/options.rs | 4 +- src/bundler/transpiler.rs | 2 +- src/js_parser/lib.rs | 49 +++++++++++-- src/js_parser/p.rs | 15 ++-- src/js_parser/visit/visit_expr.rs | 62 ++++++++-------- src/jsc/RuntimeTranspilerCache.rs | 4 +- test/bake/dev/hot.test.ts | 50 +++++++++++++ test/bundler/bundler_jsx.test.ts | 36 ++++++++++ test/bundler/transpiler/macro-test.test.ts | 40 +++++++++++ test/bundler/transpiler/transpiler.test.js | 83 ++++++++++++++++++++++ test/cli/run/transpiler-cache.test.ts | 21 ++++++ 14 files changed, 430 insertions(+), 100 deletions(-) diff --git a/src/ast/expr.rs b/src/ast/expr.rs index 056ca92028e8..57ccca6d0825 100644 --- a/src/ast/expr.rs +++ b/src/ast/expr.rs @@ -133,6 +133,87 @@ impl Expr { // `bun_js_parser_jsc::macro_::expr_from_blob`. } +/// A copied object or array whose children still point at nodes of the source. +enum LiteralContainer { + Object(StoreRef), + Array(StoreRef), +} + +impl Expr { + /// Copies a tree of object, array and string literals. The copy shares + /// string bytes with `self` and no node; a node of any other kind is + /// copied by value. Nodes go in the thread-local `Store`, lists in the + /// active `AstAlloc` scope. Iterative: the tree can be deeper than the + /// stack of this thread allows a recursion. + #[cold] + pub fn clone_literal_tree(self) -> Expr { + let mut root = self; + let mut work: Vec = Vec::new(); + let mut next = root.own_literal_node(); + while let Some(container) = next { + match container { + LiteralContainer::Object(mut object) => { + for property in object.properties.slice_mut() { + let children = [ + &mut property.key, + &mut property.value, + &mut property.initializer, + ]; + for child in children.into_iter().flatten() { + work.extend(child.own_literal_node()); + } + } + } + LiteralContainer::Array(mut array) => { + for item in array.items.slice_mut() { + work.extend(item.own_literal_node()); + } + } + } + next = work.pop(); + } + root + } + + /// Points `self` at a field-wise copy of its string, object or array node. + fn own_literal_node(&mut self) -> Option { + match self.data { + Data::EString(string) => { + self.data = Data::EString(data::Store::append(string.shallow_clone())); + None + } + Data::EObject(object) => { + let mut properties: G::PropertyList = + Vec::with_capacity_in(object.properties.len(), bun_alloc::AstAlloc); + properties.extend(object.properties.iter().map(G::Property::shallow_clone)); + let copy = data::Store::append(E::Object { + properties, + comma_after_spread: object.comma_after_spread, + is_single_line: object.is_single_line, + is_parenthesized: object.is_parenthesized, + was_originally_macro: object.was_originally_macro, + close_brace_loc: object.close_brace_loc, + }); + self.data = Data::EObject(copy); + Some(LiteralContainer::Object(copy)) + } + Data::EArray(array) => { + let copy = data::Store::append(E::Array { + items: bun_alloc::AstAlloc::vec_from_slice(array.items.slice()), + comma_after_spread: array.comma_after_spread, + is_single_line: array.is_single_line, + is_parenthesized: array.is_parenthesized, + was_originally_macro: array.was_originally_macro, + close_bracket_loc: array.close_bracket_loc, + }); + self.data = Data::EArray(copy); + Some(LiteralContainer::Array(copy)) + } + _ => None, + } + } +} + #[derive(Clone, Copy)] pub struct Query { pub expr: Expr, diff --git a/src/ast/g.rs b/src/ast/g.rs index e8f650babe9b..485c1b1bb416 100644 --- a/src/ast/g.rs +++ b/src/ast/g.rs @@ -195,6 +195,21 @@ impl Property { self.class_static_block.as_deref_mut() } + /// Field-wise copy: every expression and the static block still point at + /// the nodes of `self`. The decorator list is a new list of the same items. + pub fn shallow_clone(&self) -> Property { + Property { + initializer: self.initializer, + kind: self.kind, + flags: self.flags, + class_static_block: self.class_static_block, + ts_decorators: bun_alloc::AstAlloc::vec_from_slice(self.ts_decorators.slice()), + key: self.key, + value: self.value, + ts_metadata: self.ts_metadata.clone(), + } + } + pub(crate) fn deep_clone( &self, bump: &bun_alloc::Arena, diff --git a/src/bundler/defines.rs b/src/bundler/defines.rs index 2eefd5af1d07..f6e7c5ad4a9a 100644 --- a/src/bundler/defines.rs +++ b/src/bundler/defines.rs @@ -356,6 +356,22 @@ impl DefineDataExt for DefineData { } } + let define = |value: ExprData, can_be_removed_if_unused: bool| { + DefineData::init(Options { + value, + original_name: if !value_str.is_empty() { + Some(value_str) + } else { + None + }, + valueless: value_is_undefined, + can_be_removed_if_unused, + call_can_be_unwrapped_if_unused: bun_ast::E::CallUnwrap::Never, + method_call_must_be_replaced_with_undefined: + method_call_must_be_replaced_with_undefined_, + }) + }; + if is_ident { // Special-case undefined. it's not an identifier here // https://github.com/evanw/esbuild/issues/1407 @@ -367,25 +383,7 @@ impl DefineDataExt for DefineData { ) }; - return Ok(DefineData { - value, - // upstream `DefineData` now owns `original_name: - // Option>` (js_parser/lib.rs:1369) instead of a - // borrowed `ptr`/`len` pair. Dupe - // the value bytes — these are tiny startup-time copies. - original_name: if !value_str.is_empty() { - Some(Box::<[u8]>::from(value_str)) - } else { - None - }, - flags: Flags::new( - /* valueless: */ value_is_undefined, - /* can_be_removed_if_unused: */ true, - /* call_can_be_unwrapped_if_unused: */ bun_ast::E::CallUnwrap::Never, - /* method_call_must_be_replaced_with_undefined: */ - method_call_must_be_replaced_with_undefined_, - ), - }); + return Ok(define(value, true)); } // Fast path for the compile-time-constant literal define *values* that @@ -399,21 +397,7 @@ impl DefineDataExt for DefineData { // (created lazily below only when a value really needs JSON parsing). if let Some(value) = const_default_define_value(value_str) { let can_be_removed_if_unused = bun_ast::expr::Tag::is_primitive_literal(value.tag()); - return Ok(DefineData { - value, - original_name: if !value_str.is_empty() { - Some(Box::<[u8]>::from(value_str)) - } else { - None - }, - flags: Flags::new( - /* valueless: */ value_is_undefined, - /* can_be_removed_if_unused: */ can_be_removed_if_unused, - /* call_can_be_unwrapped_if_unused: */ bun_ast::E::CallUnwrap::Never, - /* method_call_must_be_replaced_with_undefined: */ - method_call_must_be_replaced_with_undefined_, - ), - }); + return Ok(define(value, can_be_removed_if_unused)); } // `parse_env_json` builds `E::String`/`E::Object` nodes in the @@ -446,21 +430,7 @@ impl DefineDataExt for DefineData { let data: ExprData = bun_parsers::json_parser::clone_out_of_store(&expr, &source, log, bump)?.data; let can_be_removed_if_unused = bun_ast::expr::Tag::is_primitive_literal(data.tag()); - Ok(DefineData { - value: data, - original_name: if !value_str.is_empty() { - Some(Box::<[u8]>::from(value_str)) - } else { - None - }, - flags: Flags::new( - /* valueless: */ value_is_undefined, - /* can_be_removed_if_unused: */ can_be_removed_if_unused, - /* call_can_be_unwrapped_if_unused: */ bun_ast::E::CallUnwrap::Never, - /* method_call_must_be_replaced_with_undefined: */ - method_call_must_be_replaced_with_undefined_, - ), - }) + Ok(define(data, can_be_removed_if_unused)) } fn from_input( diff --git a/src/bundler/options.rs b/src/bundler/options.rs index ac49bb525032..4941915c88f5 100644 --- a/src/bundler/options.rs +++ b/src/bundler/options.rs @@ -943,8 +943,8 @@ pub(crate) fn defines_from_transform_options( .keys() .iter() .zip(environment_defines.values().iter()) - .filter_map(|(k, v)| match &v.value { - defines::DefineValue::EString(s) if s.is_utf8() => Some((k.as_ref(), s.slice8())), + .filter_map(|(k, v)| match v.string() { + Some(s) if s.is_utf8() => Some((k.as_ref(), s.slice8())), _ => None, }), drop.iter().copied(), diff --git a/src/bundler/transpiler.rs b/src/bundler/transpiler.rs index a496e8e35ad5..de9e9c15c88f 100644 --- a/src/bundler/transpiler.rs +++ b/src/bundler/transpiler.rs @@ -538,7 +538,7 @@ impl<'a> Transpiler<'a> { if had_explicit_node_env { if let Some(node_env) = self.options.define.dots.get(b"NODE_ENV".as_slice()) { if !node_env.is_empty() { - if let Some(s) = node_env[0].data.value.e_string() { + if let Some(s) = node_env[0].data.string() { if s.eql_comptime(b"production") { is_production = true; } else if s.eql_comptime(b"development") { diff --git a/src/js_parser/lib.rs b/src/js_parser/lib.rs index fecb03f87d3f..c8ab5aabcea0 100644 --- a/src/js_parser/lib.rs +++ b/src/js_parser/lib.rs @@ -268,6 +268,7 @@ pub mod defines { use bun_core::strings; use bun_ast::E; + use bun_ast::Expr; use bun_ast::StoreRef; use bun_ast::expr::Data as ExprData; @@ -359,7 +360,9 @@ pub mod defines { #[derive(Clone)] pub struct DefineData { - pub value: ExprData, + /// Nodes of the define table. Private: `value_for_use` is the only way + /// from here to an `Expr`, and it copies every node it returns. + value: ExprData, // The `RawDefines` value bytes are owned (`Box<[u8]>`), so borrowing // would be a use-after-free once the `RawDefines` map is dropped after // `Define::init`. Own the bytes here instead — these are tiny @@ -370,12 +373,11 @@ pub mod defines { pub flags: Flags, } - // SAFETY: `ExprData` contains `StoreRef` raw pointers into immutable, - // process-lifetime AST stores. `DefineData` is only shared across threads - // via the read-only `Box` after init. Never written through. + // SAFETY: the nodes behind `value` are not written after init. They never + // enter an AST, which its passes edit in place: `value` is private and + // `value_for_use` returns copies. Threads share the table read-only. unsafe impl Send for DefineData {} - // SAFETY: see `Send` impl above — the `StoreRef` targets are immutable and - // process-lifetime, and `DefineData` is read-only after init. + // SAFETY: see the `Send` impl above. unsafe impl Sync for DefineData {} impl Default for DefineData { @@ -452,6 +454,41 @@ pub mod defines { self.flags.valueless() } + /// The identifier this define names, for the parser to resolve. + #[inline] + pub(crate) fn identifier(&self) -> Option { + match self.value { + ExprData::EIdentifier(id) => Some(id), + _ => None, + } + } + + /// The value as one use of the define gets it. A string, an object or + /// an array is a copy, so no pass over the AST can write to the table. + #[inline] + pub(crate) fn value_for_use(&self, loc: bun_ast::Loc) -> Expr { + if let ExprData::EString(string) = self.value { + return Expr::init(&*string, loc); + } + let expr = Expr { + data: self.value, + loc, + }; + if matches!(self.value, ExprData::EObject(_) | ExprData::EArray(_)) { + return expr.clone_literal_tree(); + } + expr + } + + /// The string this define holds. + #[inline] + pub fn string(&self) -> Option<&E::EString> { + match &self.value { + ExprData::EString(string) => Some(string.get()), + _ => None, + } + } + pub fn init_boolean(value: bool) -> DefineData { let mut flags = Flags::default(); flags.set_can_be_removed_if_unused(true); diff --git a/src/js_parser/p.rs b/src/js_parser/p.rs index fa4358ab4718..de9bc414b6b9 100644 --- a/src/js_parser/p.rs +++ b/src/js_parser/p.rs @@ -7114,9 +7114,8 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O ) -> Expr { // Callers gate on `!valueless()` before reaching here, so `value` is a // real Expr.Data by contract. - let value = define_data.value; - match value { - js_ast::ExprData::EIdentifier(id) => { + match define_data.identifier() { + Some(id) => { // Identifier defines always carry a name. Match the // contract so `handle_identifier`'s trailing `find_symbol` // rebind runs against the *resolved* scope ref, not the @@ -7130,7 +7129,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O // `Option<&'a [u8]>` param. let original_name: &'a [u8] = unsafe { bun_collections::detach_lifetime(original_name) }; - return self.handle_identifier( + self.handle_identifier( loc, id, Some(original_name), @@ -7138,14 +7137,10 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O .with_assign_target(assign_target) .with_is_delete_target(is_delete_target) .with_was_originally_identifier(true), - ); - } - js_ast::ExprData::EString(str_) => { - return self.new_expr(&*str_, loc); + ) } - _ => {} + None => define_data.value_for_use(loc), } - Expr { data: value, loc } } // `parts` is `&[Box<[u8]>]` to match the active `DotDefine.parts: diff --git a/src/js_parser/visit/visit_expr.rs b/src/js_parser/visit/visit_expr.rs index 5bd49840e95d..51d3454da40f 100644 --- a/src/js_parser/visit/visit_expr.rs +++ b/src/js_parser/visit/visit_expr.rs @@ -470,12 +470,6 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O None }; - // `props` may be reassigned to point inside a spread object's - // properties. Track it as a `StoreRef` (safe `Deref`/`DerefMut`) so - // the spread-collapse walk and the `push`/`take` calls below stay - // in safe code. - let mut props_handle = js_ast::StoreRef::from_bump(&mut e_.properties); - // arguments needs to be like // { // ...props, @@ -508,34 +502,40 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O // -> //
// jsx("div", {...foo}) + let mut spread_object: Option> = None; loop { - // `StoreRef: Deref` — safe arena-backed read. - if !(props_handle.len_u32() == 1 - && props_handle.slice()[0].kind == G::PropertyKind::Spread - && matches!( - props_handle.slice()[0].value.unwrap().data, - Data::EObject(..) - )) - { + let properties = match &spread_object { + Some(object) => &object.properties, + None => &e_.properties, + }; + let [ + G::Property { + kind: G::PropertyKind::Spread, + value: + Some(Expr { + data: Data::EObject(inner), + .. + }), + .. + }, + ] = properties.slice() + else { break; - } - // Compute the next handle in a block so the `DerefMut` - // borrow of `props_handle` ends before reassignment. - let next = { - let inner = props_handle.slice_mut()[0] - .value - .as_mut() - .unwrap() - .data - .e_object_mut() - .unwrap(); - js_ast::StoreRef::from_bump(&mut inner.properties) }; - props_handle = next; + spread_object = Some(*inner); } - // `StoreRef: DerefMut` — safe arena-backed handle; no aliasing - // `&mut` outstanding for the remainder of this arm. - let props = &mut *props_handle; + let mut props: G::PropertyList = match spread_object { + // Other expressions can point at this object: each use of a + // `const` that holds a macro result does. Copy its list. + Some(object) => { + let mut props = G::PropertyList::init_capacity( + object.properties.len() + usize::from(!e_.children.is_empty()), + ); + props.extend(object.properties.iter().map(G::Property::shallow_clone)); + props + } + None => bun_alloc::AstAlloc::take(&mut e_.properties), + }; // Typescript defines static jsx as children.len > 1 or single spread // https://github.com/microsoft/TypeScript/blob/d4fbc9b57d9aa7d02faac9b1e9bb7b37c687f6e9/src/compiler/transformers/jsx.ts#L340 @@ -582,7 +582,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O &mut args, p.new_expr( E::Object { - properties: bun_alloc::AstAlloc::take(props), + properties: props, ..Default::default() }, expr.loc, diff --git a/src/jsc/RuntimeTranspilerCache.rs b/src/jsc/RuntimeTranspilerCache.rs index 8b9460b7d3bc..d0b00639db1f 100644 --- a/src/jsc/RuntimeTranspilerCache.rs +++ b/src/jsc/RuntimeTranspilerCache.rs @@ -67,7 +67,9 @@ bun_core::declare_scope!(cache, visible); /// Version 35: Long hexadecimal, octal and binary literals are rounded to the nearest double. /// Version 36: `export`, `require()`, `import()`, a JSX element, an exported async function and /// an abstract class are mapped elsewhere. -const EXPECTED_VERSION: u32 = 36; +/// Version 37: A JSX spread no longer empties an object `--define` or an inlined macro +/// result. Older entries print `{}` for the uses that the module loader transpiled after it. +const EXPECTED_VERSION: u32 = 37; /// 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 diff --git a/test/bake/dev/hot.test.ts b/test/bake/dev/hot.test.ts index 92e158155dea..46ce4a3f690b 100644 --- a/test/bake/dev/hot.test.ts +++ b/test/bake/dev/hot.test.ts @@ -313,6 +313,56 @@ devTest("import.meta.hot.accept multiple modules", { await c.expectMessageInAnyOrder("Counter updated: 3", "Name updated: Charlie"); }, }); +// An array `define` is a literal at each use. `import.meta.hot.accept` replaces the items of its +// dependency list, and that must not change what the next use or the next parse gets for the define. +devTest("import.meta.hot.accept with a dependency list from a define", { + files: { + "bunfig.toml": ` + [serve.static] + define = { DEPS = '["./counter.ts"]' } + `, + "index.html": emptyHtmlFile({ + scripts: ["index.ts"], + }), + "index.ts": ` + import { count } from "./counter.ts"; + console.log("Initial: " + count + " " + JSON.stringify(DEPS)); + + import.meta.hot.accept(DEPS, (newModules) => { + console.log("Counter updated: " + newModules[0].count); + }); + `, + "counter.ts": ` + export const count = 1; + `, + }, + async test(dev) { + await using c = await dev.client("/"); + await c.expectMessage(`Initial: 1 ["./counter.ts"]`); + + await dev.write( + "counter.ts", + ` + export const count = 2; + `, + ); + await c.expectMessage("Counter updated: 2"); + + // index.ts does not accept itself, so the page reloads. The dev server parses the list again. + await c.expectReload(async () => { + await dev.patch("index.ts", { find: "Initial", replace: "Again" }); + }); + await c.expectMessage(`Again: 2 ["./counter.ts"]`); + + await dev.write( + "counter.ts", + ` + export const count = 3; + `, + ); + await c.expectMessage("Counter updated: 3"); + }, +}); devTest("import.meta.hot.data persistence", { files: { "index.html": emptyHtmlFile({ diff --git a/test/bundler/bundler_jsx.test.ts b/test/bundler/bundler_jsx.test.ts index d1ed6f089a0b..887ce3749bb7 100644 --- a/test/bundler/bundler_jsx.test.ts +++ b/test/bundler/bundler_jsx.test.ts @@ -527,6 +527,42 @@ describe("bundler", () => { }, }); + // An object `define` is a literal at each use. The automatic runtime passes a + // lone spread object as the props, and that must not take the properties + // away from the other uses of the define, in the same file or in another one. + describe("a spread of an object define", () => { + const config = { a: 1, nested: { list: [2, "three", { four: null }] } }; + const withChild = { ...config, children: "kid" }; + + // "cli" passes the value as --define, "api" as Bun.build({ define }). + for (const backend of ["cli", "api"] as const) { + itBundled(`jsx/SpreadOfObjectDefineKeepsOtherUses-${backend}`, { + files: { + "/index.jsx": /* jsx */ ` + import { inOtherFile } from "./other.jsx"; + import { withoutJsx } from "./plain.js"; + const before = CONFIG; + const spread =
; + const child =
kid
; + console.log(JSON.stringify([before, spread.props, child.props, CONFIG, inOtherFile, withoutJsx])); + `, + "/other.jsx": /* jsx */ ` + const child = kid; + export const inOtherFile = [child.props, CONFIG]; + `, + "/plain.js": /* js */ ` + export const withoutJsx = CONFIG; + `, + ...helpers, + }, + backend, + target: "bun", + define: { CONFIG: JSON.stringify(config) }, + run: { stdout: JSON.stringify([config, config, withChild, config, [withChild, config], config]) }, + }); + } + }); + // The classic transform reads the first member of the factory and fragment // member lists. A factory or fragment option whose text has no identifier // in it ("." or "..") must keep the list that was configured before it, diff --git a/test/bundler/transpiler/macro-test.test.ts b/test/bundler/transpiler/macro-test.test.ts index 4b5254346695..1dd03c4f72eb 100644 --- a/test/bundler/transpiler/macro-test.test.ts +++ b/test/bundler/transpiler/macro-test.test.ts @@ -220,6 +220,46 @@ test("object destructuring of a macro result keeps every bound property regardle expect(exitCode).toBe(0); }); +// A `const` that holds a macro result is inlined at each use, so every use points at one object. +// The automatic JSX runtime passes a lone spread object as the props. It has to copy the +// properties of that object: taking them leaves `{}` at the other uses. +describe.concurrent("a macro result spread into JSX keeps its properties at the other uses of its const", () => { + const files = { + "m.ts": `export function m() {\n return { a: 1, b: [2] };\n}\n`, + "node_modules/react/jsx-dev-runtime.js": `export const jsxDEV = (type, props) => props;\n`, + "node_modules/react/jsx-runtime.js": `export const jsx = (type, props) => props;\nexport const jsxs = jsx;\n`, + "index.jsx": [ + `import { m } from "./m.ts" with { type: "macro" };`, + `const x = m();`, + `const before = x;`, + `const child =
kid
;`, + `const spread =
;`, + `console.log(JSON.stringify([before, child, spread, x]));`, + ``, + ].join("\n"), + }; + const expected = `[{"a":1,"b":[2]},{"a":1,"b":[2],"children":"kid"},{"a":1,"b":[2]},{"a":1,"b":[2]}]`; + + async function run(cwd: string, args: 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 { lastLine: stdout.trim().split("\n").pop(), stderr, exitCode }; + } + + test("at run time", async () => { + using dir = tempDir("macro-jsx-spread-run", files); + expect(await run(String(dir), ["run", "index.jsx"])).toEqual({ lastLine: expected, stderr: "", exitCode: 0 }); + }); + + // The bundler inlines the const under --minify-syntax. + test("in bun build --minify-syntax", async () => { + using dir = tempDir("macro-jsx-spread-build", files); + const build = await run(String(dir), ["build", "--minify-syntax", "--target=bun", "--outfile=out.js", "index.jsx"]); + expect({ stderr: build.stderr, exitCode: build.exitCode }).toEqual({ stderr: "", exitCode: 0 }); + expect(await run(String(dir), ["out.js"])).toEqual({ lastLine: expected, stderr: "", exitCode: 0 }); + }); +}); + // A Response or Blob returned from a macro is inlined by its content type: JSON is parsed into an object // literal, text becomes a string, anything else becomes a base64 data URL. The type has to be classified // with its parameters stripped: `Response.json()` and most servers send `application/json;charset=utf-8`. diff --git a/test/bundler/transpiler/transpiler.test.js b/test/bundler/transpiler/transpiler.test.js index fcbdd6c5e660..9acfafd8bd3d 100644 --- a/test/bundler/transpiler/transpiler.test.js +++ b/test/bundler/transpiler/transpiler.test.js @@ -2593,6 +2593,89 @@ export default <>hi expect(exitCode).toBe(0); }); + // Each use of an object or array define is a literal of its own. The automatic JSX runtime + // passes a lone spread object as the props: `
` is `jsxDEV("div", { a: 1 })`. + // That must not take the properties away from the other uses of the define. + describe("define with an object or array value", () => { + // One key for each place that substitutes a define: an identifier, a property access, `import.meta`. + it.each(["OBJ", "process.env.CFG", "import.meta"])("is whole at each use after JSX spreads %s", name => { + const transpiler = new Bun.Transpiler({ loader: "jsx", define: { [name]: '{"a":1}' } }); + const source = [ + `export const before = ${name};`, + `export const spread =
;`, + `export const child =
kid
;`, + `export const after = ${name};`, + ].join("\n"); + const expected = `export const before = { a: 1 }; +export const spread = jsxDEV_7x81h0kn("div", { + a: 1 +}, undefined, false, undefined, this); +export const child = jsxDEV_7x81h0kn("div", { + a: 1, + children: "kid" +}, undefined, false, undefined, this); +export const after = { a: 1 }; +`; + // The define table lives as long as the transpiler, so the second call reads it again. + expect([transpiler.transformSync(source), transpiler.transformSync(source)]).toEqual([expected, expected]); + }); + + it("is whole at each use after a minified property access spreads a nested value", () => { + const transpiler = new Bun.Transpiler({ + loader: "jsx", + minify: { syntax: true }, + define: { CFG: '{"inner":{"__proto__":1,"q":"r"}}', LIST: '["s",{"k":[2]}]' }, + }); + const source = [ + `export const before = [CFG, LIST];`, + `export const spread = [
,
kid
];`, + `export const after = [CFG, [...LIST, 3]];`, + ].join("\n"); + const expected = `export const before = [{ inner: { ["__proto__"]: 1, q: "r" } }, ["s", { k: [2] }]], spread = [jsxDEV_7x81h0kn("div", { + ["__proto__"]: 1, + q: "r" +}, void 0, !1, void 0, this), jsxDEV_7x81h0kn("div", { + k: [2], + children: "kid" +}, void 0, !1, void 0, this)], after = [{ inner: { ["__proto__"]: 1, q: "r" } }, ["s", { k: [2] }, 3]]; +`; + expect([transpiler.transformSync(source), transpiler.transformSync(source)]).toEqual([expected, expected]); + }); + + it("is whole in transformSync() after scan() visited a JSX spread of it", () => { + const transpiler = new Bun.Transpiler({ loader: "jsx", define: { OBJ: '{"a":1}' } }); + expect(transpiler.scan("export const a =
;").exports).toEqual(["a"]); + expect(transpiler.transformSync("export const b = OBJ;")).toBe("export const b = { a: 1 };\n"); + }); + + // transform() parses on a pool thread. Every call reads the one define table of the transpiler. + it("is whole in every concurrent transform()", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const object = Object.fromEntries(Array.from({ length: 200 }, (_, i) => ["k" + i, "v" + i])); + const transpiler = new Bun.Transpiler({ loader: "jsx", define: { OBJ: JSON.stringify(object) } }); + const source = "export const a =
kid
; export const b = OBJ;"; + const outputs = await Promise.all(Array.from({ length: 64 }, () => transpiler.transform(source))); + const count = text => text.split('"v199"').length - 1; + console.log(JSON.stringify({ distinct: new Set(outputs).size, uses: count(outputs[0]) })); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({ + stdout: JSON.stringify({ distinct: 1, uses: 2 }), + stderr: "", + exitCode: 0, + }); + }); + }); + it("JSX keys", () => { var bun = new Bun.Transpiler({ loader: "jsx", diff --git a/test/cli/run/transpiler-cache.test.ts b/test/cli/run/transpiler-cache.test.ts index 0a3495d66484..9c3486898522 100644 --- a/test/cli/run/transpiler-cache.test.ts +++ b/test/cli/run/transpiler-cache.test.ts @@ -434,6 +434,27 @@ describe("transpiler cache", () => { expect(newCacheCount()).toBe(0); }); + // The cache keeps what a module printed. A module that spreads an object define into JSX + // must not change what the module loaded after it gets for that define. + test("--define with an object value is whole in a module loaded after a JSX spread of it", () => { + const react = join(temp_dir, "node_modules", "react"); + mkdirSync(react, { recursive: true }); + writeFileSync(join(react, "jsx-dev-runtime.js"), `export const jsxDEV = (type, props) => props;\n`); + writeFileSync(join(react, "jsx-runtime.js"), `export const jsx = (type, props) => props;\n`); + writeFileSync(join(temp_dir, "b.js"), `console.log(JSON.stringify(OBJ));` + "\n//" + filler); + writeFileSync( + join(temp_dir, "a.jsx"), + `console.log(JSON.stringify(
));\nawait import("./b.js");\n`, + ); + const define = ["--define", 'OBJ={"k":"v"}']; + + expect(run(temp_dir, [...define, "a.jsx"])).toBe('{"k":"v"}\n{"k":"v"}'); + expect(newCacheCount()).toBe(1); + // This process has no JSX. It reads the entry that the first process wrote for b.js. + expect(run(temp_dir, [...define, "b.js"])).toBe('{"k":"v"}'); + expect(newCacheCount()).toBe(0); + }); + test("--drop invalidates cache", () => { writeFileSync( join(temp_dir, "a.js"), From 3243987b6e2570d4a64b40fe678dfb7397173939 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 18 Oct 2026 16:04:46 +0000 Subject: [PATCH 2/3] Address self-review: assert that a use gets no table node, drop dead pub surface - DefineData::value_for_use asserts in debug builds that the value it returns shares no string, object or array node with the define table. - DefineData's fields and Flags are no longer public, and the SyncDefineData wrapper is gone: DefineData is Send and Sync itself. - The concurrent transform() test uses 200 spreads, which crashes a build without the fix. - Comments are one line each. --- src/ast/expr.rs | 45 ++++++++++-- src/ast/g.rs | 3 +- src/bundler/defines.rs | 4 +- src/js_parser/defines_table.rs | 80 ++++++++-------------- src/js_parser/lib.rs | 37 +++++----- src/js_parser/p.rs | 2 - src/js_parser/visit/visit_expr.rs | 3 +- src/jsc/RuntimeTranspilerCache.rs | 3 +- test/bake/dev/hot.test.ts | 2 +- test/bundler/bundler_jsx.test.ts | 2 +- test/bundler/transpiler/transpiler.test.js | 24 ++++--- 11 files changed, 107 insertions(+), 98 deletions(-) diff --git a/src/ast/expr.rs b/src/ast/expr.rs index 57ccca6d0825..ad53abf38bbd 100644 --- a/src/ast/expr.rs +++ b/src/ast/expr.rs @@ -140,14 +140,11 @@ enum LiteralContainer { } impl Expr { - /// Copies a tree of object, array and string literals. The copy shares - /// string bytes with `self` and no node; a node of any other kind is - /// copied by value. Nodes go in the thread-local `Store`, lists in the - /// active `AstAlloc` scope. Iterative: the tree can be deeper than the - /// stack of this thread allows a recursion. + /// Copies a tree of object, array and string literals. The copy shares no node with `self`. #[cold] pub fn clone_literal_tree(self) -> Expr { let mut root = self; + // A work list, not recursion: a define can nest deeper than the stack of a pool thread allows. let mut work: Vec = Vec::new(); let mut next = root.own_literal_node(); while let Some(container) = next { @@ -212,6 +209,44 @@ impl Expr { _ => None, } } + + /// Whether `self` and `other` share a string, object or array node, or a list, at the same place. + pub fn shares_literal_node_with(&self, other: &Expr) -> bool { + let mut work: Vec<(Expr, Expr)> = vec![(*self, *other)]; + while let Some((a, b)) = work.pop() { + match (a.data, b.data) { + (Data::EString(a), Data::EString(b)) => { + if a == b { + return true; + } + } + (Data::EObject(a), Data::EObject(b)) => { + if a == b + || (!a.properties.is_empty() + && a.properties.as_ptr() == b.properties.as_ptr()) + { + return true; + } + for (a, b) in a.properties.iter().zip(b.properties.iter()) { + let children = [ + (a.key, b.key), + (a.value, b.value), + (a.initializer, b.initializer), + ]; + work.extend(children.into_iter().filter_map(|(a, b)| a.zip(b))); + } + } + (Data::EArray(a), Data::EArray(b)) => { + if a == b || (!a.items.is_empty() && a.items.as_ptr() == b.items.as_ptr()) { + return true; + } + work.extend(a.items.iter().copied().zip(b.items.iter().copied())); + } + _ => {} + } + } + false + } } #[derive(Clone, Copy)] diff --git a/src/ast/g.rs b/src/ast/g.rs index 485c1b1bb416..80d6942ea2e0 100644 --- a/src/ast/g.rs +++ b/src/ast/g.rs @@ -195,8 +195,7 @@ impl Property { self.class_static_block.as_deref_mut() } - /// Field-wise copy: every expression and the static block still point at - /// the nodes of `self`. The decorator list is a new list of the same items. + /// Field-wise copy: the expressions and the static block still point at the nodes of `self`. pub fn shallow_clone(&self) -> Property { Property { initializer: self.initializer, diff --git a/src/bundler/defines.rs b/src/bundler/defines.rs index f6e7c5ad4a9a..97821e980448 100644 --- a/src/bundler/defines.rs +++ b/src/bundler/defines.rs @@ -10,7 +10,7 @@ use crate::defines_table::{ }; // ══════════════════════════════════════════════════════════════════════════ -// B-3 UNIFIED: `Define` / `DefineData` / `DotDefine` / `Flags` / `Options` / +// B-3 UNIFIED: `Define` / `DefineData` / `DotDefine` / `Options` / // `RawDefines` / `UserDefines` / `UserDefinesArray` are canonical in // `bun_js_parser::defines` (lower tier) so the parser's `P.define: &'a Define` // and `BundleOptions.define: Box` are the *same* nominal type. This @@ -21,7 +21,7 @@ use crate::defines_table::{ // directly with no cross-crate hook. // ══════════════════════════════════════════════════════════════════════════ pub use bun_js_parser::defines::{ - Define, DefineData, DotDefine, Flags, IdentifierDefine, Options, RawDefines, UserDefines, + Define, DefineData, DotDefine, IdentifierDefine, Options, RawDefines, UserDefines, UserDefinesArray, are_parts_equal, }; diff --git a/src/js_parser/defines_table.rs b/src/js_parser/defines_table.rs index ee9c23167bc8..0e7cbed9e693 100644 --- a/src/js_parser/defines_table.rs +++ b/src/js_parser/defines_table.rs @@ -192,76 +192,52 @@ pub static GLOBAL_NO_SIDE_EFFECT_FUNCTION_CALLS_SAFE_FOR_TO_STRING: &[&[&[u8]]] ]; // `OnceLock` for lazily-initialised statics (`DefineData` is not const-constructible). -// -// `DefineData` is not `Send`/`Sync` in general (it carries `ExprData`, whose -// boxed variants hold `NonNull<_>`). The four instances stored here only ever -// use the inline `EUndefined` / `ENumber` variants and `original_name = None`, -// so they contain no raw pointers and are trivially shareable. Wrap in a local -// newtype to satisfy `OnceLock` without globally relaxing -// `DefineData`'s auto-traits. -#[repr(transparent)] -struct SyncDefineData(defines::IdentifierDefine); -// SAFETY: only constructed below with pointer-free `ExprData` payloads -// (`EUndefined`/`ENumber`) and `original_name = None`; never mutated after -// `OnceLock` init. See note above. -unsafe impl Sync for SyncDefineData {} -// SAFETY: see `Sync` impl above. -unsafe impl Send for SyncDefineData {} - fn pure_global_identifier_define() -> &'static defines::IdentifierDefine { - static CELL: OnceLock = OnceLock::new(); - &CELL - .get_or_init(|| { - SyncDefineData(defines::DefineData::init(defines::Options { - value: ExprData::EUndefined(js_ast::E::Undefined), - valueless: true, - can_be_removed_if_unused: true, - ..Default::default() - })) + static CELL: OnceLock = OnceLock::new(); + CELL.get_or_init(|| { + defines::DefineData::init(defines::Options { + value: ExprData::EUndefined(js_ast::E::Undefined), + valueless: true, + can_be_removed_if_unused: true, + ..Default::default() }) - .0 + }) } mod identifiers { - use super::{ExprData, OnceLock, SyncDefineData, defines, js_ast}; + use super::{ExprData, OnceLock, defines, js_ast}; const NAN_VAL: js_ast::E::Number = js_ast::E::Number::new(f64::NAN); const INF_VAL: js_ast::E::Number = js_ast::E::Number::new(f64::INFINITY); // Step 2. Swap in certain literal values because those can be constant folded pub(super) fn undefined() -> &'static defines::IdentifierDefine { - static CELL: OnceLock = OnceLock::new(); - &CELL - .get_or_init(|| { - SyncDefineData(defines::DefineData::init(defines::Options { - value: ExprData::EUndefined(js_ast::E::Undefined), - can_be_removed_if_unused: true, - ..Default::default() - })) + static CELL: OnceLock = OnceLock::new(); + CELL.get_or_init(|| { + defines::DefineData::init(defines::Options { + value: ExprData::EUndefined(js_ast::E::Undefined), + can_be_removed_if_unused: true, + ..Default::default() }) - .0 + }) } pub(super) fn nan() -> &'static defines::IdentifierDefine { - static CELL: OnceLock = OnceLock::new(); - &CELL - .get_or_init(|| { - SyncDefineData(defines::DefineData::init(defines::Options { - value: ExprData::ENumber(NAN_VAL), - ..Default::default() - })) + static CELL: OnceLock = OnceLock::new(); + CELL.get_or_init(|| { + defines::DefineData::init(defines::Options { + value: ExprData::ENumber(NAN_VAL), + ..Default::default() }) - .0 + }) } pub(super) fn infinity() -> &'static defines::IdentifierDefine { - static CELL: OnceLock = OnceLock::new(); - &CELL - .get_or_init(|| { - SyncDefineData(defines::DefineData::init(defines::Options { - value: ExprData::ENumber(INF_VAL), - ..Default::default() - })) + static CELL: OnceLock = OnceLock::new(); + CELL.get_or_init(|| { + defines::DefineData::init(defines::Options { + value: ExprData::ENumber(INF_VAL), + ..Default::default() }) - .0 + }) } } diff --git a/src/js_parser/lib.rs b/src/js_parser/lib.rs index c8ab5aabcea0..066b2255935a 100644 --- a/src/js_parser/lib.rs +++ b/src/js_parser/lib.rs @@ -290,7 +290,7 @@ pub mod defines { /// (`E.CallUnwrap`, 2 bits), `method_call_must_be_replaced_with_undefined`. #[repr(transparent)] #[derive(Clone, Copy, Default, PartialEq, Eq)] - pub struct Flags(u8); + pub(crate) struct Flags(u8); impl Flags { const VALUELESS_SHIFT: u8 = 3; @@ -341,7 +341,7 @@ pub mod defines { self.0 = (self.0 & !(1 << Self::METHOD_CALL_UNDEF_SHIFT)) | ((v as u8) << Self::METHOD_CALL_UNDEF_SHIFT); } - pub fn new( + pub(crate) fn new( valueless: bool, can_be_removed_if_unused: bool, call_can_be_unwrapped_if_unused: E::CallUnwrap, @@ -360,17 +360,14 @@ pub mod defines { #[derive(Clone)] pub struct DefineData { - /// Nodes of the define table. Private: `value_for_use` is the only way - /// from here to an `Expr`, and it copies every node it returns. + /// A node of the define table. Only `value_for_use` turns it into an `Expr`, and it copies. value: ExprData, // The `RawDefines` value bytes are owned (`Box<[u8]>`), so borrowing // would be a use-after-free once the `RawDefines` map is dropped after // `Define::init`. Own the bytes here instead — these are tiny // startup-time copies. - // Kept `pub` so the bundler-side `parse`/`from_input` (which live a - // tier up for json-parser access) can construct directly. - pub original_name: Option>, - pub flags: Flags, + original_name: Option>, + flags: Flags, } // SAFETY: the nodes behind `value` are not written after init. They never @@ -454,7 +451,6 @@ pub mod defines { self.flags.valueless() } - /// The identifier this define names, for the parser to resolve. #[inline] pub(crate) fn identifier(&self) -> Option { match self.value { @@ -463,24 +459,27 @@ pub mod defines { } } - /// The value as one use of the define gets it. A string, an object or - /// an array is a copy, so no pass over the AST can write to the table. + /// What one use of the define gets: its own copy of a string, an object or an array. #[inline] pub(crate) fn value_for_use(&self, loc: bun_ast::Loc) -> Expr { - if let ExprData::EString(string) = self.value { - return Expr::init(&*string, loc); - } - let expr = Expr { + let table = Expr { data: self.value, loc, }; - if matches!(self.value, ExprData::EObject(_) | ExprData::EArray(_)) { - return expr.clone_literal_tree(); - } + let expr = if let ExprData::EString(string) = self.value { + Expr::init(&*string, loc) + } else if matches!(self.value, ExprData::EObject(_) | ExprData::EArray(_)) { + table.clone_literal_tree() + } else { + table + }; + debug_assert!( + !expr.shares_literal_node_with(&table), + "a use of a define got a node of the define table" + ); expr } - /// The string this define holds. #[inline] pub fn string(&self) -> Option<&E::EString> { match &self.value { diff --git a/src/js_parser/p.rs b/src/js_parser/p.rs index de9bc414b6b9..a83c4bc4083e 100644 --- a/src/js_parser/p.rs +++ b/src/js_parser/p.rs @@ -7112,8 +7112,6 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O is_delete_target: bool, define_data: &DefineData, ) -> Expr { - // Callers gate on `!valueless()` before reaching here, so `value` is a - // real Expr.Data by contract. match define_data.identifier() { Some(id) => { // Identifier defines always carry a name. Match the diff --git a/src/js_parser/visit/visit_expr.rs b/src/js_parser/visit/visit_expr.rs index 51d3454da40f..8b1d025d7ef6 100644 --- a/src/js_parser/visit/visit_expr.rs +++ b/src/js_parser/visit/visit_expr.rs @@ -525,8 +525,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O spread_object = Some(*inner); } let mut props: G::PropertyList = match spread_object { - // Other expressions can point at this object: each use of a - // `const` that holds a macro result does. Copy its list. + // Another expression can hold the spread object (a macro result), so copy its list. Some(object) => { let mut props = G::PropertyList::init_capacity( object.properties.len() + usize::from(!e_.children.is_empty()), diff --git a/src/jsc/RuntimeTranspilerCache.rs b/src/jsc/RuntimeTranspilerCache.rs index d0b00639db1f..03873592620f 100644 --- a/src/jsc/RuntimeTranspilerCache.rs +++ b/src/jsc/RuntimeTranspilerCache.rs @@ -67,8 +67,7 @@ bun_core::declare_scope!(cache, visible); /// Version 35: Long hexadecimal, octal and binary literals are rounded to the nearest double. /// Version 36: `export`, `require()`, `import()`, a JSX element, an exported async function and /// an abstract class are mapped elsewhere. -/// Version 37: A JSX spread no longer empties an object `--define` or an inlined macro -/// result. Older entries print `{}` for the uses that the module loader transpiled after it. +/// Version 37: A JSX spread no longer empties an object `--define`. Older entries can print it as `{}`. const EXPECTED_VERSION: u32 = 37; /// Source files smaller than this are not written to / read from the on-disk diff --git a/test/bake/dev/hot.test.ts b/test/bake/dev/hot.test.ts index 46ce4a3f690b..e1c4c48ad251 100644 --- a/test/bake/dev/hot.test.ts +++ b/test/bake/dev/hot.test.ts @@ -313,7 +313,7 @@ devTest("import.meta.hot.accept multiple modules", { await c.expectMessageInAnyOrder("Counter updated: 3", "Name updated: Charlie"); }, }); -// An array `define` is a literal at each use. `import.meta.hot.accept` replaces the items of its +// Every use of an array `define` gets the whole value. `import.meta.hot.accept` replaces the items of its // dependency list, and that must not change what the next use or the next parse gets for the define. devTest("import.meta.hot.accept with a dependency list from a define", { files: { diff --git a/test/bundler/bundler_jsx.test.ts b/test/bundler/bundler_jsx.test.ts index 887ce3749bb7..172671d6d3bc 100644 --- a/test/bundler/bundler_jsx.test.ts +++ b/test/bundler/bundler_jsx.test.ts @@ -527,7 +527,7 @@ describe("bundler", () => { }, }); - // An object `define` is a literal at each use. The automatic runtime passes a + // Every use of an object `define` gets the whole value. The automatic runtime passes a // lone spread object as the props, and that must not take the properties // away from the other uses of the define, in the same file or in another one. describe("a spread of an object define", () => { diff --git a/test/bundler/transpiler/transpiler.test.js b/test/bundler/transpiler/transpiler.test.js index 9acfafd8bd3d..7ce9d42c2b5d 100644 --- a/test/bundler/transpiler/transpiler.test.js +++ b/test/bundler/transpiler/transpiler.test.js @@ -2593,7 +2593,7 @@ export default <>hi expect(exitCode).toBe(0); }); - // Each use of an object or array define is a literal of its own. The automatic JSX runtime + // Every use of an object or array define gets the whole value. The automatic JSX runtime // passes a lone spread object as the props: `
` is `jsxDEV("div", { a: 1 })`. // That must not take the properties away from the other uses of the define. describe("define with an object or array value", () => { @@ -2648,19 +2648,22 @@ export const after = { a: 1 }; expect(transpiler.transformSync("export const b = OBJ;")).toBe("export const b = { a: 1 };\n"); }); - // transform() parses on a pool thread. Every call reads the one define table of the transpiler. + // transform() parses on a pool thread, and every call reads the one define table of the transpiler. + // When a call takes the properties of the table, the other calls print wrong output or crash. it("is whole in every concurrent transform()", async () => { await using proc = Bun.spawn({ cmd: [ bunExe(), "-e", ` - const object = Object.fromEntries(Array.from({ length: 200 }, (_, i) => ["k" + i, "v" + i])); - const transpiler = new Bun.Transpiler({ loader: "jsx", define: { OBJ: JSON.stringify(object) } }); - const source = "export const a =
kid
; export const b = OBJ;"; - const outputs = await Promise.all(Array.from({ length: 64 }, () => transpiler.transform(source))); - const count = text => text.split('"v199"').length - 1; - console.log(JSON.stringify({ distinct: new Set(outputs).size, uses: count(outputs[0]) })); + const transpiler = new Bun.Transpiler({ loader: "jsx", define: { OBJ: '{"a":1}' } }); + const source = Array.from( + { length: 200 }, + (_, i) => "export const c" + i + " =
kid
;\\nexport const p" + i + " = OBJ;", + ).join("\\n"); + const outputs = await Promise.all(Array.from({ length: 8 }, () => transpiler.transform(source))); + const uses = outputs[0].split("a: 1").length - 1; + console.log(JSON.stringify({ distinct: new Set(outputs).size, uses })); `, ], env: bunEnv, @@ -2668,10 +2671,11 @@ export const after = { a: 1 }; stderr: "pipe", }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({ - stdout: JSON.stringify({ distinct: 1, uses: 2 }), + expect({ stdout: stdout.trim(), stderr, exitCode, signalCode: proc.signalCode }).toEqual({ + stdout: JSON.stringify({ distinct: 1, uses: 400 }), stderr: "", exitCode: 0, + signalCode: null, }); }); }); From 3a1811e5ef187755185f5765f86d1f5f679182bb Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 18 Oct 2026 16:47:27 +0000 Subject: [PATCH 3/3] value_for_use: test for a string before it builds the table expression --- src/js_parser/lib.rs | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/js_parser/lib.rs b/src/js_parser/lib.rs index 066b2255935a..2225d11d6f7d 100644 --- a/src/js_parser/lib.rs +++ b/src/js_parser/lib.rs @@ -460,21 +460,19 @@ pub mod defines { } /// What one use of the define gets: its own copy of a string, an object or an array. - #[inline] pub(crate) fn value_for_use(&self, loc: bun_ast::Loc) -> Expr { - let table = Expr { - data: self.value, - loc, - }; - let expr = if let ExprData::EString(string) = self.value { - Expr::init(&*string, loc) - } else if matches!(self.value, ExprData::EObject(_) | ExprData::EArray(_)) { - table.clone_literal_tree() - } else { - table + let expr = match self.value { + ExprData::EString(string) => Expr::init(&*string, loc), + value @ (ExprData::EObject(_) | ExprData::EArray(_)) => { + Expr { data: value, loc }.clone_literal_tree() + } + value => Expr { data: value, loc }, }; debug_assert!( - !expr.shares_literal_node_with(&table), + !expr.shares_literal_node_with(&Expr { + data: self.value, + loc + }), "a use of a define got a node of the define table" ); expr