Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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
43 changes: 35 additions & 8 deletions src/js_parser/parse/parse_entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1052,10 +1052,19 @@
}

js_ast::StmtData::SClass(class) => {
// Move class export statements to the top of the file if we can
// This automatically resolves some cyclical import issues
// https://github.com/kysely-org/kysely/issues/412
let should_move = !p.options.bundle && class.class.can_be_moved();
// Move class statements ahead of other code to help cyclical imports
// (https://github.com/kysely-org/kysely/issues/412). Skip when an
// earlier statement already references the name so its TDZ is kept.
let used_before_decl = match class.class.class_name {
Some(name) => {
p.symbols.as_slice()[name.ref_.inner_index() as usize]
.use_count_estimate
> 0
}
None => false,
};
let should_move =
!p.options.bundle && !used_before_decl && class.class.can_be_moved();

Check warning on line 1067 in src/js_parser/parse/parse_entry.rs

View check run for this annotation

Claude / Claude Code Review

use_count_estimate guard counts references in non-executing scopes, may over-suppress the cyclic-import hoist

The `use_count_estimate` check is broader than the description implies: `append_part()` walks into function bodies, so `function make() { return new K(); }` textually before `class K {}` bumps K's count and suppresses the hoist even though there's no runtime TDZ hazard — and the enum-preprocessing loop at ~L979-1001 visits every top-level enum *before* this loop, so a *later* `enum E { X = K.name.length }` does the same. Not a correctness issue (declining to hoist is always spec-safe, and arguab
Comment thread
robobun marked this conversation as resolved.

let sliced = arena.alloc_slice_copy(&[*stmt]);
p.append_part(&mut parts, sliced)?;
Expand All @@ -1066,10 +1075,28 @@
}
}
js_ast::StmtData::SExportDefault(value) => {
// We move export default statements when we can
// This automatically resolves some cyclical import issues in packages like luxon
// https://github.com/oven-sh/bun/issues/1961
let should_move = !p.options.bundle && value.can_be_moved();
// Move export default ahead of other code to help cyclical imports
// (e.g. luxon, #1961). Like `SClass` above, keep a named default
// class in place if an earlier statement already references it.
let class_name_ref = match &value.value {
js_ast::StmtOrExpr::Stmt(s) => match &s.data {
js_ast::StmtData::SClass(c) => {
c.class.class_name.map(|name| name.ref_)
}
_ => None,
},
// Class-expression names are not visible at module scope.
js_ast::StmtOrExpr::Expr(_) => None,
};
let used_before_decl = match class_name_ref {
Some(ref_) => {
p.symbols.as_slice()[ref_.inner_index() as usize].use_count_estimate
> 0
}
None => false,
};
let should_move =
!p.options.bundle && !used_before_decl && value.can_be_moved();
let sliced = arena.alloc_slice_copy(&[*stmt]);
p.append_part(&mut parts, sliced)?;

Expand Down
74 changes: 74 additions & 0 deletions test/bundler/transpiler/runtime-transpiler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,3 +252,77 @@ describe("unterminated string literals in large files", () => {
expect(exitCode).toBe(1);
});
});

// The runtime transpiler hoists top-level class declarations to help cyclic
// imports. That hoist must not jump over an earlier reference to the class
// binding, or the temporal dead zone disappears.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
describe("top-level class declaration TDZ", () => {
const tdzFixture = (decl: string) => /* js */ `
const out = [];
try {
out.push("typeof=" + typeof K);
out.push("constructed=" + new K().constructor.name);
} catch (e) {
out.push("ERR=" + e.constructor.name);
}
${decl}
out.push("after=" + typeof K);
console.log(out.join(" | "));
`;

async function run(files: Record<string, string>, entry: string) {
using dir = tempDir("runtime-transpiler-class-tdz", files);
await using proc = Bun.spawn({
cmd: [bunExe(), entry],
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.each([
["class K { m() {} }", "class declaration"],
["export class K { m() {} }", "exported class declaration"],
["export default class K { m() {} }", "export default named class"],
])("preserved for a %s referenced before its declaration (%s)", async decl => {
const { stdout, stderr, exitCode } = await run({ "index.mjs": tdzFixture(decl) }, "index.mjs");
expect(stderr).toBe("");
expect(stdout.trim()).toBe("ERR=ReferenceError | after=function");
expect(exitCode).toBe(0);
Comment thread
robobun marked this conversation as resolved.
Outdated
});

test.concurrent("preserved for a CommonJS top-level class declaration", async () => {
const { stdout, stderr, exitCode } = await run({ "index.cjs": tdzFixture("class K { m() {} }") }, "index.cjs");
expect(stderr).toBe("");
expect(stdout.trim()).toBe("ERR=ReferenceError | after=function");
expect(exitCode).toBe(0);
});

// Classes with no earlier reference are still hoisted, so the cyclic-import
// workaround itself stays in place.
test.concurrent("still hoisted when no earlier statement references it", async () => {
using dir = tempDir("runtime-transpiler-class-hoist", {
"index.mjs": /* js */ `
const unrelated = 1;
export class A { m() { return "A"; } }
console.log(new A().m(), unrelated);
`,
});
await using proc = Bun.spawn({
cmd: [bunExe(), "build", "--no-bundle", "--target=bun", "index.mjs"],
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(stderr).toBe("");
// The class declaration was moved ahead of `const unrelated`.
expect(stdout.indexOf("class A")).toBeGreaterThanOrEqual(0);
expect(stdout.indexOf("class A")).toBeLessThan(stdout.indexOf("unrelated = 1"));
expect(exitCode).toBe(0);
});
});
Loading