Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(mangler): keep exported symbols for top_level: true #7927

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
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
1 change: 1 addition & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions crates/oxc_mangler/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,4 @@ oxc_ast = { workspace = true }
oxc_index = { workspace = true }
oxc_semantic = { workspace = true }
oxc_span = { workspace = true }
rustc-hash = { workspace = true }
49 changes: 44 additions & 5 deletions crates/oxc_mangler/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use itertools::Itertools;
use oxc_ast::ast::Program;
use oxc_ast::ast::{Declaration, Program, Statement};
use oxc_index::{index_vec, Idx, IndexVec};
use oxc_semantic::{ReferenceId, ScopeTree, SemanticBuilder, SymbolId, SymbolTable};
use oxc_span::CompactStr;
use rustc_hash::FxHashSet;

type Slot = usize;

Expand Down Expand Up @@ -85,6 +86,12 @@ impl Mangler {
pub fn build<'a>(mut self, program: &'a Program<'a>) -> Mangler {
let semantic = SemanticBuilder::new().build(program).semantic;

let (exported_names, exported_symbols) = if self.options.top_level {
Mangler::collect_exported_symbols(program)
} else {
Default::default()
};

// Mangle the symbol table by computing slots from the scope tree.
// A slot is the occurrence index of a binding identifier inside a scope.
let (mut symbol_table, scope_tree) = semantic.into_symbol_table_and_scope_tree();
Expand Down Expand Up @@ -126,8 +133,13 @@ impl Mangler {
}
}

let frequencies =
self.tally_slot_frequencies(&symbol_table, &scope_tree, total_number_of_slots, &slots);
let frequencies = self.tally_slot_frequencies(
&symbol_table,
&exported_symbols,
&scope_tree,
total_number_of_slots,
&slots,
);

let root_unresolved_references = scope_tree.root_unresolved_references();
let root_bindings = scope_tree.get_bindings(scope_tree.root_scope_id());
Expand All @@ -145,7 +157,8 @@ impl Mangler {
if !is_keyword(n)
&& !is_special_name(n)
&& !root_unresolved_references.contains_key(n)
&& (self.options.top_level || !root_bindings.contains_key(n))
&& !(root_bindings.contains_key(n)
&& (!self.options.top_level || exported_names.contains(n)))
{
break name;
}
Expand Down Expand Up @@ -206,14 +219,17 @@ impl Mangler {
fn tally_slot_frequencies(
&self,
symbol_table: &SymbolTable,
exported_symbols: &FxHashSet<SymbolId>,
scope_tree: &ScopeTree,
total_number_of_slots: usize,
slots: &IndexVec<SymbolId, Slot>,
) -> Vec<SlotFrequency> {
let root_scope_id = scope_tree.root_scope_id();
let mut frequencies = vec![SlotFrequency::default(); total_number_of_slots];
for (symbol_id, slot) in slots.iter_enumerated() {
if !self.options.top_level && symbol_table.get_scope_id(symbol_id) == root_scope_id {
if symbol_table.get_scope_id(symbol_id) == root_scope_id
&& (!self.options.top_level || exported_symbols.contains(&symbol_id))
{
continue;
}
if is_special_name(symbol_table.get_name(symbol_id)) {
Expand All @@ -228,6 +244,29 @@ impl Mangler {
frequencies.sort_unstable_by_key(|x| std::cmp::Reverse(x.frequency));
frequencies
}

fn collect_exported_symbols(program: &Program) -> (FxHashSet<CompactStr>, FxHashSet<SymbolId>) {
program
.body
.iter()
.filter_map(|statement| {
let Statement::ExportNamedDeclaration(v) = statement else { return None };
v.declaration.as_ref()
})
.flat_map(|decl| {
if let Declaration::VariableDeclaration(decl) = decl {
itertools::Either::Left(
decl.declarations
.iter()
.filter_map(|decl| decl.id.get_binding_identifier()),
)
} else {
itertools::Either::Right(decl.id().into_iter())
}
})
.map(|id| (id.name.to_compact_str(), id.symbol_id()))
.collect()
}
Copy link
Member

Choose a reason for hiding this comment

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

The best approach is to have a symbol flag for the exports 🤔 but there aren't any...

}

fn is_special_name(name: &str) -> bool {
Expand Down
8 changes: 7 additions & 1 deletion crates/oxc_minifier/tests/mangler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,13 @@ fn mangler() {
"import { x } from 's'; export { x }",
"function _ (exports) { Object.defineProperty(exports, '__esModule', { value: true }) }",
];
let top_level_cases = ["function foo(a) {a}"];
let top_level_cases = [
"function foo(a) {a}",
"export function foo() {}; foo()",
"export default function foo() {}; foo()",
"export const foo = 1; foo",
"const foo = 1; foo; export { foo }",
];

let mut snapshot = String::new();
cases.into_iter().fold(&mut snapshot, |w, case| {
Expand Down
19 changes: 19 additions & 0 deletions crates/oxc_minifier/tests/mangler/snapshots/mangler.snap
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,22 @@ function foo(a) {a}
function a(b) {
b;
}

export function foo() {}; foo()
export function foo() {}
;
foo();

export default function foo() {}; foo()
export default function a() {}
;
a();

export const foo = 1; foo
export const foo = 1;
foo;

const foo = 1; foo; export { foo }
const a = 1;
a;
export { a as foo };
Loading