From cf414ea1e077c5d60ab15a3de46286f2e9b42a56 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 15 Oct 2026 11:37:01 +0000 Subject: [PATCH 1/4] js_parser: roll back a removed statement's tracked uses along with its use counts When the visit leaves a top-level statement empty, the parser rolls the statement's uses back from `use_count_estimate`. Two counts that are compared with `use_count_estimate` were not rolled back: - the tracked uses of an import() / require() namespace local. One removed `!ns;` or `if (ns && process.env.NODE_ENV !== "production") ...` hid one use of the whole namespace, and the importee lost every export. - the `module.exports` rewrites. One removed `if (module.exports.a) {}` hid one other use of `module`, and the file lost its CommonJS wrapper. Each count now keeps the share that the open part noted. `revert_dead_part` replaces `clear_symbol_usages_from_dead_part` and takes that share back together with the uses. The roll-back no longer sets the use count of a symbol the part declared to zero. An unwrapped `var x = require()` leaves its statement empty, and a use of `x` in a function above it was lost. `ignore_usage_of_runtime_require` calls `ignore_usage` for `require`. A folded `typeof require` or `require.main === module` subtracted a use outside the part's record, so a removed statement subtracted it twice, and unreachable code subtracted a use that was never counted. --- src/js_parser/fold.rs | 2 +- src/js_parser/p.rs | 115 ++++++++---- src/js_parser/parse/parse_entry.rs | 9 +- src/js_parser/visit/mod.rs | 7 +- test/bundler/bundler_cjs2esm.test.ts | 105 +++++++++++ .../bundler_dynamic_import_dce.test.ts | 171 ++++++++++++++++++ 6 files changed, 369 insertions(+), 40 deletions(-) diff --git a/src/js_parser/fold.rs b/src/js_parser/fold.rs index 1e01b7280c76..ca8de8f5a203 100644 --- a/src/js_parser/fold.rs +++ b/src/js_parser/fold.rs @@ -441,7 +441,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O } // rewrite `module.exports` to `exports` - p.module_exports_rewrite_count += 1; + p.module_exports_rewrites.note(p.part_generation); return Some(Expr { data: js_ast::ExprData::ESpecial(E::Special::ModuleExports), loc: name_loc, diff --git a/src/js_parser/p.rs b/src/js_parser/p.rs index f980076379b9..aaa584043367 100644 --- a/src/js_parser/p.rs +++ b/src/js_parser/p.rs @@ -235,6 +235,40 @@ pub enum ReactRefreshExportKind { Default, } +/// Uses of one symbol that stay in its `use_count_estimate` while the visit +/// answers for them another way. A reader compares `total` with that count. +/// A part the visit leaves empty takes its uses back from the count, so it +/// takes its notes back from `total` too (`revert_dead_part`). +#[derive(Default, Clone, Copy)] +pub(crate) struct AccountedUses { + pub(crate) total: u32, + /// The notes in `total` from the part `part_generation` names. + in_part: u32, + part_generation: u32, +} + +impl AccountedUses { + pub(crate) fn note(&mut self, part_generation: u32) { + if self.part_generation != part_generation { + self.part_generation = part_generation; + self.in_part = 0; + } + self.in_part += 1; + self.total = self.total.saturating_add(1); + } + + fn revert(&mut self, part_generation: u32) { + if self.part_generation != part_generation { + return; + } + // `u32::MAX` marks "more uses than accounted for"; it is not a count. + if self.total != u32::MAX { + self.total = self.total.saturating_sub(self.in_part); + } + self.in_part = 0; + } +} + // ───────────────────────────────────────────────────────────────────────────── // P — the parser struct. // `'a` covers borrowed init() params (log/define/source) AND the arena (`bump`). @@ -360,15 +394,16 @@ pub struct P<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> { /// Per namespace ref, how many of its `use_count_estimate` uses were an /// accounted-for read (`ns.a`, a destructure, a truthiness test). Kept /// beside the real count rather than decremented from it so the minifier's - /// single-use substitution still sees every use. - pub(crate) namespace_tracked_uses: HashMap, + /// single-use substitution still sees every use. The synthetic ref of an + /// `import()` has no use count: a note there says its consumer was tracked. + pub(crate) namespace_tracked_uses: HashMap, pub(crate) unwrap_all_requires: bool, pub(crate) commonjs_named_exports: bun_ast::ast_result::CommonJSNamedExports, pub(crate) commonjs_named_exports_deoptimized: bool, pub(crate) commonjs_module_exports_assigned_deoptimized: bool, /// Uses of `module` that became `module.exports` and stay in its use count. - pub(crate) module_exports_rewrite_count: u32, + pub(crate) module_exports_rewrites: AccountedUses, pub(crate) commonjs_named_exports_needs_conversion: u32, pub(crate) had_commonjs_named_exports_this_visit: bool, pub(crate) commonjs_replacement_stmts: StmtNodeList, @@ -1287,8 +1322,15 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O if self.is_revisit_for_substitution || self.is_control_flow_dead { return; } - let n = self.namespace_tracked_uses.entry(ns).or_default(); - *n = n.saturating_add(1); + // `revert_dead_part` finds a local's notes through its use in the part. + debug_assert!( + !self.dynamic_import_namespace_locals.contains_key(&ns) + || self.existing_part_use(ns).is_some() + ); + self.namespace_tracked_uses + .entry(ns) + .or_default() + .note(self.part_generation); } /// `ns` in a position that only tests it for null/truthiness (`if (ns)`, @@ -1804,22 +1846,38 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O self.deoptimize_common_js_named_exports(); } - fn clear_symbol_usages_from_dead_part(&mut self, part: &js_ast::Part) { - let symbol_use_refs = part.symbol_uses.keys(); - let symbol_use_values = part.symbol_uses.values(); - let symbols = self.symbols.as_mut_slice(); - - debug_assert_eq!(symbol_use_refs.len(), symbol_use_values.len()); - for (r#ref, prev) in symbol_use_refs.iter().zip(symbol_use_values) { - symbols[r#ref.inner_index() as usize].use_count_estimate = symbols - [r#ref.inner_index() as usize] - .use_count_estimate - .saturating_sub(prev.count_estimate()); - } - let declared_refs = part.declared_symbols.refs(); - for declared in declared_refs { - symbols[declared.inner_index() as usize].use_count_estimate = 0; + /// The visit left the current part without a statement. Take back what it + /// recorded, so the file's counts read as if the part was never there: + /// its uses from `use_count_estimate`, and its notes from the counts that + /// are compared with it. A symbol the part declared keeps the uses other + /// parts have of it: an unwrapped `const x = require()` leaves its part + /// empty and `x` is still the import's namespace. + fn revert_dead_part(&mut self) { + let part_generation = self.part_generation; + let live = u64::from(part_generation) << 32; + for (at, &(ref_, use_)) in self.part_uses.iter().enumerate() { + let i = ref_.inner_index() as usize; + if self.part_use_slot[i] == live | at as u64 { + let symbol = &mut self.symbols[i]; + symbol.use_count_estimate = symbol + .use_count_estimate + .saturating_sub(use_.count_estimate()); + } + // A note on a namespace local follows a use of it in the same + // part, so the local is listed here, also once `ignore_usage` + // forgot the entry. An `import()`'s synthetic ref has no use, and + // its notes stay. + if !self.namespace_tracked_uses.is_empty() + && let Some(tracked) = self.namespace_tracked_uses.get_mut(&ref_) + { + tracked.revert(part_generation); + } } + self.module_exports_rewrites.revert(part_generation); + self.part_uses.clear(); + self.part_generation += 1; + self.declared_symbols.clear_retaining_capacity(); + self.import_records_for_current_part.clear(); } // s() lives in the impl block above (deduped). @@ -5735,15 +5793,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O // to empty by `core::mem::take` above; no second assignment needed. self.had_commonjs_named_exports_this_visit = false; } else if self.declared_symbols.len() > 0 || !self.part_uses.is_empty() { - // if the part is dead, invalidate all the usage counts - let symbol_uses = self.take_symbol_uses()?; - self.clear_symbol_usages_from_dead_part(&js_ast::Part { - declared_symbols: self.declared_symbols.clone()?, - symbol_uses, - ..Default::default() - }); - self.declared_symbols.clear_retaining_capacity(); - self.import_records_for_current_part.clear(); + self.revert_dead_part(); } Ok(()) } @@ -6159,10 +6209,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O debug_assert!(!self.runtime_imports.__require.is_empty()); let r = self.runtime_identifier_ref(b"__require"); self.ignore_usage(r); - self.symbols[self.require_ref.inner_index() as usize].use_count_estimate = self.symbols - [self.require_ref.inner_index() as usize] - .use_count_estimate - .saturating_sub(1); + self.ignore_usage(self.require_ref); } } @@ -9883,7 +9930,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O unwrap_all_requires, commonjs_named_exports: Default::default(), commonjs_module_exports_assigned_deoptimized: false, - module_exports_rewrite_count: 0, + module_exports_rewrites: AccountedUses::default(), commonjs_named_exports_needs_conversion: u32::MAX, had_commonjs_named_exports_this_visit: false, commonjs_replacement_stmts: js_ast::StmtNodeList::EMPTY, diff --git a/src/js_parser/parse/parse_entry.rs b/src/js_parser/parse/parse_entry.rs index 4fc84c1320b0..36276d5677d4 100644 --- a/src/js_parser/parse/parse_entry.rs +++ b/src/js_parser/parse/parse_entry.rs @@ -1312,7 +1312,7 @@ impl<'a> Parser<'a> { // record (the synthetic ref from `transpose_import` plus a // `const ns = …` / `.then(ns => …)` / `{...rest}` local). A record is // fully tracked only when *every* such ref had all its uses accounted - // for (`use_count_estimate == 0`); the alias set is the union across + // for (`namespace_tracked_uses`); the alias set is the union across // them. Untracked records get no entry and keep every export. if !p .imports_to_convert_from_dynamic_import @@ -1347,7 +1347,10 @@ impl<'a> Parser<'a> { // linked symbol was merged with another declaration (a // hoisted `var`, a parameter), so its own use count says // nothing. - let tracked = p.namespace_tracked_uses.get(&ns_ref).copied().unwrap_or(0); + let tracked = p + .namespace_tracked_uses + .get(&ns_ref) + .map_or(0, |uses| uses.total); // A source-visible local escapes when it has uses nobody // accounted for; the synthetic per-`import()` ref (never // referenced in source) escapes when its one consumer did not. @@ -1552,7 +1555,7 @@ impl<'a> Parser<'a> { p.deoptimize_commonjs_named_exports(); } else if p.symbols.as_slice()[p.module_ref.inner_index() as usize] .use_count_estimate - > p.module_exports_rewrite_count + > p.module_exports_rewrites.total { // `module.constructor` and other uses of `module` need the wrapper. p.deoptimize_commonjs_named_exports(); diff --git a/src/js_parser/visit/mod.rs b/src/js_parser/visit/mod.rs index ddd948afc870..3f132c584951 100644 --- a/src/js_parser/visit/mod.rs +++ b/src/js_parser/visit/mod.rs @@ -1966,10 +1966,13 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O // moved into `replacement`; unless it was an accounted-for read // (`f(ns.a)`), the namespace escapes there. if p.dynamic_import_namespace_locals.contains_key(&id) - && p.namespace_tracked_uses.get(&id).copied().unwrap_or(0) == 0 + && p.namespace_tracked_uses + .get(&id) + .map_or(0, |uses| uses.total) + == 0 { // Read as "more uses than accounted for" when finalizing. - p.namespace_tracked_uses.insert(id, u32::MAX); + p.namespace_tracked_uses.entry(id).or_default().total = u32::MAX; } match local.decls.len_u32() { 1 => { diff --git a/test/bundler/bundler_cjs2esm.test.ts b/test/bundler/bundler_cjs2esm.test.ts index 57e31a43c888..3b1a41b6f523 100644 --- a/test/bundler/bundler_cjs2esm.test.ts +++ b/test/bundler/bundler_cjs2esm.test.ts @@ -2496,6 +2496,111 @@ describe("bundler", () => { stdout: "1 2 false", }, }); + for (const backend of ["cli", "api"] as const) { + // The build removes `if (module.exports.a) {}`. The other uses of + // `module` still need the wrapper, and a file with none still lifts. + itBundled(`cjs2esm/RemovedModuleExportsReadKeepsWrapper_${backend}`, { + backend, + files: { + "/entry.js": /* js */ ` + import before from "./before.cjs"; + import after from "./after.cjs"; + import twice from "./twice.cjs"; + import rest from "./rest.cjs"; + import lifted from "./lifted.cjs"; + console.log(before.kind, after.kind, twice.kinds.join(), rest.keys, lifted.a); + `, + "/before.cjs": /* js */ ` + exports.a = 1; + if (module.exports.a) {} + exports.kind = typeof module; + `, + "/after.cjs": /* js */ ` + exports.a = 1; + exports.kind = typeof module; + if (module.exports.a) {} + `, + "/twice.cjs": /* js */ ` + exports.a = 1; + if (module.exports.a) {} + if (module.exports.a) {} + exports.kinds = [typeof module, typeof module]; + `, + // Here `module` is a copy of another file's namespace. + "/rest.cjs": /* js */ ` + var { a, ...module } = require("./esm.mjs"); + !module; + exports.keys = Object.keys(module).join(); + `, + "/esm.mjs": `export const a = 1, b = 2, c = 3;`, + "/lifted.cjs": /* js */ ` + exports.a = 1; + if (module.exports.a) {} + `, + }, + minifySyntax: true, + cjs2esm: { unhandled: ["/before.cjs", "/after.cjs", "/twice.cjs", "/rest.cjs"] }, + run: { stdout: "object object object,object b,c 1" }, + }); + + // `require` is the only CommonJS sign of these files. A folded + // `typeof require` or `require.main === module` is no use of it, and + // takes no other use away. + itBundled(`cjs2esm/FoldedRequireReadKeepsOtherRequireUses_${backend}`, { + backend, + files: { + "/entry.js": /* js */ ` + import removed from "./removed.js"; + import unreachable from "./unreachable.js"; + import main from "./main.js"; + console.log(typeof removed, typeof unreachable, typeof main, globalThis.seen.join()); + `, + "/dep.js": `exports.value = 7;`, + "/removed.js": /* js */ ` + const dep = require("./dep.js"); + typeof require; + (globalThis.seen ??= []).push(dep.value); + `, + "/unreachable.js": /* js */ ` + const dep = require("./dep.js"); + if (false) console.log(typeof require); + (globalThis.seen ??= []).push(dep.value); + `, + "/main.js": /* js */ ` + const dep = require("./dep.js"); + require.main === module; + (globalThis.seen ??= []).push(dep.value); + `, + }, + run: { stdout: "object object object 7,7,7" }, + }); + + // `var impl = require()` becomes an import and leaves its statement + // empty. A use in a function above it is still a use. + itBundled(`cjs2esm/UseAboveUnwrappedRequireDeclaration_${backend}`, { + backend, + files: { + "/entry.js": /* js */ ` + import ReactDOM from "react-dom"; + import { keys } from "./typed.ts"; + console.log(Object.keys(ReactDOM).join(), Object.keys(globalThis.above()).join(), keys()); + `, + "/node_modules/react-dom/index.js": /* js */ ` + function above() { return impl; } + var impl = require("./impl.js"); + globalThis.above = above; + module.exports = impl; + `, + "/node_modules/react-dom/impl.js": `exports.a = 1; exports.b = 2;`, + "/typed.ts": /* ts */ ` + export function keys() { return Object.keys(React).join(); } + const React = require("react"); + `, + "/node_modules/react/index.js": `exports.version = "19.0.0"; exports.createElement = function () {};`, + }, + run: { stdout: "a,b a,b version,createElement" }, + }); + } // Module code makes a top-level function declaration lexical, so a `var` // with the same name is a SyntaxError there. A function body allows it, so // these files keep their `__commonJS` wrapper. In sloppy mode the bundler diff --git a/test/bundler/bundler_dynamic_import_dce.test.ts b/test/bundler/bundler_dynamic_import_dce.test.ts index 77991e8d2d7b..60ed13bd16b5 100644 --- a/test/bundler/bundler_dynamic_import_dce.test.ts +++ b/test/bundler/bundler_dynamic_import_dce.test.ts @@ -4752,6 +4752,177 @@ describe("bundler", () => { }); } + // ── Statements the build removes ────────────────────────────────────── + // A test of a namespace local (`!ns`, `if (ns && DEV)`) observes no export. + // When the build removes the statement, the test goes with it: the uses + // that remain decide what the importee keeps. + + const named = (name: string) => `export const from = "${name}"; export const other = 1;`; + const marked = (name: string) => `export const a = "${name}_A"; export const b = "${name}_B";`; + + for (const backend of ["cli", "api"] as const) { + // Every way to bind the namespace, each with a removed test and one use + // of the whole namespace. + itBundled(`dynamic_import_dce/RemovedTestKeepsWholeUse_${backend}`, { + backend, + files: { + "/entry.js": /* js */ ` + const viaImport = await import("./import.js"); + !viaImport; + const viaRequire = require("./require.js"); + !viaRequire; + const gated = process.argv.length > 0 ? require("./gated.js") : null; + !gated; + const [viaAll] = await Promise.all([import("./all.js")]); + !viaAll; + const { ...rest } = await import("./rest.js"); + !rest; + let viaLet = await import("./let.js"); + !viaLet; + const first = 1, second = await import("./second.js"); + !second; + for (const ns of [viaImport, viaRequire, gated, viaAll, rest, viaLet, second]) { + console.log(JSON.stringify({ from: first, ...ns })); + } + `, + "/import.js": named("import"), + "/require.js": named("require"), + "/gated.js": named("gated"), + "/all.js": named("all"), + "/rest.js": named("rest"), + "/let.js": named("let"), + "/second.js": named("second"), + }, + run: { + stdout: ["import", "require", "gated", "all", "rest", "let", "second"] + .map(from => JSON.stringify({ from, other: 1 })) + .join("\n"), + }, + }); + + // The removed statement before or after the use, a use above the + // declaration, and one removed test for each use. + itBundled(`dynamic_import_dce/RemovedTestOrderAndCount_${backend}`, { + backend, + files: { + "/entry.js": /* js */ ` + function above() { return Object.keys(hoisted).join(); } + + const before = await import("./before.js"); + !before; + const beforeKeys = Object.keys(before).join(); + + const after = await import("./after.js"); + const afterKeys = Object.keys(after).join(); + !after; + + const hoisted = await import("./hoisted.js"); + !hoisted; + + const once = await import("./once.js"); + !once; + const onceKeys = Object.keys(once).join() + Object.keys({ ...once }).join(); + + const twice = await import("./twice.js"); + !twice; + typeof twice; + const twiceKeys = Object.keys(twice).join() + Object.keys({ ...twice }).join(); + + const inArrow = await import("./arrow.js"); + (() => inArrow.a); + const arrowKeys = Object.keys(inArrow).join(); + + export function spread() { return Object.keys({ ...exported }).join(); } + const exported = await import("./exported.js"); + !exported; + + console.log(beforeKeys, afterKeys, above(), onceKeys, twiceKeys, arrowKeys, spread()); + `, + "/before.js": `export const a = 1, b = 2;`, + "/after.js": `export const a = 1, b = 2;`, + "/hoisted.js": `export const a = 1, b = 2;`, + "/once.js": `export const a = 1, b = 2;`, + "/twice.js": `export const a = 1, b = 2;`, + "/arrow.js": `export const a = 1, b = 2;`, + "/exported.js": `export const a = 1, b = 2;`, + }, + run: { stdout: "a,b a,b a,b a,ba,b a,ba,b a,b a,b" }, + }); + + // A removed statement keeps no export alive by itself. + itBundled(`dynamic_import_dce/RemovedStatementStillNarrows_${backend}`, { + backend, + files: { + "/entry.js": /* js */ ` + const whole = await import("./whole.js"); + !whole; + console.log(Object.keys(whole).join()); + + const unused = await import("./unused.js"); + !unused; + + const bare = await import("./bare.js"); + console.log(bare.a); + bare; + + (async () => { const { a } = await import("./callback.js"); return a; }); + const { b } = await import("./callback.js"); + console.log(b); + `, + "/whole.js": marked("WHOLE"), + "/unused.js": marked("UNUSED"), + "/bare.js": marked("BARE"), + "/callback.js": marked("CALLBACK"), + }, + run: { stdout: "a,b\nBARE_A\nCALLBACK_B" }, + onAfterBundle(api) { + const kept = api.readFile("/out.js").match(/[A-Z]+_[AB]\b/g)!; + expect([...new Set(kept)].sort()).toEqual(["BARE_A", "CALLBACK_B", "WHOLE_A", "WHOLE_B"]); + }, + }); + + // The statements that `--minify-syntax`, `--drop` and `--define` remove. + itBundled(`dynamic_import_dce/RemovedTestFromBuildFlags_${backend}`, { + backend, + files: { + "/entry.js": /* js */ ` + const gate = await import("./gate.js"); + if (gate && process.env.NODE_ENV !== "production") console.log("development build"); + const logged = await import("./logged.js"); + logged && console.log("loaded"); + const flagged = await import("./flagged.js"); + if (flagged && DEBUG) console.log(flagged); + const member = await import("./member.js"); + if (member.a) {} + const keys = [gate, logged, flagged, member].map(ns => Object.keys(ns).join()); + process.stdout.write(keys.join(" ") + "\\n"); + `, + "/gate.js": `export const a = 1, b = 2;`, + "/logged.js": `export const a = 1, b = 2;`, + "/flagged.js": `export const a = 1, b = 2;`, + "/member.js": `export const a = 1, b = 2;`, + }, + minifySyntax: true, + drop: ["console"], + define: { "process.env.NODE_ENV": '"production"', DEBUG: "false" }, + run: { stdout: "a,b a,b a,b a,b" }, + }); + } + + itBundled("dynamic_import_dce/RemovedTestProduction", { + backend: "cli", + files: { + "/entry.js": /* js */ ` + const ns = await import("./lib.js"); + if (ns && process.env.NODE_ENV !== "production") console.log("development build"); + console.log(JSON.stringify(Object.keys(ns))); + `, + "/lib.js": `export const a = 1; export const b = 2;`, + }, + production: true, + run: { stdout: '["a","b"]' }, + }); + // ── Cases that keep the namespace object ────────────────────────────── function itKeepsNamespace( From 95583045fdcb219e4bdab3dbdc7552c1bc9e28e5 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 18 Oct 2026 03:02:08 +0000 Subject: [PATCH 2/4] js_parser: take a folded `require` use back on the revisit too `const r = require; return typeof r;` folds to "function" on the revisit that follows single-use substitution. `ignore_usage` does nothing on a revisit, so after the previous commit the use of `require` that the first visit recorded stayed, and a file with no other CommonJS sign got a `__commonJS` wrapper that it does not get on main. `ignore_usage_of_runtime_require` now takes the `require` use back from the count and from the part's record whenever control flow is live. A file like that is plain ESM again. The same fold inside a removed statement still takes exactly one use away. --- src/js_parser/p.rs | 34 ++++++++++++++++++---------- test/bundler/bundler_cjs2esm.test.ts | 27 ++++++++++++++++++++++ 2 files changed, 49 insertions(+), 12 deletions(-) diff --git a/src/js_parser/p.rs b/src/js_parser/p.rs index aaa584043367..f9da36dc8108 100644 --- a/src/js_parser/p.rs +++ b/src/js_parser/p.rs @@ -6209,7 +6209,11 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O debug_assert!(!self.runtime_imports.__require.is_empty()); let r = self.runtime_identifier_ref(b"__require"); self.ignore_usage(r); - self.ignore_usage(self.require_ref); + // `const r = require; typeof r` folds on the revisit that follows + // the substitution. The first visit recorded that use of `require`. + if !self.is_control_flow_dead { + self.ignore_recorded_usage(self.require_ref); + } } } @@ -6811,23 +6815,29 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O pub(crate) fn ignore_usage(&mut self, r#ref: Ref) { if !self.is_control_flow_dead && !self.is_revisit_for_substitution { - debug_assert!((r#ref.inner_index() as usize) < self.symbols.len()); - self.symbols[r#ref.inner_index() as usize].use_count_estimate = self.symbols - [r#ref.inner_index() as usize] - .use_count_estimate - .saturating_sub(1); - if let Some(use_) = self.existing_part_use(r#ref) { - use_.subtract(1); - if use_.count_estimate() == 0 { - self.forget_part_use(r#ref); - } - } + self.ignore_recorded_usage(r#ref); } // Don't roll back the "tsUseCounts" increment. This must be counted even if // the value is ignored because that's what the TypeScript compiler does. } + /// Takes one use of `ref` back from its count and from the current part. + #[inline] + fn ignore_recorded_usage(&mut self, r#ref: Ref) { + debug_assert!((r#ref.inner_index() as usize) < self.symbols.len()); + self.symbols[r#ref.inner_index() as usize].use_count_estimate = self.symbols + [r#ref.inner_index() as usize] + .use_count_estimate + .saturating_sub(1); + if let Some(use_) = self.existing_part_use(r#ref) { + use_.subtract(1); + if use_.count_estimate() == 0 { + self.forget_part_use(r#ref); + } + } + } + pub(crate) fn ignore_usage_of_identifier_in_dot_chain(&mut self, expr: Expr) { let mut current = expr; loop { diff --git a/test/bundler/bundler_cjs2esm.test.ts b/test/bundler/bundler_cjs2esm.test.ts index 3b1a41b6f523..4c388d329ded 100644 --- a/test/bundler/bundler_cjs2esm.test.ts +++ b/test/bundler/bundler_cjs2esm.test.ts @@ -2575,6 +2575,33 @@ describe("bundler", () => { run: { stdout: "object object object 7,7,7" }, }); + // The minifier moves `require` into `typeof r` and folds it after the + // first visit. In folded.js no use of `require` remains, so the file is + // no CommonJS file. In removed.js the `require()` call still is a use. + itBundled(`cjs2esm/SubstitutedRequireReadFoldsOneUse_${backend}`, { + backend, + files: { + "/entry.js": /* js */ ` + import "./folded.js"; + import removed from "./removed.js"; + console.log(globalThis.folded, typeof removed, globalThis.removed); + `, + "/folded.js": /* js */ ` + function f() { const r = require; return typeof r; } + globalThis.folded = f(); + `, + "/removed.js": /* js */ ` + const dep = require("./dep.js"); + (function () { const r = require; return typeof r; }); + globalThis.removed = dep.value; + `, + "/dep.js": `exports.value = 7;`, + }, + minifySyntax: true, + cjs2esm: { unhandled: ["/removed.js", "/dep.js"] }, + run: { stdout: "function object 7" }, + }); + // `var impl = require()` becomes an import and leaves its statement // empty. A use in a function above it is still a use. itBundled(`cjs2esm/UseAboveUnwrappedRequireDeclaration_${backend}`, { From 65d3b8bcc0a784b89519d2d6c84f3800918c7528 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 18 Oct 2026 03:06:28 +0000 Subject: [PATCH 3/4] js_parser: shorten the comments of the dead-part roll-back No code change. --- src/js_parser/p.rs | 24 +++++------------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/src/js_parser/p.rs b/src/js_parser/p.rs index f9da36dc8108..0cc979c5978c 100644 --- a/src/js_parser/p.rs +++ b/src/js_parser/p.rs @@ -235,10 +235,7 @@ pub enum ReactRefreshExportKind { Default, } -/// Uses of one symbol that stay in its `use_count_estimate` while the visit -/// answers for them another way. A reader compares `total` with that count. -/// A part the visit leaves empty takes its uses back from the count, so it -/// takes its notes back from `total` too (`revert_dead_part`). +/// Notes beside a symbol's `use_count_estimate`. A part left empty takes its own back (`revert_dead_part`). #[derive(Default, Clone, Copy)] pub(crate) struct AccountedUses { pub(crate) total: u32, @@ -394,8 +391,7 @@ pub struct P<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> { /// Per namespace ref, how many of its `use_count_estimate` uses were an /// accounted-for read (`ns.a`, a destructure, a truthiness test). Kept /// beside the real count rather than decremented from it so the minifier's - /// single-use substitution still sees every use. The synthetic ref of an - /// `import()` has no use count: a note there says its consumer was tracked. + /// single-use substitution still sees every use. pub(crate) namespace_tracked_uses: HashMap, pub(crate) unwrap_all_requires: bool, @@ -1846,12 +1842,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O self.deoptimize_common_js_named_exports(); } - /// The visit left the current part without a statement. Take back what it - /// recorded, so the file's counts read as if the part was never there: - /// its uses from `use_count_estimate`, and its notes from the counts that - /// are compared with it. A symbol the part declared keeps the uses other - /// parts have of it: an unwrapped `const x = require()` leaves its part - /// empty and `x` is still the import's namespace. + /// The visit left the current part empty: takes back the uses and the notes it recorded. fn revert_dead_part(&mut self) { let part_generation = self.part_generation; let live = u64::from(part_generation) << 32; @@ -1863,10 +1854,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O .use_count_estimate .saturating_sub(use_.count_estimate()); } - // A note on a namespace local follows a use of it in the same - // part, so the local is listed here, also once `ignore_usage` - // forgot the entry. An `import()`'s synthetic ref has no use, and - // its notes stay. + // For a forgotten entry too: `ignore_usage` drops the entry, not the note. if !self.namespace_tracked_uses.is_empty() && let Some(tracked) = self.namespace_tracked_uses.get_mut(&ref_) { @@ -6209,8 +6197,6 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O debug_assert!(!self.runtime_imports.__require.is_empty()); let r = self.runtime_identifier_ref(b"__require"); self.ignore_usage(r); - // `const r = require; typeof r` folds on the revisit that follows - // the substitution. The first visit recorded that use of `require`. if !self.is_control_flow_dead { self.ignore_recorded_usage(self.require_ref); } @@ -6822,7 +6808,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O // the value is ignored because that's what the TypeScript compiler does. } - /// Takes one use of `ref` back from its count and from the current part. + /// `ignore_usage` for a use the first visit recorded: a fold on the revisit takes it back too. #[inline] fn ignore_recorded_usage(&mut self, r#ref: Ref) { debug_assert!((r#ref.inner_index() as usize) < self.symbols.len()); From 40864409bd1f35afe48e5eae745cb254683910e0 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 20 Oct 2026 09:18:23 +0000 Subject: [PATCH 4/4] js_parser: roll back the property reads and the escape mark of a removed statement A removed statement that reads a property of a destructured import() or require() local left one use on that local. The roll-back zeroed the locals the statement declared, which hid it. The roll-back now takes the property reads back, and a read of a local declared outside the statement keeps no export either. The mark that single-use substitution leaves on a namespace local goes with its statement. `var module = require()` and `var exports = require()` of an unwrapped package make the wrapper's variable an import namespace. For these two symbols the roll-back still zeroes the use count, as before this branch: a use above the declaration is a use of the namespace. --- src/js_parser/p.rs | 40 ++++-- src/js_parser/visit/mod.rs | 5 +- test/bundler/bundler_cjs2esm.test.ts | 81 +++++++++++- .../bundler_dynamic_import_dce.test.ts | 121 +++++++++++++----- 4 files changed, 202 insertions(+), 45 deletions(-) diff --git a/src/js_parser/p.rs b/src/js_parser/p.rs index 959fd1367860..f884f6e71d02 100644 --- a/src/js_parser/p.rs +++ b/src/js_parser/p.rs @@ -238,31 +238,41 @@ pub enum ReactRefreshExportKind { /// Notes beside a symbol's `use_count_estimate`. A part left empty takes its own back (`revert_dead_part`). #[derive(Default, Clone, Copy)] pub(crate) struct AccountedUses { + /// `u32::MAX` is not a count: it says "more uses than accounted for". pub(crate) total: u32, - /// The notes in `total` from the part `part_generation` names. + /// The share of `total` from the part `part_generation` names. in_part: u32, part_generation: u32, } impl AccountedUses { pub(crate) fn note(&mut self, part_generation: u32) { + if self.total == u32::MAX { + return; + } if self.part_generation != part_generation { self.part_generation = part_generation; self.in_part = 0; } self.in_part += 1; - self.total = self.total.saturating_add(1); + self.total += 1; + } + + /// The open part holds a use that nothing accounts for. `total` is 0 before. + pub(crate) fn mark_escaped(&mut self, part_generation: u32) { + debug_assert_eq!(self.total, 0); + *self = Self { + total: u32::MAX, + in_part: u32::MAX, + part_generation, + }; } fn revert(&mut self, part_generation: u32) { - if self.part_generation != part_generation { - return; - } - // `u32::MAX` marks "more uses than accounted for"; it is not a count. - if self.total != u32::MAX { + if self.part_generation == part_generation { self.total = self.total.saturating_sub(self.in_part); + self.in_part = 0; } - self.in_part = 0; } } @@ -1847,7 +1857,21 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O tracked.revert(part_generation); } } + // `record_import_property_use` moved these reads out of `part_uses`. `exports` lists calls, not reads. + for (ref_, reads) in self.import_symbol_property_uses.iter() { + if *ref_ != self.exports_ref { + let count: u32 = reads.values().map(|read| read.count_estimate).sum(); + let symbol = &mut self.symbols[ref_.inner_index() as usize]; + symbol.use_count_estimate = symbol.use_count_estimate.saturating_sub(count); + } + } self.module_exports_rewrites.revert(part_generation); + // `var module = require()` made the wrapper's variable an import namespace: a use above it is not of the wrapper's. + for declared in self.declared_symbols.refs() { + if *declared == self.module_ref || *declared == self.exports_ref { + self.symbols[declared.inner_index() as usize].use_count_estimate = 0; + } + } self.part_uses.clear(); self.part_generation += 1; self.declared_symbols.clear_retaining_capacity(); diff --git a/src/js_parser/visit/mod.rs b/src/js_parser/visit/mod.rs index 2e0b8d34549e..dafd57319ce7 100644 --- a/src/js_parser/visit/mod.rs +++ b/src/js_parser/visit/mod.rs @@ -1972,7 +1972,10 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O == 0 { // Read as "more uses than accounted for" when finalizing. - p.namespace_tracked_uses.entry(id).or_default().total = u32::MAX; + p.namespace_tracked_uses + .entry(id) + .or_default() + .mark_escaped(p.part_generation); } match local.decls.len_u32() { 1 => { diff --git a/test/bundler/bundler_cjs2esm.test.ts b/test/bundler/bundler_cjs2esm.test.ts index 4c388d329ded..6a4611d9057a 100644 --- a/test/bundler/bundler_cjs2esm.test.ts +++ b/test/bundler/bundler_cjs2esm.test.ts @@ -2508,7 +2508,14 @@ describe("bundler", () => { import twice from "./twice.cjs"; import rest from "./rest.cjs"; import lifted from "./lifted.cjs"; + import pair from "./pair.cjs"; + import keptThenRemoved from "./keptThenRemoved.cjs"; + import alias from "./alias.cjs"; + import aliasRemoved from "./aliasRemoved.cjs"; + import call from "./call.cjs"; console.log(before.kind, after.kind, twice.kinds.join(), rest.keys, lifted.a); + console.log(pair.kind, keptThenRemoved.b, alias.a, alias.get(), aliasRemoved.kind); + console.log(call.seen, typeof call.f); `, "/before.cjs": /* js */ ` exports.a = 1; @@ -2537,10 +2544,50 @@ describe("bundler", () => { exports.a = 1; if (module.exports.a) {} `, + // One removed statement with two reads. + "/pair.cjs": /* js */ ` + exports.a = 1; + exports.b = 2; + if (module.exports.a && module.exports.b) {} + exports.kind = typeof module; + `, + // The read in the kept statement still counts: the file lifts. + "/keptThenRemoved.cjs": /* js */ ` + exports.a = 1; + exports.b = module.exports.a + 1; + if (module.exports.a) {} + `, + // The minifier moves `module` into `m.exports` and the read is + // found on the second visit: in a kept statement, then a removed one. + "/alias.cjs": /* js */ ` + exports.a = 1; + exports.get = function () { const m = module; return m.exports.a + 1; }; + `, + "/aliasRemoved.cjs": /* js */ ` + exports.a = 1; + (function () { const m = module; return m.exports.a + 1; }); + exports.kind = typeof module; + `, + // The removed call of `exports.f` is no use of `exports`. The one in `Object.keys` stays. + "/call.cjs": /* js */ ` + exports.f = function () { return 1; }; + exports.seen = Object.keys(exports).join(); + (function () { exports.f(); }); + `, }, minifySyntax: true, - cjs2esm: { unhandled: ["/before.cjs", "/after.cjs", "/twice.cjs", "/rest.cjs"] }, - run: { stdout: "object object object,object b,c 1" }, + cjs2esm: { + unhandled: [ + "/before.cjs", + "/after.cjs", + "/twice.cjs", + "/rest.cjs", + "/pair.cjs", + "/aliasRemoved.cjs", + "/call.cjs", + ], + }, + run: { stdout: "object object object,object b,c 1\nobject 2 1 2 object\nf function" }, }); // `require` is the only CommonJS sign of these files. A folded @@ -2627,6 +2674,36 @@ describe("bundler", () => { }, run: { stdout: "a,b a,b version,createElement" }, }); + + // `var module = require()` becomes an import too, and `module` is then + // the namespace, not the variable of the CommonJS wrapper: also for a + // use in a function above the declaration, and for `module.id`. + itBundled(`cjs2esm/VarModuleIsTheUnwrappedRequireNamespace_${backend}`, { + backend, + files: { + "/entry.js": /* js */ ` + import above from "react-dom/above.js"; + import "react-dom/property.js"; + import "react-dom/member.js"; + console.log(above.keys, globalThis.property, globalThis.member); + `, + "/node_modules/react-dom/above.js": /* js */ ` + function keys() { return Object.keys(module).join(); } + var module = require("./impl.js"); + exports.keys = keys(); + `, + "/node_modules/react-dom/property.js": /* js */ ` + var exports = require("./impl.js"); + globalThis.property = typeof exports.a; + `, + "/node_modules/react-dom/member.js": /* js */ ` + var module = require("./impl.js"); + globalThis.member = [module.id, typeof module.exports].join(); + `, + "/node_modules/react-dom/impl.js": `exports.a = 1; exports.id = "impl"; exports.exports = "impl";`, + }, + run: { stdout: "a,id,exports number impl,string" }, + }); } // Module code makes a top-level function declaration lexical, so a `var` // with the same name is a SyntaxError there. A function body allows it, so diff --git a/test/bundler/bundler_dynamic_import_dce.test.ts b/test/bundler/bundler_dynamic_import_dce.test.ts index 60ed13bd16b5..d0fc62988682 100644 --- a/test/bundler/bundler_dynamic_import_dce.test.ts +++ b/test/bundler/bundler_dynamic_import_dce.test.ts @@ -4801,7 +4801,7 @@ describe("bundler", () => { }); // The removed statement before or after the use, a use above the - // declaration, and one removed test for each use. + // declaration, one removed test for each use, and two in one statement. itBundled(`dynamic_import_dce/RemovedTestOrderAndCount_${backend}`, { backend, files: { @@ -4836,7 +4836,12 @@ describe("bundler", () => { const exported = await import("./exported.js"); !exported; - console.log(beforeKeys, afterKeys, above(), onceKeys, twiceKeys, arrowKeys, spread()); + const pair = await import("./pair.js"); + !pair, typeof pair; + (() => [pair.a, pair.b]); + const pairKeys = Object.keys(pair).join() + Object.keys({ ...pair }).join(); + + console.log(beforeKeys, afterKeys, above(), onceKeys, twiceKeys, arrowKeys, spread(), pairKeys); `, "/before.js": `export const a = 1, b = 2;`, "/after.js": `export const a = 1, b = 2;`, @@ -4845,41 +4850,89 @@ describe("bundler", () => { "/twice.js": `export const a = 1, b = 2;`, "/arrow.js": `export const a = 1, b = 2;`, "/exported.js": `export const a = 1, b = 2;`, + "/pair.js": `export const a = 1, b = 2;`, }, - run: { stdout: "a,b a,b a,b a,ba,b a,ba,b a,b a,b" }, + run: { stdout: "a,b a,b a,b a,ba,b a,ba,b a,b a,b a,ba,b" }, }); - // A removed statement keeps no export alive by itself. - itBundled(`dynamic_import_dce/RemovedStatementStillNarrows_${backend}`, { - backend, - files: { - "/entry.js": /* js */ ` - const whole = await import("./whole.js"); - !whole; - console.log(Object.keys(whole).join()); - - const unused = await import("./unused.js"); - !unused; - - const bare = await import("./bare.js"); - console.log(bare.a); - bare; - - (async () => { const { a } = await import("./callback.js"); return a; }); - const { b } = await import("./callback.js"); - console.log(b); - `, - "/whole.js": marked("WHOLE"), - "/unused.js": marked("UNUSED"), - "/bare.js": marked("BARE"), - "/callback.js": marked("CALLBACK"), - }, - run: { stdout: "a,b\nBARE_A\nCALLBACK_B" }, - onAfterBundle(api) { - const kept = api.readFile("/out.js").match(/[A-Z]+_[AB]\b/g)!; - expect([...new Set(kept)].sort()).toEqual(["BARE_A", "CALLBACK_B", "WHOLE_A", "WHOLE_B"]); - }, - }); + // Nothing that a removed statement read stays alive because of it: a + // tested namespace, a name of a pattern and a property of that name, a + // namespace that `--minify-syntax` moved into its only use. A read in a + // kept statement keeps its export (`liveA`, the namespace of `viaSwitch`). + for (const minifySyntax of [false, true]) { + const mode = minifySyntax ? "MinifySyntax" : ""; + itBundled(`dynamic_import_dce/RemovedStatementStillNarrows${mode}_${backend}`, { + backend, + files: { + "/entry.js": /* js */ ` + const whole = await import("./whole.js"); + !whole; + console.log(Object.keys(whole).join()); + + const unused = await import("./unused.js"); + !unused; + + const bare = await import("./bare.js"); + console.log(bare.a); + bare; + + (async () => { const { a } = await import("./callback.js"); return a; }); + const { b } = await import("./callback.js"); + console.log(b); + + const mixed = await import("./mixed.js"); + console.log(mixed.a); + !mixed; + + (function () { const { a } = require("./property.js"); return a.length; }); + const { b: propertyB } = require("./property.js"); + console.log(propertyB); + + const { a: outsideA, b: outsideB } = require("./outside.js"); + (function () { return outsideA.length + outsideA.name + outsideA.length; }); + console.log(outsideB); + + const { a: liveA, b: liveB } = require("./live.js"); + console.log(liveA.length); + (function () { return liveA.length + liveB.length; }); + + (function () { const items = require("./items.js"); return items.a.length; }); + const { b: itemsB } = require("./items.js"); + console.log(itemsB); + + (function () { const escaped = require("./escaped.js"); return escaped; }); + const { b: escapedB } = require("./escaped.js"); + console.log(escapedB); + + function viaSwitch(k) { + switch (k) { + case 1: const ns = require("./switch.js"); return ns; + case 2: return typeof ns; + } + } + console.log(Object.keys(viaSwitch(1)).join()); + `, + "/whole.js": marked("WHOLE"), + "/unused.js": marked("UNUSED"), + "/bare.js": marked("BARE"), + "/callback.js": marked("CALLBACK"), + "/mixed.js": marked("MIXED"), + "/property.js": marked("PROPERTY"), + "/outside.js": marked("OUTSIDE"), + "/live.js": marked("LIVE"), + "/items.js": marked("ITEMS"), + "/escaped.js": marked("ESCAPED"), + "/switch.js": marked("SWITCH"), + }, + minifySyntax, + run: { stdout: "a,b\nBARE_A\nCALLBACK_B\nMIXED_A\nPROPERTY_B\nOUTSIDE_B\n6\nITEMS_B\nESCAPED_B\na,b" }, + onAfterBundle(api) { + const kept = api.readFile("/out.js").match(/[A-Z]+_[AB]\b/g)!; + const live = ["WHOLE_A", "WHOLE_B", "BARE_A", "CALLBACK_B", "MIXED_A", "PROPERTY_B", "OUTSIDE_B", "LIVE_A"]; + expect([...new Set(kept)].sort()).toEqual([...live, "ITEMS_B", "ESCAPED_B", "SWITCH_A", "SWITCH_B"].sort()); + }, + }); + } // The statements that `--minify-syntax`, `--drop` and `--define` remove. itBundled(`dynamic_import_dce/RemovedTestFromBuildFlags_${backend}`, {