diff --git a/src/bundler/ParseTask.rs b/src/bundler/ParseTask.rs index b9ade1d446c0..5a9f28930bc4 100644 --- a/src/bundler/ParseTask.rs +++ b/src/bundler/ParseTask.rs @@ -2483,7 +2483,7 @@ pub mod parse_worker { }; opts.features.minify_syntax = topts.minify_syntax; opts.features.minify_identifiers = topts.minify_identifiers; - opts.features.minify_keep_names = topts.keep_names; + opts.features.minify_keep_names = topts.keep_names && !task.source_index.is_runtime(); opts.features.minify_whitespace = topts.minify_whitespace; opts.use_define_for_class_fields = task.use_define_for_class_fields; opts.features.emit_decorator_metadata = task.emit_decorator_metadata; diff --git a/src/bundler/transpiler.rs b/src/bundler/transpiler.rs index d9a670faaaf4..3dd08f0e9e04 100644 --- a/src/bundler/transpiler.rs +++ b/src/bundler/transpiler.rs @@ -1631,6 +1631,7 @@ impl<'a> Transpiler<'a> { opts.features.inject_jest_globals = this_parse.inject_jest_globals; opts.features.minify_syntax = self.options.minify_syntax; opts.features.minify_identifiers = self.options.minify_identifiers; + opts.features.minify_keep_names = self.options.keep_names; opts.features.dead_code_elimination = self.options.dead_code_elimination; opts.features.remove_cjs_module_wrapper = this_parse.remove_cjs_module_wrapper; // `Features.bundler_feature_flags` is currently owned diff --git a/src/js_parser/p.rs b/src/js_parser/p.rs index 285111d1dc80..665bb6054b0a 100644 --- a/src/js_parser/p.rs +++ b/src/js_parser/p.rs @@ -5254,8 +5254,82 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O } } - pub(crate) fn keep_expr_symbol_name(&mut self, _value: Expr, _name: &[u8]) -> Expr { - _value + pub(crate) fn keep_expr_symbol_name(&mut self, value: Expr, name: &'a [u8]) -> Expr { + if !self.options.features.minify_keep_names { + return value; + } + if let Some(mut class) = value.data.e_class() { + self.keep_class_symbol_name(&mut class, name, value.loc); + return value; + } + let loc = value.loc; + let target = self.runtime_identifier(loc, b"__name"); + let name_expr = self.new_expr(E::String::init(name), loc); + let args = self.arena.alloc_slice_copy(&[value, name_expr]); + self.new_expr( + E::Call { + target, + args: ExprNodeList::from_arena_slice(args), + can_be_unwrapped_if_unused: E::CallUnwrap::IfUnused, + ..Default::default() + }, + loc, + ) + } + + /// `__name(foo, "foo");` emitted after a function declaration. + pub(crate) fn keep_stmt_symbol_name( + &mut self, + loc: bun_ast::Loc, + ref_: Ref, + name: &'a [u8], + ) -> Stmt { + let name_expr = self.new_expr(E::String::init(name), loc); + let args = self + .arena + .alloc_slice_copy(&[Expr::init_identifier(ref_, loc), name_expr]); + let value = self.call_runtime(loc, b"__name", ExprNodeList::from_arena_slice(args)); + self.s( + S::SExpr { + value, + does_not_affect_tree_shaking: true, + }, + loc, + ) + } + + /// Prepends `static { __name(this, "Foo"); }` so `.name` survives identifier renaming. + pub(crate) fn keep_class_symbol_name( + &mut self, + class: &mut G::Class, + name: &'a [u8], + loc: bun_ast::Loc, + ) { + let this = self.new_expr(E::This, loc); + let name_expr = self.new_expr(E::String::init(name), loc); + let args = self.arena.alloc_slice_copy(&[this, name_expr]); + let call = self.call_runtime(loc, b"__name", ExprNodeList::from_arena_slice(args)); + let stmt = self.s( + S::SExpr { + value: call, + does_not_affect_tree_shaking: true, + }, + loc, + ); + let stmts = + bun_alloc::AstVec::::from_arena_slice(self.arena.alloc_slice_copy(&[stmt])); + let block = self.arena.alloc(G::ClassStaticBlock { loc, stmts }); + let mut properties = BumpVec::with_capacity_in(class.properties.len() + 1, self.arena); + properties.push(G::Property { + kind: js_ast::g::PropertyKind::ClassStaticBlock, + class_static_block: Some(js_ast::StoreRef::from_bump(block)), + ..Default::default() + }); + for prop in class.properties.slice() { + // SAFETY: arena-resident AST nodes never run `Drop`, so the shallow copy cannot double free. + properties.push(unsafe { core::ptr::read(prop) }); + } + class.properties = bun_ast::StoreSlice::new_mut(properties.into_bump_slice_mut()); } pub(crate) fn is_simple_parameter_list(args: &[G::Arg], has_rest_arg: bool) -> bool { diff --git a/src/js_parser/visit/mod.rs b/src/js_parser/visit/mod.rs index 53786e4db37d..43f8923e1917 100644 --- a/src/js_parser/visit/mod.rs +++ b/src/js_parser/visit/mod.rs @@ -792,6 +792,19 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O if let Some(name) = class.class_name { self.record_declared_symbol(name.ref_); } + let name_to_keep: Option<&'a [u8]> = if !self.options.features.minify_keep_names { + None + } else if let Some(name) = class.class_name { + Some( + self.symbols[name.ref_.inner_index() as usize] + .original_name + .slice(), + ) + } else if !default_name_ref.is_empty() { + Some(js_ast::ClauseItem::DEFAULT_ALIAS) + } else { + None + }; self.push_scope_for_visit_pass(ScopeKind::ClassName, name_scope_loc) .expect("unreachable"); @@ -1254,6 +1267,10 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O // class name scope self.pop_scope(); + if let Some(name) = name_to_keep { + self.keep_class_symbol_name(class, name, class.body_loc); + } + shadow_ref } diff --git a/src/js_parser/visit/visit_expr.rs b/src/js_parser/visit/visit_expr.rs index c3aa10f99ae6..e9b458739d39 100644 --- a/src/js_parser/visit/visit_expr.rs +++ b/src/js_parser/visit/visit_expr.rs @@ -1705,6 +1705,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O } if let Some(value) = &mut property.value { + let was_anonymous_named_expr = value.is_anonymous_named(); // Propagate name from property key for decorated anonymous class expressions // e.g., { Foo: @dec class {} } should give the class .name = "Foo" if in_.assign_target == js_ast::AssignTarget::None @@ -1740,6 +1741,18 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O }, ); p.decorator_class_name = None; + + if was_anonymous_named_expr + && p.options.features.minify_keep_names + && property.kind == bun_ast::g::PropertyKind::Normal + && !property.flags.contains(Flags::Property::IsMethod) + && !property.flags.contains(Flags::Property::IsComputed) + && let Some(key) = property.key + && let Some(key_str) = key.data.e_string() + && !key_str.is_utf16 + { + *value = p.keep_expr_symbol_name(*value, key_str.data.slice()); + } } if property.initializer.is_some() { @@ -2561,11 +2574,18 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O // Restore now so the stack-local pointer never escapes this frame. p.react_refresh.hook_ctx_storage = prev_hook_ctx; + let name_to_keep: Option<&'a [u8]> = match e_.func.name { + Some(name) if p.options.features.minify_keep_names && !name.ref_.is_empty() => Some( + p.symbols[name.ref_.inner_index() as usize] + .original_name + .slice(), + ), + _ => None, + }; + // Remove unused function names when minifying (only when bundling is enabled) - // unless --keep-names is specified if p.options.features.minify_syntax && p.options.bundle - && !p.options.features.minify_keep_names // SAFETY: current_scope is a live arena ptr while the parser exists. && !p.current_scope().contains_direct_eval && e_.func.name.is_some() @@ -2592,14 +2612,8 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O } } - if let Some(name) = e_.func.name { - final_expr = p.keep_expr_symbol_name( - final_expr, - // SAFETY: original_name is arena-owned, valid for 'a. - p.symbols[name.ref_.inner_index() as usize] - .original_name - .slice(), - ); + if let Some(name) = name_to_keep { + final_expr = p.keep_expr_symbol_name(final_expr, name); replaced = true; } @@ -2631,10 +2645,8 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O } // Remove unused class names when minifying (only when bundling is enabled) - // unless --keep-names is specified if p.options.features.minify_syntax && p.options.bundle - && !p.options.features.minify_keep_names // SAFETY: current_scope is a live arena ptr while the parser exists. && !p.current_scope().contains_direct_eval && e_.class_name.is_some() diff --git a/src/js_parser/visit/visit_stmt.rs b/src/js_parser/visit/visit_stmt.rs index 2b3cd18271bc..0c48bb7c4b92 100644 --- a/src/js_parser/visit/visit_stmt.rs +++ b/src/js_parser/visit/visit_stmt.rs @@ -764,6 +764,12 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O } stmts.push(*stmt); + if p.options.features.minify_keep_names + && matches!(data.value, js_ast::StmtOrExpr::Stmt(_)) + && let Some(fname) = func_name + { + stmts.push(p.keep_stmt_symbol_name(fname.loc, fname.ref_, name)); + } } p.react_refresh.hook_ctx_storage = prev; @@ -938,6 +944,9 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O ), Expr::init_identifier(func_name.ref_, func_name.loc), )); + if p.options.features.minify_keep_names { + stmts.push(p.keep_stmt_symbol_name(func_name.loc, func_name.ref_, original_name)); + } } else if !mark_as_dead { if remove_overwritten { // restore on early return. @@ -977,6 +986,10 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O )); } else { stmts.push(*stmt); + if p.options.features.minify_keep_names { + let name = data.func.name.expect("infallible: name checked"); + stmts.push(p.keep_stmt_symbol_name(name.loc, name.ref_, original_name)); + } } } else if mark_as_dead { if let Some(replacement) = p diff --git a/test/bundler/bundler_minify.test.ts b/test/bundler/bundler_minify.test.ts index 0f49f32e1795..05d428ff8f7f 100644 --- a/test/bundler/bundler_minify.test.ts +++ b/test/bundler/bundler_minify.test.ts @@ -107,57 +107,87 @@ describe("bundler", () => { itBundled("minify/KeepNamesPreservesNames", { files: { "/entry.js": /* js */ ` - export var AB = function A() { }; - export var CD = function B() { return 1; }; - export var EF = function C() { C(); }; - export var GH = function() { }; - export var IJ = class D { }; - export var KL = class E { constructor() {} }; - export var MN = class F { method() { return F; } }; - export var OP = class { }; + var AB = function A() { }; + var CD = function B() { return 1; }; + var EF = function C() { C(); }; + var GH = function() { }; + var IJ = class D { }; + var KL = class E { constructor() {} }; + var MN = class F { method() { return F; } }; + var OP = class { }; + var QR = () => {}; + var ST = { a: () => {}, b: function() {}, c: class {}, d() {} }; + function UV() {} + class WX { static self = this; } + console.log([AB, CD, EF, GH, IJ, KL, MN, OP, QR, ST.a, ST.b, ST.c, ST.d, UV, WX].map(x => x.name).join(" ")); `, }, - onAfterBundle(api) { - const code = api.readFile("/out.js"); - // With keepNames, all names should be preserved even when minifying - expect(code).toContain("function A()"); - expect(code).toContain("function B()"); - expect(code).toContain("function C()"); - expect(code).toContain("class D"); - expect(code).toContain("class E"); - expect(code).toContain("class F"); - // Anonymous functions/classes stay anonymous - expect(code).toMatch(/\w+ = function\(\) \{\}/); // GH stays anonymous - expect(code).toMatch(/\w+ = class \{\s*\}/); // OP stays anonymous - }, + run: { stdout: "A B C GH D E F OP QR a b c d UV WX" }, minifySyntax: true, - minifyIdentifiers: false, // Don't minify identifiers to make testing easier + minifyIdentifiers: true, keepNames: true, target: "bun", }); - itBundled("minify/KeepNamesWithMinifyIdentifiers", { + itBundled("minify/KeepNamesClassErrorSubclass", { files: { "/entry.js": /* js */ ` - export var AB = function A() { }; - export var CD = function B() { return 1; }; - export var EF = class C { }; + class BaseError extends Error { + constructor(message) { + super(message); + this.name = this.constructor.name; + } + } + class AbortError extends BaseError {} + export default class extends BaseError {} + const err = new AbortError("x"); + console.log(err.name, err instanceof AbortError, AbortError.name, BaseError.name); `, }, - onAfterBundle(api) { - const code = api.readFile("/out.js"); - // With keepNames + minifyIdentifiers, names are preserved but minified - // The original names A, B, C should still exist (though minified) - expect(code).toMatch(/function \w+\(\)/); // Functions should have names - expect(code).toMatch(/class \w+/); // Classes should have names - // Should not have anonymous functions/classes - expect(code).not.toContain("function()"); - expect(code).not.toContain("class {"); + run: { stdout: "AbortError true AbortError BaseError" }, + minifySyntax: true, + minifyIdentifiers: true, + keepNames: true, + target: "bun", + }); + itBundled("minify/KeepNamesNestedAndNamespace", { + files: { + "/entry.ts": /* ts */ ` + export default function () {} + function outer() { + function inner() {} + return inner; + } + namespace NS { export function nsFn() {} export class NsClass {} } + console.log(outer.name, outer().name, NS.nsFn.name, NS.NsClass.name); + `, + "/main.ts": /* ts */ ` + import def from "./entry.ts"; + console.log(def.name); + `, }, + entryPoints: ["/main.ts"], + run: { stdout: "outer inner nsFn NsClass\ndefault" }, minifySyntax: true, minifyIdentifiers: true, keepNames: true, target: "bun", }); + itBundled("minify/KeepNamesTreeShakesUnused", { + files: { + "/entry.js": /* js */ ` + function REMOVE_fn() {} + class REMOVE_cls {} + const REMOVE_arrow = () => {}; + const REMOVE_expr = function REMOVE_named() {}; + export function keepFn() {} + `, + }, + dce: true, + dceKeepMarkerCount: false, + minifySyntax: true, + keepNames: true, + target: "bun", + }); itBundled("minify/PrivateIdentifiersNameCollision", { files: { "/entry.js": /* js */ ` diff --git a/test/bundler/esbuild/default.test.ts b/test/bundler/esbuild/default.test.ts index 950319df7bab..9c0da146caec 100644 --- a/test/bundler/esbuild/default.test.ts +++ b/test/bundler/esbuild/default.test.ts @@ -4655,30 +4655,29 @@ describe.concurrent("bundler", () => { // }, // }); itBundled("default/KeepNamesTreeShaking", { - todo: true, // TODO: Full keepNames implementation with Object.defineProperty - files: { - "/entry.js": /* js */ ` - (function() { - function fnStmtRemove() {} - function fnStmtKeep() {} - x = fnStmtKeep - - let fnExprRemove = function remove() {} - let fnExprKeep = function keepFn() {} - x = fnExprKeep - - class clsStmtRemove {} - class clsStmtKeep {} - new clsStmtKeep() - - let clsExprRemove = class remove {} - let clsExprKeep = class keepClass {} - new clsExprKeep() - })(); + files: { + "/entry.js": /* js */ ` + function fnStmtRemove() {} + function fnStmtKeep() {} + x = fnStmtKeep + + let fnExprRemove = function remove() {} + let fnExprKeep = function keepFn() {} + x = fnExprKeep + + class clsStmtRemove {} + class clsStmtKeep {} + new clsStmtKeep() + + let clsExprRemove = class remove {} + let clsExprKeep = class keepClass {} + new clsExprKeep() `, }, keepNames: true, + minifySyntax: true, dce: true, + dceKeepMarkerCount: false, onAfterBundle(api) { // to properly check that keep names actually worked, we need to minify the // file and THEN check for the names. we do this separatly just so that we know that @@ -4692,7 +4691,6 @@ describe.concurrent("bundler", () => { }, }); itBundled("default/KeepNamesClassStaticName", { - todo: true, // TODO: Full keepNames implementation with Object.defineProperty files: { "/entry.js": /* js */ ` class ClassName1A { static foo = 1 } diff --git a/test/bundler/esbuild/extra.test.ts b/test/bundler/esbuild/extra.test.ts index 9354073c7229..33d96f7fad73 100644 --- a/test/bundler/esbuild/extra.test.ts +++ b/test/bundler/esbuild/extra.test.ts @@ -1639,7 +1639,6 @@ describe("bundler", () => { run: true, }); itBundled(`extra/FunctionHoistingKeepNames1`, { - todo: true, // keepNames requires Object.defineProperty implementation files: { "in.js": ` var f @@ -1651,7 +1650,6 @@ describe("bundler", () => { run: true, }); itBundled(`extra/FunctionHoistingKeepNames2`, { - todo: true, // keepNames requires Object.defineProperty implementation files: { "in.js": ` var f