From 7401a056127646f56a7f85407ce40fad2bbe2c76 Mon Sep 17 00:00:00 2001 From: Yunfei He Date: Thu, 1 Feb 2024 00:46:19 +0800 Subject: [PATCH] fix: keep the context of function call while deconflict symbols --- .../rolldown/src/bundler/utils/finalizer.rs | 92 ++++++++++++------- .../default/es6_from_common_js/artifacts.snap | 2 +- .../import_missing_common_js/artifacts.snap | 2 +- .../nested_es6_from_common_js/artifacts.snap | 2 +- crates/rolldown_oxc/src/ast_snippet.rs | 31 +++++++ .../src/dummy/impl_for_ast_node.rs | 6 ++ 6 files changed, 98 insertions(+), 37 deletions(-) diff --git a/crates/rolldown/src/bundler/utils/finalizer.rs b/crates/rolldown/src/bundler/utils/finalizer.rs index b81f00b1158..97879f7ddc5 100644 --- a/crates/rolldown/src/bundler/utils/finalizer.rs +++ b/crates/rolldown/src/bundler/utils/finalizer.rs @@ -80,6 +80,57 @@ where } None } + + fn finalize_identifier_reference(&self, expr: &mut ast::Expression<'ast>, is_callee: bool) { + let ast::Expression::Identifier(id_ref) = expr else { + return; + }; + let Some(reference_id) = id_ref.reference_id.get() else { + // Some `IdentifierReference`s constructed by bundler don't have `ReferenceId` and we just ignore them. + return; + }; + + let Some(symbol_id) = self.scope.symbol_id_for(reference_id) else { + // we will hit this branch if the reference is for a global variable + return; + }; + + let symbol_ref: SymbolRef = (self.ctx.id, symbol_id).into(); + let canonical_ref = self.ctx.symbols.par_canonical_ref_for(symbol_ref); + let symbol = self.ctx.symbols.get(canonical_ref); + + if let Some(ns_alias) = &symbol.namespace_alias { + let canonical_ns_name = self + .canonical_name_for(ns_alias.namespace_ref) + .expect("namespace alias should have a canonical name"); + let prop_name = &ns_alias.property_name; + if is_callee { + let callee = ast::Expression::MemberExpression( + self + .snippet + .identifier_member_expression(canonical_ns_name.clone(), prop_name.clone()) + .into_in(self.alloc), + ); + let wrapped_callee = self.snippet.seq_expr2(self.snippet.number_expr(0.0), callee); + *expr = wrapped_callee; + } else { + *expr = ast::Expression::MemberExpression( + self + .snippet + .identifier_member_expression(canonical_ns_name.clone(), prop_name.clone()) + .into_in(self.alloc), + ); + } + } else { + if let Some(canonical_name) = self.canonical_name_for(canonical_ref) { + if id_ref.name != canonical_name { + id_ref.name = canonical_name.clone(); + } + } else { + // FIXME: all bindings should have a canonical name + } + } + } } // visit @@ -253,41 +304,14 @@ impl<'ast, 'me: 'ast> VisitMut<'ast> for Finalizer<'me, 'ast> { #[allow(clippy::collapsible_else_if)] fn visit_expression(&mut self, expr: &mut ast::Expression<'ast>) { - if let ast::Expression::Identifier(id_ref) = expr { - if let Some(reference_id) = id_ref.reference_id.get() { - if let Some(symbol_id) = self.scope.symbol_id_for(reference_id) { - let symbol_ref: SymbolRef = (self.ctx.id, symbol_id).into(); - let canonical_ref = self.ctx.symbols.par_canonical_ref_for(symbol_ref); - let symbol = self.ctx.symbols.get(canonical_ref); - - if let Some(ns_alias) = &symbol.namespace_alias { - let canonical_ns_name = self - .canonical_name_for(ns_alias.namespace_ref) - .expect("namespace alias should have a canonical name"); - let prop_name = &ns_alias.property_name; - *expr = ast::Expression::MemberExpression( - self - .snippet - .identifier_member_expression(canonical_ns_name.clone(), prop_name.clone()) - .into_in(self.alloc), - ); - } else { - if let Some(canonical_name) = self.canonical_name_for(canonical_ref) { - if id_ref.name != canonical_name { - id_ref.name = canonical_name.clone(); - } - } else { - // FIXME: all bindings should have a canonical name - } - } - } else { - // we will hit this branch if the reference is for a global variable - }; - } else { - // Some `IdentifierReference`s constructed by bundler don't have `ReferenceId` and we just ignore them. + if let ast::Expression::CallExpression(call_exp) = expr { + if let ast::Expression::Identifier(_) = &mut call_exp.callee { + self.finalize_identifier_reference(&mut call_exp.callee, true); } - } else { - self.visit_expression_match(expr); } + + self.finalize_identifier_reference(expr, false); + + self.visit_expression_match(expr); } } diff --git a/crates/rolldown/tests/esbuild/default/es6_from_common_js/artifacts.snap b/crates/rolldown/tests/esbuild/default/es6_from_common_js/artifacts.snap index d8f599c4406..bb2d43f1587 100644 --- a/crates/rolldown/tests/esbuild/default/es6_from_common_js/artifacts.snap +++ b/crates/rolldown/tests/esbuild/default/es6_from_common_js/artifacts.snap @@ -26,6 +26,6 @@ var require_bar = __commonJS((exports, module) => { // entry.js var import_foo = require_foo(); -console.log(import_foo.foo(), import_bar.bar()); +console.log((0,import_foo.foo)(), (0,import_bar.bar)()); var import_bar = require_bar(); ``` diff --git a/crates/rolldown/tests/esbuild/default/import_missing_common_js/artifacts.snap b/crates/rolldown/tests/esbuild/default/import_missing_common_js/artifacts.snap index c1d3a52f906..b9051a8ce07 100644 --- a/crates/rolldown/tests/esbuild/default/import_missing_common_js/artifacts.snap +++ b/crates/rolldown/tests/esbuild/default/import_missing_common_js/artifacts.snap @@ -18,6 +18,6 @@ var require_foo = __commonJS((exports, module) => { // entry.js var import_foo = require_foo(); if (false) { - console.log(import_foo.default(import_foo.x, import_foo.y)); + console.log((0,import_foo.default)(import_foo.x, import_foo.y)); } ``` diff --git a/crates/rolldown/tests/esbuild/default/nested_es6_from_common_js/artifacts.snap b/crates/rolldown/tests/esbuild/default/nested_es6_from_common_js/artifacts.snap index 2c66d204c0a..801f03a2c2b 100644 --- a/crates/rolldown/tests/esbuild/default/nested_es6_from_common_js/artifacts.snap +++ b/crates/rolldown/tests/esbuild/default/nested_es6_from_common_js/artifacts.snap @@ -20,6 +20,6 @@ var require_foo = __commonJS((exports, module) => { // entry.js var import_foo = require_foo(); (() => { - console.log(import_foo.fn()); + console.log((0,import_foo.fn)()); })(); ``` diff --git a/crates/rolldown_oxc/src/ast_snippet.rs b/crates/rolldown_oxc/src/ast_snippet.rs index 3130e6474d5..caf7869d407 100644 --- a/crates/rolldown_oxc/src/ast_snippet.rs +++ b/crates/rolldown_oxc/src/ast_snippet.rs @@ -169,4 +169,35 @@ impl<'ast> AstSnippet<'ast> { ast::Expression::CallExpression(commonjs_call_expr.into_in(self.alloc)), ) } + + /// ```js + /// (a, b) + /// ``` + pub fn seq_expr2( + &self, + a: ast::Expression<'ast>, + b: ast::Expression<'ast>, + ) -> ast::Expression<'ast> { + let mut expressions = allocator::Vec::new_in(self.alloc); + expressions.push(a); + expressions.push(b); + ast::Expression::SequenceExpression( + ast::SequenceExpression { expressions, ..Dummy::dummy(self.alloc) }.into_in(self.alloc), + ) + } + + /// ```js + /// 42 + /// ``` + pub fn number_expr(&self, value: f64) -> ast::Expression<'ast> { + ast::Expression::NumberLiteral( + ast::NumberLiteral { + span: Dummy::dummy(self.alloc), + value, + raw: self.alloc.alloc(value.to_string()), + base: oxc::syntax::NumberBase::Decimal, + } + .into_in(self.alloc), + ) + } } diff --git a/crates/rolldown_oxc/src/dummy/impl_for_ast_node.rs b/crates/rolldown_oxc/src/dummy/impl_for_ast_node.rs index eb379ad3ce9..b1b61621b0a 100644 --- a/crates/rolldown_oxc/src/dummy/impl_for_ast_node.rs +++ b/crates/rolldown_oxc/src/dummy/impl_for_ast_node.rs @@ -245,3 +245,9 @@ impl<'ast> DummyIn<'ast> for ast::FormalParameter<'ast> { } } } + +impl<'ast> DummyIn<'ast> for ast::SequenceExpression<'ast> { + fn dummy(alloc: &'ast Allocator) -> Self { + Self { span: DummyIn::dummy(alloc), expressions: DummyIn::dummy(alloc) } + } +}