Skip to content

Commit

Permalink
fix: rewrite unresolved symbol reference callee to indirect call (#94)
Browse files Browse the repository at this point in the history
  • Loading branch information
underfin committed Oct 25, 2023
1 parent 5b980d5 commit 6284028
Show file tree
Hide file tree
Showing 9 changed files with 61 additions and 22 deletions.
10 changes: 9 additions & 1 deletion crates/rolldown/src/bundler/visitors/commonjs_source_render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,15 @@ impl<'ast> Visit<'ast> for CommonJsSourceRender<'ast> {
for arg in &expr.arguments {
self.visit_argument(arg);
}
self.visit_expression(&expr.callee);
if let oxc::ast::ast::Expression::Identifier(s) = &expr.callee {
self.ctx.visit_identifier_reference(s, true);
} else {
self.visit_expression(&expr.callee);
}
}

fn visit_identifier_reference(&mut self, ident: &'ast oxc::ast::ast::IdentifierReference) {
self.ctx.visit_identifier_reference(ident, false);
}

fn visit_import_declaration(&mut self, decl: &'ast oxc::ast::ast::ImportDeclaration<'ast>) {
Expand Down
8 changes: 6 additions & 2 deletions crates/rolldown/src/bundler/visitors/esm_source_render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ impl<'ast> Visit<'ast> for EsmSourceRender<'ast> {
}

fn visit_identifier_reference(&mut self, ident: &'ast oxc::ast::ast::IdentifierReference) {
self.ctx.visit_identifier_reference(ident);
self.ctx.visit_identifier_reference(ident, false);
}

fn visit_import_declaration(&mut self, decl: &'ast oxc::ast::ast::ImportDeclaration<'ast>) {
Expand Down Expand Up @@ -106,7 +106,11 @@ impl<'ast> Visit<'ast> for EsmSourceRender<'ast> {
for arg in &expr.arguments {
self.visit_argument(arg);
}
self.visit_expression(&expr.callee);
if let oxc::ast::ast::Expression::Identifier(s) = &expr.callee {
self.ctx.visit_identifier_reference(s, true);
} else {
self.visit_expression(&expr.callee);
}
}

fn visit_statement(&mut self, stmt: &'ast oxc::ast::ast::Statement<'ast>) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ impl<'ast> Visit<'ast> for EsmWrapSourceRender<'ast> {
}

fn visit_identifier_reference(&mut self, ident: &'ast oxc::ast::ast::IdentifierReference) {
self.ctx.visit_identifier_reference(ident);
self.ctx.visit_identifier_reference(ident, false);
}

fn visit_import_declaration(&mut self, decl: &'ast oxc::ast::ast::ImportDeclaration<'ast>) {
Expand Down Expand Up @@ -189,7 +189,11 @@ impl<'ast> Visit<'ast> for EsmWrapSourceRender<'ast> {
for arg in &expr.arguments {
self.visit_argument(arg);
}
self.visit_expression(&expr.callee);
if let oxc::ast::ast::Expression::Identifier(s) = &expr.callee {
self.ctx.visit_identifier_reference(s, true);
} else {
self.visit_expression(&expr.callee);
}
}

fn visit_statement(&mut self, stmt: &'ast oxc::ast::ast::Statement<'ast>) {
Expand Down
38 changes: 26 additions & 12 deletions crates/rolldown/src/bundler/visitors/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,17 @@ impl<'ast> RendererContext<'ast> {
}
}

pub fn visit_identifier_reference(&mut self, ident: &'ast oxc::ast::ast::IdentifierReference) {
pub fn visit_identifier_reference(
&mut self,
ident: &'ast oxc::ast::ast::IdentifierReference,
is_call: bool,
) {
if let Some(symbol_id) =
self.graph.symbols.references_table[self.module.id][ident.reference_id.get().unwrap()]
{
// If import symbol from commonjs, the reference of the symbol is not resolved,
// Here need write it to property access. eg `import { a } from 'cjs'; console.log(a)` => `console.log(cjs_ns.a)`
// Note: we should rewrite call expression to indirect call, eg `import { a } from 'cjs'; console.log(a())` => `console.log((0, cjs_ns.a)())`
let symbol_ref = (self.module.id, symbol_id).into();
if let Some(unresolved_symbol) = self.linker_module.unresolved_symbols.get(&symbol_ref) {
let importee_namespace_symbol_name = get_symbol_final_name(
Expand All @@ -161,17 +168,24 @@ impl<'ast> RendererContext<'ast> {
self.final_names,
)
.unwrap();
self.source.update(
ident.span.start,
ident.span.end,
format!(
"{importee_namespace_symbol_name}{}",
unresolved_symbol
.reference_name
.as_ref()
.map_or_else(String::new, |name| format!(".{name}"))
),
);
if let Some(reference_name) = &unresolved_symbol.reference_name {
let property_access = format!("{importee_namespace_symbol_name}.{reference_name}",);
if is_call {
self.source.update(
ident.span.start,
ident.span.end,
format!("(0, {property_access})",),
);
} else {
self.source.update(ident.span.start, ident.span.end, property_access);
}
} else if ident.name != importee_namespace_symbol_name {
self.source.update(
ident.span.start,
ident.span.end,
importee_namespace_symbol_name.to_string(),
);
}
} else if let Some(name) = self.get_symbol_final_name(symbol_ref) {
if ident.name != name {
self.rename_symbol(ident.span, name.clone());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,6 @@ exports.foo = function() {
var foo_ns = __toESM(require_foo());
var bar_ns = __toESM(require_bar());
console.log(foo_ns.foo(), bar_ns.bar())
console.log((0, foo_ns.foo)(), (0, bar_ns.bar)())
// This should be hoisted
```
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@ exports.fn = function() {
var foo_ns = __toESM(require_foo());
(() => {
console.log(foo_ns.fn())
console.log((0, foo_ns.fn)())
})()
```
9 changes: 8 additions & 1 deletion crates/rolldown/tests/fixtures/mix-cjs-esm/artifacts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,19 @@ var foo_ns = {
var init_foo = __esm({
'foo.js'() {
}
});
// cjs.js
var require_cjs = __commonJS({
'cjs.js'(exports, module) {
module.exports = 1;
}
});
// esm_import_cjs_require.js
init_foo();
var cjs_ns = __toESM(require_cjs());
(init_foo(), __toCommonJS(foo_ns))
console.log(cjs_ns.a)
// esm_import_cjs_export.js
var require_esm_import_cjs_export = __commonJS({
'esm_import_cjs_export.js'(exports, module) {
Expand Down
1 change: 1 addition & 0 deletions crates/rolldown/tests/fixtures/mix-cjs-esm/cjs.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = 1;
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
import './foo'
require('./foo')
import { a } from './cjs'
require('./foo')
console.log(a)

0 comments on commit 6284028

Please sign in to comment.