Skip to content
Open
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
25 changes: 19 additions & 6 deletions src/ast/e.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1859,11 +1859,13 @@ impl EString {
}

pub fn eql_bytes(&self, other: &[u8]) -> bool {
if self.is_utf8() {
strings::eql_long(&self.data, other, true)
} else {
strings::utf16_eql_string(self.slice16(), other)
if !self.is_utf8() {
return strings::utf16_eql_string(self.slice16(), other);
}
if self.next.is_none() {
return strings::eql_long(&self.data, other, true);
}
self.eql8_rope(other)
}

pub fn eql_comptime(&self, value: &'static [u8]) -> bool {
Expand Down Expand Up @@ -1911,7 +1913,15 @@ impl EString {

/// Return UTF-8 bytes, transcoding if UTF-16.
/// The transcode allocates via the global arena then copies into `bump`.
///
/// Reads `data` only: a string that went through the visit pass may be a
/// rope (`fold_string_addition`), of which this is just the first segment.
/// Use `slice` (or `resolve_rope_if_needed` first) on those.
Comment thread
robobun marked this conversation as resolved.
Outdated
pub fn string<'b>(&self, bump: &'b Bump) -> Result<&'b [u8], AllocError> {
debug_assert!(
self.next.is_none(),
"EString::string() called on an unresolved rope; use slice()"
);
Comment thread
claude[bot] marked this conversation as resolved.
Outdated
if self.is_utf8() {
// `self.data` is arena-owned with the same lifetime as `bump`;
// StoreStr re-borrows under that contract.
Expand Down Expand Up @@ -2438,15 +2448,18 @@ impl Import {
self.import_record_index == u32::MAX
}

pub fn import_record_loader(&self) -> Option<crate::Loader> {
pub fn import_record_loader(&self, bump: &Bump) -> Option<crate::Loader> {
let crate::ExprData::EObject(obj) = &self.options.data else {
return None;
};
let with = Object::get(obj, b"with").or_else(|| Object::get(obj, b"assert"))?;
let crate::ExprData::EObject(with_obj) = &with.data else {
return None;
};
let str_ = Object::get(with_obj, b"type")?.data.as_e_string()?;
let mut str_ = Object::get(with_obj, b"type")?.data.as_e_string()?;
// The options object is visited with constant folding forced on, so
// `"js" + "on"` arrives here as a rope.
Comment thread
robobun marked this conversation as resolved.
Outdated
str_.resolve_rope_if_needed(bump);

if !str_.is_utf16 {
if let Some(loader) = crate::Loader::from_string(&str_.data) {
Expand Down
20 changes: 13 additions & 7 deletions src/ast/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -678,12 +678,14 @@ impl ArrayIterator {
// raw-ptr returns). Those drafts were dropped; only the methods without a live
// counterpart remain.
impl Expr {
/// Unlike `as_utf8_string_literal`, this accepts visited strings, which may
/// be ropes: the rope is flattened in place.
Comment thread
robobun marked this conversation as resolved.
Outdated
#[inline]
pub fn as_string_literal<'b>(&self, bump: &'b Bump) -> Option<&'b [u8]> {
let Data::EString(s) = &self.data else {
let Data::EString(mut s) = self.data else {
return None;
};
s.string(bump).ok()
Some(s.slice(bump))
}

/// `as_string_hash` for JSON-parsed trees (always UTF-8, no rope) where no
Expand Down Expand Up @@ -2304,12 +2306,16 @@ impl Data {
hasher.update(&e.value);
}
Data::EString(e) => {
// Only the *first* rope segment is hashed.
let current: &E::String = e;
if current.is_utf8() {
hasher.update(&current.data);
if e.is_utf8() {
// Hashing the segments back to back gives a folded
// `"a" + "b"` the same hash as `"ab"`.
Comment thread
robobun marked this conversation as resolved.
Outdated
let mut segment: Option<&E::String> = Some(e.get());
while let Some(current) = segment {
hasher.update(&current.data);
segment = current.next.as_deref();
}
} else {
hasher.update(bytemuck::cast_slice::<u16, u8>(current.slice16()));
hasher.update(bytemuck::cast_slice::<u16, u8>(e.slice16()));
}
hasher.update(b"\x00");
}
Expand Down
27 changes: 14 additions & 13 deletions src/js_parser/p.rs
Original file line number Diff line number Diff line change
Expand Up @@ -801,20 +801,24 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
arg: Expr,
buf: &'b mut BumpVec<'a, u8>,
) -> Result<&'b [u8], crate::Error> {
if let Some(tmpl) = arg.data.e_template() {
if let Some(mut tmpl) = arg.data.e_template() {
if tmpl.tag.is_some() {
return Ok(b""); // tagged template — opaque
}
match &tmpl.head {
// After folding "./fo" + `o/${x}` the head (or a tail) is a rope;
// `string` alone would give only its first segment.
Comment thread
robobun marked this conversation as resolved.
Outdated
match &mut tmpl.head {
js_ast::e::TemplateContents::Cooked(head) => {
head.resolve_rope_if_needed(self.arena);
buf.extend_from_slice(head.string(self.arena)?);
}
js_ast::e::TemplateContents::Raw(_) => return Ok(b""), // shouldn't happen post-visit but be safe
}
for part in tmpl.parts().iter() {
for part in tmpl.parts_mut().iter_mut() {
buf.push(0); // \x00 placeholder per interpolation
match &part.tail {
match &mut part.tail {
js_ast::e::TemplateContents::Cooked(tail) => {
tail.resolve_rope_if_needed(self.arena);
buf.extend_from_slice(tail.string(self.arena)?);
}
js_ast::e::TemplateContents::Raw(_) => return Ok(b""), // raw tail — treat as opaque
Expand Down Expand Up @@ -1080,15 +1084,11 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
return self.new_expr(E::Null {}, arg.loc);
}

let import_record_index = self.add_import_record(
ImportKind::RequireResolve,
arg.loc,
arg.data
.e_string()
.expect("infallible: variant checked")
.string(self.arena)
.expect("unreachable"),
);
// `slice` flattens the rope left behind by folding "./fo" + "o";
// `string` would record only its first segment.
Comment thread
robobun marked this conversation as resolved.
Outdated
let mut str_ = arg.data.e_string().expect("infallible: variant checked");
let import_record_index =
self.add_import_record(ImportKind::RequireResolve, arg.loc, str_.slice(self.arena));
self.import_records.items_mut()[import_record_index as usize]
.flags
.set(
Expand Down Expand Up @@ -7541,6 +7541,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
str_: &mut E::String,
loc: bun_ast::Loc,
) -> Option<js_ast::ExprData> {
str_.resolve_rope_if_needed(self.arena);
let _ = str_.to_utf8(self.arena);
let specifier = str_.data;

Expand Down
11 changes: 8 additions & 3 deletions src/js_parser/visit/visit_expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1003,6 +1003,9 @@
let unwrapped = e_.index.unwrap_inlined();
if let Some(mut s) = unwrapped.data.e_string() {
if !s.is_utf16 {
// Both uses of `s.data` below need the whole string,
// not the first segment of a folded `'fo' + 'o'`.
Comment thread
robobun marked this conversation as resolved.
Outdated
s.resolve_rope_if_needed(p.arena);

Check warning on line 1008 in src/js_parser/visit/visit_expr.rs

View check run for this annotation

Claude / Claude Code Review

Post-visit .data sweep missed decorator_class_name in e_object

The follow-up sweep (1563476b) fixed the four post-visit `.data` readers the earlier review named, but one same-class site remains: `visit_expr.rs:1729-1736` in `e_object`, where `p.decorator_class_name` is read from `key_str.data.slice()` after the key was visited at line 1662, with no `!IsComputed` guard (unlike the `__proto__` check at 1670 or the class-body sibling at `visit/mod.rs:927`). Under `minify_syntax`, `{ ["Fo" + "o"]: @dec class {} }` folds to a rope whose `.data` is only `"Fo"`, s
Comment thread
robobun marked this conversation as resolved.
Outdated
// "a['b' + '']" => "a.b"
// "enum A { B = 'b' }; a[A.B]" => "a.b"
if p.options.features.minify_syntax && s.is_identifier(p.arena) {
Expand Down Expand Up @@ -1827,7 +1830,7 @@
),
import_options: e_.options,
loc: e_.expr.loc,
import_loader: e_.import_record_loader(),
import_loader: e_.import_record_loader(p.arena),

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

View check run for this annotation

Claude / Claude Code Review

e_import save/restore of the fold flag is hardcoded to true

Pre-existing (from d4ccab46, not this PR): the save side of the fold-flag save/restore at line 1809 is `let prev_should_fold_typescript_constant_expressions = true;` — a hardcoded `true`, not `p.should_fold_typescript_constant_expressions` (compare the correct pattern in `e_call` at 1947-1948). The "restore" at 1837-1838 / 1842-1843 therefore leaves the flag `true` after every `import(...)`, so `fold_string_addition` (and thus ropes) run for every expression visited after the first dynamic impor
Comment thread
robobun marked this conversation as resolved.
..Default::default()
};

Expand Down Expand Up @@ -2380,9 +2383,11 @@
}

// Check if the feature flag is enabled
// Use the underlying string data directly without allocation.
// Feature flag names should be ASCII identifiers, so UTF-16 is unexpected.
let flag_string = arg.data.e_string().expect("infallible: variant checked");
// The argument has already been visited, so under minify_syntax
// `"fo" + "o"` arrives as a rope whose `data` is only "fo".
Comment thread
robobun marked this conversation as resolved.
Outdated
let mut flag_string = arg.data.e_string().expect("infallible: variant checked");
flag_string.resolve_rope_if_needed(p.arena);
if flag_string.is_utf16 {
p.log().add_error(
Some(p.source),
Expand Down
29 changes: 29 additions & 0 deletions test/bundler/bun-build-api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,35 @@ describe("Bun.build", () => {
expect(await bunRun(build.outputs[0].path)).toSpawn("world");
});

test("reactFastRefresh signature hashes the whole folded string literal", async () => {
// minify.syntax folds "a" + "b" into a rope; the hook signature must cover
// every segment, otherwise "a" + "b" and "a" + "c" get the same signature.
using dir = tempDir("bun-build-api-refresh-sig", {
"ab.tsx": `import { useState } from "react"; export function C() { const [v] = useState("ab"); return <b>{v}</b>; }`,
"a-b.tsx": `import { useState } from "react"; export function C() { const [v] = useState("a" + "b"); return <b>{v}</b>; }`,
"a-c.tsx": `import { useState } from "react"; export function C() { const [v] = useState("a" + "c"); return <b>{v}</b>; }`,
});
const signatureOf = async (file: string) => {
const build = await Bun.build({
entrypoints: [join(String(dir), file)],
reactFastRefresh: true,
minify: { syntax: true },
external: ["react"],
});
const output = await build.outputs[0].text();
const match = output.match(/_s\w*\(C, "([^"]+)"\)/);
if (!match) throw new Error(`no refresh signature in ${file}:\n${output}`);
return match[1];
};
const [ab, aPlusB, aPlusC] = await Promise.all([
signatureOf("ab.tsx"),
signatureOf("a-b.tsx"),
signatureOf("a-c.tsx"),
]);
expect(aPlusB).toBe(ab);
expect(aPlusC).not.toBe(ab);
});

test("passing undefined doesnt segfault", () => {
try {
// @ts-ignore
Expand Down
51 changes: 51 additions & 0 deletions test/bundler/bundler_allow_unresolved.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,4 +226,55 @@ describe("bundler", () => {
backend: "cli",
allowUnresolved: ["./locales/*.json"],
});

// 17-20. Constant folding turns the template head/tail into a rope of string
// segments. The shape must include every segment ("./locales/*.json"), not
// just the first one ("./loc*.json"), or a matching pattern is rejected.
itBundled("allow-unresolved/RopeHeadFromStringAddition", {
files: {
"/entry.js": /* js */ `
export function load(x) {
return import("./loc" + \`ales/\${x}.json\`);
}
`,
},
outdir: "/out",
allowUnresolved: ["./locales/*.json"],
});

itBundled("allow-unresolved/RopeHeadFromTemplateFolding", {
files: {
"/entry.js": /* js */ `
export function load(x) {
return import(\`./loc\${"ales"}/\${x}.json\`);
}
`,
},
outdir: "/out",
allowUnresolved: ["./locales/*.json"],
});

itBundled("allow-unresolved/RopeTailFromStringAddition", {
files: {
"/entry.js": /* js */ `
export function load(x) {
return import(\`./locales/\${x}\` + ".json");
}
`,
},
outdir: "/out",
allowUnresolved: ["./locales/*.json"],
});

itBundled("allow-unresolved/RequireResolveRopeHead", {
files: {
"/entry.js": /* js */ `
export function load(x) {
return require.resolve("./loc" + \`ales/\${x}.json\`);
}
`,
},
outdir: "/out",
allowUnresolved: ["./locales/*.json"],
});
});
18 changes: 18 additions & 0 deletions test/bundler/bundler_edgecase.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2913,6 +2913,24 @@ describe("bundler", () => {
},
run: { stdout: "true 1" },
});
// require() arguments are constant-folded even without minifySyntax, so the
// index here is a rope ("fo" -> "o"). Rewriting ns[...] to an import binding
// has to use the whole name: it used to bind the `fo` export instead of `foo`.
itBundled("edgecase/FoldedStringIndexOnNamespaceImport", {
files: {
"/entry.js": /* js */ `
import * as ns from "./ns.js";
console.log(require(ns["fo" + "o"] === "yes" ? "./yes.js" : "./no.js"));
`,
"/ns.js": /* js */ `
export const foo = "yes";
export const fo = "no";
`,
"/yes.js": `module.exports = "took yes";`,
"/no.js": `module.exports = "took no";`,
},
run: { stdout: "took yes" },
});
// The bundler rewrites bare `require`/`require.main`/`require.resolve` to an
// ERequireCallTarget / ERequireMain / ERequireResolveCallTarget that prints
// as `__require` / `__require.main` / `__require.resolve`.
Expand Down
22 changes: 22 additions & 0 deletions test/bundler/bundler_feature_flag.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,28 @@ if (feature("DISABLED_FEATURE")) {
},
});

// minifySyntax folds "ENABLED_" + "FEATURE" before feature() sees it; the
// lookup must use the whole folded name, not its first segment.
itBundled(`feature_flag/${backend}/FoldedFlagName`, {
backend,
files: {
"/a.js": `
import { feature } from "bun:bundle";
if (feature("ENABLED_" + "FEATURE")) {
console.log("this should be kept");
} else {
console.log("this should be removed");
}
`,
},
features: ["ENABLED_FEATURE"],
minifySyntax: true,
onAfterBundle(api) {
api.expectFile("out.js").toInclude("this should be kept");
api.expectFile("out.js").not.toInclude("this should be removed");
},
});

itBundled(`feature_flag/${backend}/ImportRemoved`, {
backend,
files: {
Expand Down
26 changes: 26 additions & 0 deletions test/bundler/bundler_loader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,32 @@ describe("bundler", async () => {
});
}

// import() options are visited with constant folding forced on, so these
// attribute strings reach the loader lookup as ropes; the lookup must see the
// whole string ("json"), not just its first segment ("js" is a real loader).
itBundled("bun/loader-dynamic-import-attribute-folded-type", {
target: "bun",
files: {
"/entry.ts": /* js */ `
const mod = await import('./hello.notjson', { with: { type: "js" + "on" } });
console.write(JSON.stringify(mod.default));
`,
"/hello.notjson": JSON.stringify({ hello: "world" }),
},
run: { stdout: '{"hello":"world"}' },
});
itBundled("bun/loader-dynamic-import-attribute-folded-keys", {
target: "bun",
files: {
"/entry.ts": /* js */ `
const mod = await import('./hello.notjson', { ["wi" + "th"]: { ["ty" + "pe"]: "json" } });
console.write(JSON.stringify(mod.default));
`,
"/hello.notjson": JSON.stringify({ hello: "world" }),
},
run: { stdout: '{"hello":"world"}' },
});

itBundled("bun/loader-text-file", {
target: "bun",
outfile: "",
Expand Down
24 changes: 24 additions & 0 deletions test/bundler/transpiler/macro-test.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,30 @@ test("object destructuring of a macro result keeps every bound property regardle
expect(exitCode).toBe(0);
});

test("object destructuring of a macro result matches a computed key built from folded string literals", async () => {
// ["a" + "b"] is folded into a rope; matching it against the macro result must use the whole
// string, or the `ab` property is dropped from the inlined object.
using dir = tempDir("macro-destructure-folded-key", {
"m.ts": `export function m() {\n return { ab: 1, c: 2 };\n}\n`,
"index.ts": [
`import { m } from "./m.ts" with { type: "macro" };`,
`const { ["a" + "b"]: x, c } = m();`,
`console.log(JSON.stringify([x, c]));`,
``,
].join("\n"),
});
await using proc = Bun.spawn({
cmd: [bunExe(), "run", "index.ts"],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect({ lastLine: stdout.trim().split("\n").pop(), stderr }).toEqual({ lastLine: "[1,2]", stderr: "" });
expect(exitCode).toBe(0);
});

describe("--no-macros", () => {
const files = {
"macro.ts": `
Expand Down
Loading
Loading