Skip to content

Commit

Permalink
fix(cjs): should keep correct esm semantic in importing external modules
Browse files Browse the repository at this point in the history
  • Loading branch information
hyf0 committed May 19, 2024
1 parent f102cc0 commit bfc53db
Show file tree
Hide file tree
Showing 12 changed files with 94 additions and 21 deletions.
24 changes: 15 additions & 9 deletions crates/rolldown/src/ast_scanner/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,8 +297,10 @@ impl<'me> AstScanner<'me> {
let record_id = self.add_import_record(&source.value, ImportKind::Import);
decl.specifiers.iter().for_each(|spec| {
self.add_re_export(spec.exported.name(), spec.local.name(), record_id);
self.result.imports.insert(decl.span, record_id);
});
self.result.imports.insert(decl.span, record_id);
// `export {} from '...'`
self.result.import_records[record_id].is_plain_import = decl.specifiers.is_empty();
} else {
decl.specifiers.iter().for_each(|spec| {
self.add_local_export(spec.exported.name(), self.get_root_binding(spec.local.name()));
Expand Down Expand Up @@ -358,25 +360,29 @@ impl<'me> AstScanner<'me> {
}

fn scan_import_decl(&mut self, decl: &ImportDeclaration) {
let id = self.add_import_record(&decl.source.value, ImportKind::Import);
self.result.imports.insert(decl.span, id);
let rec_id = self.add_import_record(&decl.source.value, ImportKind::Import);
self.result.imports.insert(decl.span, rec_id);
// // `import '...'` or `import {} from '...'`
self.result.import_records[rec_id].is_plain_import =
decl.specifiers.as_ref().map_or(true, |s| s.is_empty());

let Some(specifiers) = &decl.specifiers else { return };
specifiers.iter().for_each(|spec| match spec {
oxc::ast::ast::ImportDeclarationSpecifier::ImportSpecifier(spec) => {
let sym = spec.local.expect_symbol_id();
let imported = spec.imported.name();
self.add_named_import(sym, imported, id);
self.add_named_import(sym, imported, rec_id);
if imported == "default" {
self.result.import_records[id].contains_import_default = true;
self.result.import_records[rec_id].contains_import_default = true;
}
}
oxc::ast::ast::ImportDeclarationSpecifier::ImportDefaultSpecifier(spec) => {
self.add_named_import(spec.local.expect_symbol_id(), "default", id);
self.result.import_records[id].contains_import_default = true;
self.add_named_import(spec.local.expect_symbol_id(), "default", rec_id);
self.result.import_records[rec_id].contains_import_default = true;
}
oxc::ast::ast::ImportDeclarationSpecifier::ImportNamespaceSpecifier(spec) => {
self.add_star_import(spec.local.expect_symbol_id(), id);
self.result.import_records[id].contains_import_star = true;
self.add_star_import(spec.local.expect_symbol_id(), rec_id);
self.result.import_records[rec_id].contains_import_star = true;
}
});
}
Expand Down
6 changes: 6 additions & 0 deletions crates/rolldown/src/stages/link_stage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,12 @@ impl<'a> LinkStage<'a> {
ModuleId::External(_) => {
// Make sure symbols from external modules are included and de_conflicted
stmt_info.side_effect = true;
if matches!(self.input_options.format, OutputFormat::Cjs)
&& matches!(rec.kind, ImportKind::Import)
&& !rec.is_plain_import
{
stmt_info.referenced_symbols.push(self.runtime.resolve_symbol("__toESM"));
}
}
ModuleId::Normal(importee_id) => {
let importee_linking_info = &self.metas[importee_id];
Expand Down
31 changes: 28 additions & 3 deletions crates/rolldown/src/utils/chunk/render_chunk_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,12 @@ pub fn render_chunk_imports(
});

// render external imports

let imports_from_external_modules = &chunk.imports_from_external_modules;

if imports_from_external_modules.is_empty() {
return s;
}

imports_from_external_modules.iter().for_each(|(importee_id, named_imports)| {
let importee = &graph.module_table.external_modules[*importee_id];
let mut is_importee_imported = false;
Expand All @@ -106,7 +110,11 @@ pub fn render_chunk_imports(
s.push_str(&format!("import * as {alias} from \"{importee_name}\";\n",));
}
rolldown_common::OutputFormat::Cjs => {
s.push_str(&format!("const {alias} = require(\"{importee_name}\");\n",));
let to_esm_fn_name = &chunk.canonical_names
[&graph.symbols.par_canonical_ref_for(graph.runtime.resolve_symbol("__toESM"))];
s.push_str(&format!(
"const {alias} = {to_esm_fn_name}(require(\"{importee_name}\"));\n",
));
}
}

Expand All @@ -118,7 +126,24 @@ pub fn render_chunk_imports(
.collect::<Vec<_>>();
import_items.sort();
if !import_items.is_empty() {
render_import_stmt(&import_items, &importee.name, &mut s);
match options.format {
rolldown_common::OutputFormat::Esm => {
s.push_str(&format!(
"import {{ {} }} from \"{importee_module_specifier}\";\n",
import_items.join(", "),
importee_module_specifier = &importee.name
));
}
rolldown_common::OutputFormat::Cjs => {
let to_esm_fn_name = &chunk.canonical_names
[&graph.symbols.par_canonical_ref_for(graph.runtime.resolve_symbol("__toESM"))];
s.push_str(&format!(
"const {{ {} }} = {to_esm_fn_name}(require(\"{importee_module_specifier}\"));\n",
import_items.join(", "),
importee_module_specifier = &importee.name
));
}
}
} else if !is_importee_imported {
// Ensure the side effect
render_plain_import(&importee.name, &mut s);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,5 @@
],
"format": "cjs",
"external": ["fs"]
},
"_comment": "TODO: support excution of cjs format",
"expectExecuted": false
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ input_file: crates/rolldown/tests/esbuild/default/import_fs_node_common_js
## entry_js.cjs

```js
const fs = require("fs");
const { default: defaultValue, readFileSync } = require("fs");
const { __toESM } = require("./$runtime$.cjs");
const fs = __toESM(require("fs"));
const { default: defaultValue, readFileSync } = __toESM(require("fs"));
// entry.js
console.log(fs, readFileSync, defaultValue);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ input_file: crates/rolldown/tests/esbuild/import_star/re_export_star_as_external
## entry_js.cjs

```js
const out = require("foo");
const { __toESM } = require("./$runtime$.cjs");
const out = __toESM(require("foo"));
exports.out = out;
```
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,6 @@ input_file: crates/rolldown/tests/esbuild/import_star/re_export_star_external_co
## entry_js.cjs

```js
const { __toESM } = require("./$runtime$.cjs");
require("foo");
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"config": {
"format": "cjs",
"external": ["foo", "bar", "baz"]
},
"expectExecuted": false
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
source: crates/rolldown/tests/common/case.rs
expression: content
input_file: crates/rolldown/tests/fixtures/function/format/cjs/plain_import_should_not_introduce_to_esm
---
# Assets

## main.cjs

```js
require("foo");
require("bar");
require("baz");
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import 'foo'
import {} from 'bar'
export {} from 'baz'
13 changes: 10 additions & 3 deletions crates/rolldown/tests/snapshots/fixtures__filename_with_hash.snap
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,8 @@ expression: "snapshot_outputs.join(\"\\n\")"

# tests/esbuild/default/import_fs_node_common_js

- entry_js-!~{000}~.cjs => entry_js-TE_F6vZL.cjs
- $runtime$-!~{001}~.cjs => $runtime$-OBYYRk4K.cjs
- entry_js-!~{000}~.cjs => entry_js-98OoslfP.cjs

# tests/esbuild/default/import_missing_common_js

Expand Down Expand Up @@ -531,7 +532,8 @@ expression: "snapshot_outputs.join(\"\\n\")"

# tests/esbuild/import_star/re_export_star_as_external_common_js

- entry_js-!~{000}~.cjs => entry_js-xqfVgBNO.cjs
- $runtime$-!~{001}~.cjs => $runtime$-OBYYRk4K.cjs
- entry_js-!~{000}~.cjs => entry_js-lO9-PfCm.cjs

# tests/esbuild/import_star/re_export_star_as_external_es6

Expand All @@ -544,7 +546,8 @@ expression: "snapshot_outputs.join(\"\\n\")"

# tests/esbuild/import_star/re_export_star_external_common_js

- entry_js-!~{000}~.cjs => entry_js-rud5zVrH.cjs
- $runtime$-!~{001}~.cjs => $runtime$-OBYYRk4K.cjs
- entry_js-!~{000}~.cjs => entry_js-XWUyJeqD.cjs

# tests/esbuild/import_star/re_export_star_external_es6

Expand Down Expand Up @@ -1240,6 +1243,10 @@ expression: "snapshot_outputs.join(\"\\n\")"
- main-!~{000}~.mjs => main-LeJVwZoF.mjs
- share-!~{002}~.mjs => share-eIGIrVeP.mjs

# tests/fixtures/function/format/cjs/plain_import_should_not_introduce_to_esm

- main-!~{000}~.cjs => main-rClOc9Ow.cjs

# tests/fixtures/function/resolve/browser_filed_false

- $runtime$-!~{001}~.mjs => $runtime$-aUKPtNTY.mjs
Expand Down
4 changes: 4 additions & 0 deletions crates/rolldown_common/src/types/import_record.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ pub struct RawImportRecord {
pub namespace_ref: SymbolRef,
pub contains_import_star: bool,
pub contains_import_default: bool,
pub is_plain_import: bool,
}

impl RawImportRecord {
Expand All @@ -62,6 +63,7 @@ impl RawImportRecord {
namespace_ref,
contains_import_default: false,
contains_import_star: false,
is_plain_import: false,
}
}

Expand All @@ -73,6 +75,7 @@ impl RawImportRecord {
namespace_ref: self.namespace_ref,
contains_import_star: self.contains_import_star,
contains_import_default: self.contains_import_default,
is_plain_import: self.is_plain_import,
}
}
}
Expand All @@ -86,4 +89,5 @@ pub struct ImportRecord {
pub namespace_ref: SymbolRef,
pub contains_import_star: bool,
pub contains_import_default: bool,
pub is_plain_import: bool,
}

0 comments on commit bfc53db

Please sign in to comment.