Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
12 changes: 9 additions & 3 deletions src/js_parser/lower/lower_decorators.rs
Original file line number Diff line number Diff line change
Expand Up @@ -720,25 +720,31 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool, const SEMA: bool>

// ── Public API ───────────────────────────────────────

/// `default_export` is the binding of an `export default class` statement,
/// the one class statement that can have no name. Such a class is named
/// "default", and takes the binding as its name only when class decorators
/// have to rebind it.
Comment on lines +723 to +726

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

pub(crate) fn lower_standard_decorators_stmt(
&mut self,
stmt: Stmt,
default_export: Option<js_ast::LocRef>,
out: &mut BumpVec<'a, Stmt>,
) {
let mut s_class = match stmt.data {
js_ast::StmtData::SClass(c) => c,
_ => unreachable!(),
};
let lowered = self.lower_class_body(&mut s_class.class, stmt.loc, None);
let name_from_context = default_export.map(|_| js_ast::ClauseItem::DEFAULT_ALIAS);
let lowered = self.lower_class_body(&mut s_class.class, stmt.loc, name_from_context);
out.extend(lowered.temps);
let Some(decorators) = lowered.class_decorators else {
out.push(stmt);
return;
};
let name = s_class
let name = *s_class
.class
.class_name
.expect("a class statement has a name");
.get_or_insert_with(|| default_export.expect("a class statement has a name"));
Comment on lines +744 to +747

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 (optional) Users who transpile (not bundle) export default @ dec class {} next to a local named <file>_default now get SyntaxError: Cannot declare a class twice where main loaded the module. When a class decorator needs a rebind, the anonymous class takes the default export's generated symbol as its declared name at src/js_parser/lower/lower_decorators.rs:747, and without a bundle no renamer guards that name against user bindings. Fix: rebind through a name that cannot collide (a fresh temp, or print the anonymous class as an expression assigned to a temp and export default that), covering @ dec export default class {} too, which already fails this way on main. The PR notes this under #44721 with two test.todo rows; the todo documents the cost but does not bound it.

Why this was flagged

Input: a JS or TS module test.js containing const test_default = 1; and export default @ dec class {} with standard decorators, run directly with bun test.js (no bundle). Parsing now takes the statement path (src/js_parser/parse/parse_stmt.rs:1591), and s_export_default calls lower_standard_decorators_stmt with Some(data.default_name) (src/js_parser/visit/visit_stmt.rs:840-843). Because the class has a class decorator, lowered.class_decorators is Some and get_or_insert_with at src/js_parser/lower/lower_decorators.rs:747 sets class_name to the generated <file>_default symbol; the printer then emits export default class test_default {} plus test_default = _default with no renaming pass. The engine rejects the module: SyntaxError: Cannot declare a class twice: 'test_default'. On main the same input parsed as a class expression and was lowered with temps only, so the module loaded and printed 1. The author lists this as a downside and marks it test.todo, which records the failure but leaves the user-facing load error in place.

Verification: Trigger: a module run without a bundle containing export default @ dec class {} together with a user binding whose name equals the generated default-export name. src/js_parser/lower/lower_decorators.rs:744-752 does class_name.get_or_insert_with(|| default_export.expect(..)) and emits class <name> {}. On bd599f5 the same input was parsed as a class expression, so the module loaded.

let decorated = self.use_ref(decorators.decorated, name.loc);
let rebind = self.assign_to(name.ref_, decorated, name.loc);
out.push(self.expr_stmt(decorators.evaluate, stmt.loc));
Expand Down
5 changes: 3 additions & 2 deletions src/js_parser/p.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7452,9 +7452,10 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool, const SEMA: bool>
// Standard decorator lowering path (for both JS and TS files)
if s_class.class.should_lower_standard_decorators {
// `lower_standard_decorators_stmt` takes an out-param Vec; wrap to
// keep this function's slice contract.
// keep this function's slice contract. `s_export_default` calls
// it itself, so a class that arrives here has a name.
Comment on lines +7455 to +7456

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

let mut out = BumpVec::<Stmt>::new_in(self.arena);
self.lower_standard_decorators_stmt(stmt, &mut out);
self.lower_standard_decorators_stmt(stmt, None, &mut out);
return out.into_bump_slice_mut();
}

Expand Down
20 changes: 16 additions & 4 deletions src/js_parser/parse/parse_stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1584,8 +1584,11 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool, const SEMA: bool>
));
}

// "@" is in the lookahead set that rules out an expression after
// "export default": a decorator here starts a class declaration.
Comment on lines +1587 to +1588

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

if p.lexer.token == T::TFunction
|| p.lexer.token == T::TClass
|| p.lexer.token == T::TAt
|| p.lexer.is_contextual_keyword(b"interface")
{
let mut _opts = ParseStatementOptions {
Expand All @@ -1594,6 +1597,8 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool, const SEMA: bool>
lexical_decl: LexicalDecl::AllowAll,
..Default::default()
};
let errors_before_decorators =
(p.lexer.token == T::TAt).then(|| p.log().errors);
let stmt = p.parse_stmt(&mut _opts)?;

let default_name: LocRef = 'default_name_getter: {
Expand Down Expand Up @@ -1623,11 +1628,18 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool, const SEMA: bool>
// declaration: the nested statement came back as an
// expression statement ("export default interface = 2",
// "export default interface => 1") or a labeled statement
// ("export default interface: 0"). None of these can be a
// default export value, so report a syntax error instead of
// building an S.ExportDefault that the visit and print
// passes don't support.
// ("export default interface: 0"). Decorators that no class
// follows end here too ("export default @dec abstract = 1").
// None of these can be a default export value, so report a
// syntax error instead of building an S.ExportDefault that the
// visit and print passes don't support.
Comment on lines +1631 to +1635

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

_ => {
// `t_at` has reported the token that is not a class.
if errors_before_decorators
.is_some_and(|errors| p.log().errors > errors)
{
return Err(crate::Error::SyntaxError);
}
let r =
js_lexer::range_of_identifier(p.source, p.real_loc(stmt.loc));
p.log().add_range_error_fmt(
Expand Down
59 changes: 47 additions & 12 deletions src/js_parser/visit/visit_stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -798,30 +798,32 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool, const SEMA: bool>
replace_expr,
) = entry
{
// The class is discarded. Lowering it would put it back
// in `data.value`.
Comment on lines +801 to +802

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

data.value = js_ast::StmtOrExpr::Expr(replace_expr);
stmts.push(*stmt);
} else {
let _ = p.inject_replacement_export(
stmts,
Ref::NONE,
bun_ast::Loc::EMPTY,
&entry,
);
restore_dead!();
record_on_exit!();
return Ok(());
}
restore_dead!();
record_on_exit!();
return Ok(());
}

if !data.default_name.ref_.is_symbol() {
data.default_name = p.create_default_name(stmt.loc);
}

// We only inject a name into classes when decorator lowering
// needs one: legacy TS decorators (`has_decorators`) or
// standard decorator lowering, which also covers classes with
// only auto-accessor fields and no decorators.
// The legacy TS decorator lowering reads `class_name`, so an
// anonymous class takes the default export's symbol as its name.
// The standard lowering takes that symbol as an argument.
Comment on lines +822 to +824

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

if class.class.has_decorators
|| class.class.should_lower_standard_decorators
&& !class.class.should_lower_standard_decorators
{
if class.class.class_name.is_none()
|| class.class.class_name.unwrap().ref_.is_empty()
Expand All @@ -833,7 +835,17 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool, const SEMA: bool>
// Lower the class (handles both TS legacy and standard decorators).
// Standard decorator lowering may produce prefix statements
// (variable declarations) before the class statement.
let class_stmts = p.lower_class(js_ast::StmtOrExpr::Stmt(s2_copy));
let class_stmts = if class.class.should_lower_standard_decorators {
let mut lowered: StmtList<'a> = BumpVec::new_in(p.arena);
p.lower_standard_decorators_stmt(
s2_copy,
Some(data.default_name),
&mut lowered,
);
lowered.into_bump_slice_mut()
} else {
p.lower_class(js_ast::StmtOrExpr::Stmt(s2_copy))
};

// Find the s_class statement in the returned list
let mut class_stmt_idx: usize = 0;
Expand All @@ -847,13 +859,36 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool, const SEMA: bool>
// Emit any prefix statements before the export default
stmts.extend_from_slice(&class_stmts[0..class_stmt_idx]);

let after_class = &class_stmts[class_stmt_idx + 1..];
if p.options.features.server_components.wraps_exports()
&& !after_class.is_empty()
{
// Decorator lowering assigns to the class by name after it, so
// the class stays a declaration and the export wraps its binding.
Comment on lines +866 to +867

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

let name = class
.class
.class_name
.expect("decorator lowering names the class");
stmts.push(class_stmts[class_stmt_idx]);
stmts.extend_from_slice(after_class);
p.record_usage(name.ref_);
data.value = js_ast::StmtOrExpr::Expr(
p.wrap_value_for_server_component_reference(
Expr::init_identifier(name.ref_, name.loc),
b"default",
),
);
stmts.push(*stmt);
restore_dead!();
record_on_exit!();
return Ok(());
Comment on lines +863 to +884

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Production Bake builds of a "use client" or "use server" module with a decorated export default class now emit a bundle that throws a SyntaxError on load; on main the bundle loaded. The new branch at src/js_parser/visit/visit_stmt.rs:872-881 keeps the class as a declaration named by class_name and sets data.value to registerClientReference(<same name>), while data.default_name is that same ref. src/bundler/linker_context/convertStmtsForChunk.rs:625-641 then prints var Df = ... next to class Df {}. Fix: give the export a symbol distinct from the class binding on this path (e.g. a fresh create_default_name for data.default_name before wrapping) for named, anonymous, legacy and standard cases.

Why this was flagged

Trigger: a Bake production build, a module under "use server" (src/bundler/ParseTask.rs:2588) or "use client" without a separate SSR graph (src/bundler/ParseTask.rs:2585), containing export default @ dec class Df {} or @ dec export default class Df {}. The new branch at src/js_parser/visit/visit_stmt.rs:863-884 pushes the class statement as a standalone declaration named class_name and sets data.value to an identifier wrapped by wrap_value_for_server_component_reference. data.default_name is the same ref as the class name: for a named class parse_stmt.rs:1620-1624 takes it from the class. The production linker rewrites the export default <expr> into var <default_name> = <expr> (convertStmtsForChunk.rs:625-641), so the chunk contains class Df {} and var Df = registerClientReference(Df, "default"), and a var sharing a name with a class declaration in the same scope is an early SyntaxError when the bundle is loaded. On main the class was wrapped as a class expression and the chunk loaded. The dev server does not hit this because InternalBakeDev uses convertStmtsForChunkForDevServer.

Verification: src/js_parser/visit/visit_stmt.rs:872-880 pushes the lowered class as a standalone declaration and sets data.value to registerClientReference(Identifier(name.ref_), "default"); default_name is the class's own ref (src/js_parser/parse/parse_stmt.rs:1619-1625). src/bundler/linker_context/convertStmtsForChunk.rs:625-641 rewrites it to var Df = registerClientReference(Df, "default") in the same scope as class Df {}, an early SyntaxError.

}

data.value = js_ast::StmtOrExpr::Stmt(class_stmts[class_stmt_idx]);
stmts.push(*stmt);

// Emit any suffix statements after the export default
if class_stmt_idx + 1 < class_stmts.len() {
stmts.extend_from_slice(&class_stmts[class_stmt_idx + 1..]);
}
stmts.extend_from_slice(after_class);

if p.options.features.server_components.wraps_exports() {
// `data.value` is mutated *after* pushing `stmt`; the pushed
Expand Down
4 changes: 3 additions & 1 deletion src/jsc/RuntimeTranspilerCache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,9 @@ bun_core::declare_scope!(cache, visible);
/// `onResolve` rewrote (`namespace:path`). Older entries request the bare path, and
/// the cache-HIT path reinstates #33904 for them.
/// Version 34: An import that a plugin `onResolve` answers is printed as it is written.
const EXPECTED_VERSION: u32 = 34;
/// Version 35: `export default @dec class` is a class declaration, and standard decorator
/// lowering names an anonymous `export default class` "default".
Comment on lines +67 to +68

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

const EXPECTED_VERSION: u32 = 35;

/// 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
32 changes: 32 additions & 0 deletions test/bake/dev/bundle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -940,3 +940,35 @@ devTest("a render() that does not return a Response is reported as that", {
}).toEqual({ status: 500, saysWhatIsWrong: true, referenceError: false });
},
});
// A "use client" module without a separate SSR graph exports `registerClientReference(value, ...)`.
// The statements that decorator lowering puts after a class assign to the class by name, so the
// class has to stay a declaration: as the wrapped value it left `Df = ...` with no binding
// ("ReferenceError: Df is not defined").
for (const spelling of [
"@dec export default class Df {}",
"export default @dec class Df {}",
"export default @dec class {}",
]) {
devTest(`a "use client" module keeps the binding of \`${spelling}\` (experimentalDecorators)`, {
framework: minimalFramework,
files: {
"tsconfig.json": JSON.stringify({ compilerOptions: { experimentalDecorators: true } }),
"client.ts": `
"use client";
function dec(cls: any) {
globalThis.decorated = (globalThis.decorated ?? 0) + 1;
}
${spelling}
`,
"routes/index.ts": `
import Client from "../client";
export default function (req, meta) {
return new Response(JSON.stringify([typeof Client.value, Client.uid, globalThis.decorated]));
}
`,
},
async test(dev) {
await dev.fetch("/").equals('["function","default",1]');
},
});
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 18 additions & 0 deletions test/bundler/transpiler/decorators.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
// @ts-nocheck
import { describe, expect, test } from "bun:test";
import { bunEnv, bunExe } from "harness";
import DecoratedAfterDefaultClass, {
decorated as decoratedAfterDefault,
binding as decoratedAfterDefaultBinding,
} from "./decorator-after-export-default-class-fixture";
import DecoratedAfterDefaultAnonClass, {
decorated as decoratedAfterDefaultAnon,
} from "./decorator-after-export-default-class-fixture-anon";
import DecoratedClass from "./decorator-export-default-class-fixture";
import DecoratedAnonClass from "./decorator-export-default-class-fixture-anon";

Expand Down Expand Up @@ -1020,6 +1027,17 @@ test("export default class works (anonymous name)", () => {
expect(new DecoratedAnonClass()["methoddecorated"]).toBe(true);
});

test("export default @decorator class Named runs the decorator and binds the name", () => {
expect(decoratedAfterDefault).toEqual(["DecoratedClass"]);
expect(decoratedAfterDefaultBinding).toBe(DecoratedAfterDefaultClass);
expect(new DecoratedAfterDefaultClass()["replaced"]).toBe(true);
});

test("export default @decorator class runs the decorator (anonymous name)", () => {
expect(decoratedAfterDefaultAnon).toEqual([DecoratedAfterDefaultAnonClass]);
expect(new DecoratedAfterDefaultAnonClass().method()).toBe(42);
});

test("field with supra-BMP string-literal key and initializer is assigned under the correct key", () => {
function dec(_t: any, _k: any) {}
class Foo {
Expand Down
Loading
Loading