diff --git a/src/js_parser/visit/mod.rs b/src/js_parser/visit/mod.rs index dd62b2fc2ed2..ea1d9ed7894a 100644 --- a/src/js_parser/visit/mod.rs +++ b/src/js_parser/visit/mod.rs @@ -279,10 +279,11 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O .features .replace_exports .get_ptr(name) - .map(|r| (bun_ptr::BackRef::new(r), r.is_replace())); - if let Some((ptr, is_replace)) = found { + .map(bun_ptr::BackRef::new); + if let Some(ptr) = found { replacement = Some(ptr); - if self.options.features.dead_code_elimination && !is_replace { + // Every entry kind discards this initializer, so nothing it references is a use. + if self.options.features.dead_code_elimination { self.is_control_flow_dead = true; } } diff --git a/src/js_parser/visit/visit_stmt.rs b/src/js_parser/visit/visit_stmt.rs index dfce8bec1923..049f45fca853 100644 --- a/src/js_parser/visit/visit_stmt.rs +++ b/src/js_parser/visit/visit_stmt.rs @@ -381,13 +381,17 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O } let mut mark_for_replace: bool = false; + let mut replace_keeps_stmt: bool = false; let orig_dead = p.is_control_flow_dead; if p.options.features.replace_exports.count() > 0 { if let Some(entry) = p.options.features.replace_exports.get_ptr(b"default") { - p.is_control_flow_dead = - p.options.features.dead_code_elimination && !entry.is_replace(); + // Every entry kind discards the exported value, so nothing it references is a use. + if p.options.features.dead_code_elimination { + p.is_control_flow_dead = true; + } mark_for_replace = true; + replace_keeps_stmt = entry.is_replace(); } } @@ -397,6 +401,15 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O }; } + // The statement a `Replace` entry keeps is live; only its old value was dead. + macro_rules! discarded_value_visited { + () => { + if replace_keeps_stmt { + restore_dead!(); + } + }; + } + match &mut data.value { js_ast::StmtOrExpr::Expr(expr) => { let was_anonymous_named_expr = expr.is_anonymous_named(); @@ -423,6 +436,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O p.react_compiler_candidate_name = None; p.react_compiler_in_react_hoc = false; p.decorator_class_name = prev_decorator_class_name; + discarded_value_visited!(); if p.is_control_flow_dead { restore_dead!(); @@ -605,6 +619,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O let open_parens_loc = func.func.open_parens_loc; func.func = p.visit_func(core::mem::take(&mut func.func), open_parens_loc); p.react_compiler_candidate_name = None; + discarded_value_visited!(); if p.is_control_flow_dead { p.react_refresh.hook_ctx_storage = prev; @@ -774,6 +789,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O StmtData::SClass(mut class_ref) => { let class: &mut S::Class = &mut *class_ref; let _ = p.visit_class(s2_loc, &mut class.class, data.default_name.ref_); + discarded_value_visited!(); if p.is_control_flow_dead { restore_dead!(); @@ -794,6 +810,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O ) = entry { data.value = js_ast::StmtOrExpr::Expr(replace_expr); + stmts.push(*stmt); } else { let _ = p.inject_replacement_export( stmts, @@ -801,10 +818,10 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O 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() { diff --git a/test/bundler/transpiler/transpiler.test.js b/test/bundler/transpiler/transpiler.test.js index a9a0d382aaed..6f50b3c26ab8 100644 --- a/test/bundler/transpiler/transpiler.test.js +++ b/test/bundler/transpiler/transpiler.test.js @@ -2043,6 +2043,95 @@ export default class { expect(output.includes("localVarToReplace")).toBe(true); expect(output.includes("localVarToRemove")).toBe(false); }); + + describe("the value an entry discards is dead code", () => { + // A plain replacement (`foo`), an injected export (`getStaticProps`) and an eliminated + // export (`loader`) all drop the original value, so the three must trim its imports alike. + const replacing = new Bun.Transpiler({ + exports: { + replace: { foo: 42, getStaticProps: ["__N_SSG", true] }, + eliminate: ["loader"], + }, + treeShaking: true, + trimUnusedImports: true, + }); + const replacingDefault = new Bun.Transpiler({ + exports: { replace: { default: 42 } }, + treeShaking: true, + trimUnusedImports: true, + }); + const deadImport = `import deadFS from 'fs';\n`; + const deadCall = `deadFS.readFileSync("/etc/passwd")`; + + it.each([ + ["export const, replaced", `export const foo = () => ${deadCall};`, "export const foo = 42;\n"], + ["export var, replaced", `export var foo = () => ${deadCall};`, "export var foo = 42;\n"], + ["export let, replaced", `export let foo = () => ${deadCall};`, "export let foo = 42;\n"], + ["export function, replaced", `export function foo() { return ${deadCall}; }`, "export var foo = 42;\n"], + [ + "export const, injected", + `export const getStaticProps = () => ${deadCall};`, + "export const __N_SSG = true;\n", + ], + ["export const, eliminated", `export const loader = () => ${deadCall};`, ""], + ])("%s: trims an import only the discarded value used", (_, source, expected) => { + expect(replacing.transformSync(deadImport + source)).toBe(expected); + expect(replacing.scan(deadImport + source).imports).toEqual([]); + }); + + it.each([ + ["expression", `export default ${deadCall};`], + ["arrow function", `export default () => ${deadCall};`], + ["function declaration", `export default function Page() { return ${deadCall}; }`], + ["class declaration", `export default class Page { method() { return ${deadCall}; } }`], + ["anonymous class", `export default class { method() { return ${deadCall}; } }`], + // The import is referenced outside of any method body here, so the class itself must go. + ["class extending the import", `export default class Page extends deadFS {}`], + ["class with a static initializer", `export default class { static contents = ${deadCall}; }`], + ])("export default %s, replaced: trims an import only the discarded value used", (_, source) => { + expect(replacingDefault.transformSync(deadImport + source)).toBe("export default 42;\n"); + expect(replacingDefault.scan(deadImport + source)).toEqual({ exports: ["default"], imports: [] }); + }); + + it("does not record an import() made inside the discarded value", () => { + const source = `export const foo = () => import("./only-used-here");`; + expect(replacing.transformSync(source)).toBe("export const foo = 42;\n"); + expect(replacing.scan(source).imports).toEqual([]); + }); + + it("typescript: drops the binding but keeps the import statement, like eliminate does", () => { + // TypeScript removes an import statement only when its bindings are unused in the + // whole file, dead code included, so the statement survives as a side effect import. + const ts = new Bun.Transpiler({ + loader: "ts", + exports: { replace: { foo: 42 }, eliminate: ["loader"] }, + treeShaking: true, + trimUnusedImports: true, + }); + expect(ts.transformSync(deadImport + `export const foo = () => ${deadCall};`)).toBe( + 'import"fs";\nexport const foo = 42;\n', + ); + expect(ts.transformSync(deadImport + `export const loader = () => ${deadCall};`)).toBe('import"fs";\n'); + }); + + it("keeps an import that is also used outside the discarded value", () => { + expect( + replacing.transformSync( + `import deadFS from 'fs'; + import liveFS from 'fs'; + export const foo = () => ${deadCall}; + export function baz() { return liveFS.readFileSync("/etc/passwd"); }`, + ), + ).toBe( + 'import liveFS from "fs";\nexport const foo = 42;\nexport function baz() {\n return liveFS.readFileSync("/etc/passwd");\n}\n', + ); + + // Only the replaced declaration is dead, not the other declarations of the same statement. + expect( + replacing.transformSync(`import { dead, live } from 'fs';\nexport const foo = () => dead(), other = live;`), + ).toBe('import { live } from "fs";\nexport const foo = 42;\nexport const other = live;\n'); + }); + }); }); const bunTranspiler = new Bun.Transpiler({