Skip to content

Commit

Permalink
feat: support css url import dependency (#2598)
Browse files Browse the repository at this point in the history
<!-- Thank you for contributing! -->

### Description
1. Use `Span` instead of `module_request_strart` since the span of URL
is more complicated and can't use tricks to reduce memory.
2. try to resolve
https://github.com/rolldown/rolldown/blob/63875e40b7687f9917b70556c23f60e925b111aa/scripts/snap-diff/stats/aggregated-reason.md#L114-L114
<!-- Please insert your description here and provide especially info
about the "what" this PR is solving -->
  • Loading branch information
IWANABETHATGUY authored Nov 5, 2024
1 parent 7c543a4 commit 829d80a
Show file tree
Hide file tree
Showing 33 changed files with 263 additions and 121 deletions.
4 changes: 2 additions & 2 deletions crates/rolldown/src/ast_scanner/impl_visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ impl<'me, 'ast: 'me> Visit<'ast> for AstScanner<'me, 'ast> {
let id = self.add_import_record(
request.value.as_str(),
ImportKind::DynamicImport,
expr.source.span().start,
expr.source.span(),
if expr.source.span().is_empty() {
ImportRecordMeta::IS_UNSPANNED_IMPORT
} else {
Expand Down Expand Up @@ -190,7 +190,7 @@ impl<'me, 'ast: 'me> Visit<'ast> for AstScanner<'me, 'ast> {
let id = self.add_import_record(
request.value.as_str(),
ImportKind::Require,
request.span().start,
request.span(),
if request.span().is_empty() {
ImportRecordMeta::IS_UNSPANNED_IMPORT
} else {
Expand Down
13 changes: 6 additions & 7 deletions crates/rolldown/src/ast_scanner/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ impl<'me, 'ast: 'me> AstScanner<'me, 'ast> {
&mut self,
module_request: &str,
kind: ImportKind,
module_request_start: u32,
span: Span,
init_meta: ImportRecordMeta,
) -> ImportRecordIdx {
// If 'foo' in `import ... from 'foo'` is finally a commonjs module, we will convert the import statement
Expand All @@ -253,9 +253,8 @@ impl<'me, 'ast: 'me> AstScanner<'me, 'ast> {
)
.into(),
);
let rec =
RawImportRecord::new(Rstr::from(module_request), kind, namespace_ref, module_request_start)
.with_meta(init_meta);
let rec = RawImportRecord::new(Rstr::from(module_request), kind, namespace_ref, span)
.with_meta(init_meta);

let id = self.result.import_records.push(rec);
self.current_stmt_info.import_records.push(id);
Expand Down Expand Up @@ -408,7 +407,7 @@ impl<'me, 'ast: 'me> AstScanner<'me, 'ast> {
let id = self.add_import_record(
decl.source.value.as_str(),
ImportKind::Import,
decl.source.span().start,
decl.source.span(),
if decl.source.span().is_empty() {
ImportRecordMeta::IS_UNSPANNED_IMPORT
} else {
Expand All @@ -431,7 +430,7 @@ impl<'me, 'ast: 'me> AstScanner<'me, 'ast> {
let record_id = self.add_import_record(
source.value.as_str(),
ImportKind::Import,
source.span().start,
source.span(),
if source.span().is_empty() {
ImportRecordMeta::IS_UNSPANNED_IMPORT
} else {
Expand Down Expand Up @@ -523,7 +522,7 @@ impl<'me, 'ast: 'me> AstScanner<'me, 'ast> {
let rec_id = self.add_import_record(
decl.source.value.as_str(),
ImportKind::Import,
decl.source.span().start,
decl.source.span(),
if decl.source.span().is_empty() {
ImportRecordMeta::IS_UNSPANNED_IMPORT
} else {
Expand Down
23 changes: 22 additions & 1 deletion crates/rolldown/src/css/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use arcstr::ArcStr;
use oxc::{
index::{Idx, IndexVec},
semantic::SymbolId,
span::Span,
};
use rolldown_common::{
CssRenderer, CssView, ImportKind, ImportRecordIdx, ModuleIdx, RawImportRecord, SymbolRef,
Expand All @@ -18,6 +19,7 @@ pub fn create_css_view(
css_module_lexer::collect_dependencies(source, css_module_lexer::Mode::Css);

let mut dependencies: IndexVec<ImportRecordIdx, RawImportRecord> = IndexVec::default();
let mut record_idx_to_span: IndexVec<ImportRecordIdx, Span> = IndexVec::default();

let mut css_renderer = CssRenderer::default();

Expand All @@ -28,8 +30,9 @@ pub fn create_css_view(
request.into(),
ImportKind::AtImport,
SymbolRef::from((ModuleIdx::from_raw(0), SymbolId::from_usize(0))),
range.start,
Span::new(range.start, range.end),
));
record_idx_to_span.push(Span::new(range.start, range.end));
let mut range_end = range.end as usize;
if source.is_char_boundary(range_end) {
if source[range_end..].starts_with("\r\n") {
Expand All @@ -41,6 +44,23 @@ pub fn create_css_view(
}
css_renderer.at_import_ranges.push((range.start as usize, range_end));
}
css_module_lexer::Dependency::Url { request, range, kind } => {
// css_module_lexer return span of `request` if kind is `string`, return whole span of `url(dep)`, if the kind is function
// so we need to tweak a little to get the correct span we want that used to replace
// asset filename
let span = if matches!(kind, css_module_lexer::UrlRangeKind::String) {
Span::new(range.start + 1, range.end - 1)
} else {
Span::new(range.start + 4 /*length of `url(`*/, range.end - 1)
};
dependencies.push(RawImportRecord::new(
request.into(),
ImportKind::UrlImport,
SymbolRef::from((ModuleIdx::from_raw(0), SymbolId::from_usize(0))),
span,
));
record_idx_to_span.push(span);
}
_ => {}
}
}
Expand All @@ -50,6 +70,7 @@ pub fn create_css_view(
source: source.clone(),
import_records: IndexVec::default(),
mutations: vec![Box::new(css_renderer)],
record_idx_to_span,
},
dependencies,
)
Expand Down
3 changes: 1 addition & 2 deletions crates/rolldown/src/module_loader/module_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use crate::runtime::{RuntimeModuleBrief, RUNTIME_MODULE_ID};
use crate::type_alias::IndexEcmaAst;
use arcstr::ArcStr;
use oxc::index::IndexVec;
use oxc::span::Span;
use oxc::transformer::ReplaceGlobalDefinesConfig;
use rolldown_common::side_effects::{DeterminedSideEffects, HookSideEffects};
use rolldown_common::{
Expand Down Expand Up @@ -261,7 +260,7 @@ impl ModuleLoader {
let owner = ModuleTaskOwner::new(
normal_module.source.clone(),
normal_module.stable_id.as_str().into(),
Span::new(raw_rec.module_request_start, raw_rec.module_request_end()),
raw_rec.span,
);
let id = self.try_spawn_new_task(info, Some(owner));
// Dynamic imported module will be considered as an entry
Expand Down
24 changes: 11 additions & 13 deletions crates/rolldown/src/module_loader/module_task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,13 +200,17 @@ impl ModuleTask {
return Ok(());
}
};

if !matches!(module_type, ModuleType::Css) {
for (record, info) in raw_import_records.iter().zip(&resolved_deps) {
if record.kind.is_static() {
ecma_view.imported_ids.push(ArcStr::clone(&info.id).into());
} else {
ecma_view.dynamically_imported_ids.push(ArcStr::clone(&info.id).into());
match record.kind {
ImportKind::Import | ImportKind::Require => {
ecma_view.imported_ids.push(ArcStr::clone(&info.id).into());
}
ImportKind::DynamicImport => {
ecma_view.dynamically_imported_ids.push(ArcStr::clone(&info.id).into());
}
// for a none css module, we should not have `at-import` or `url-import`
ImportKind::AtImport | ImportKind::UrlImport => unreachable!(),
}
}
}
Expand Down Expand Up @@ -326,10 +330,7 @@ impl ModuleTask {
if dep.is_unspanned() || is_css_module {
DiagnosableArcstr::String(specifier.as_str().into())
} else {
DiagnosableArcstr::Span(Span::new(
dep.module_request_start,
dep.module_request_end(),
))
DiagnosableArcstr::Span(dep.state.span)
},
"Module not found, treating it as an external dependency".into(),
Some("UNRESOLVED_IMPORT"),
Expand All @@ -353,10 +354,7 @@ impl ModuleTask {
if dep.is_unspanned() || is_css_module {
DiagnosableArcstr::String(specifier.as_str().into())
} else {
DiagnosableArcstr::Span(Span::new(
dep.module_request_start,
dep.module_request_end(),
))
DiagnosableArcstr::Span(dep.state.span)
},
reason,
None,
Expand Down
26 changes: 23 additions & 3 deletions crates/rolldown/src/stages/generate_stage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ use rolldown_std_utils::OptionExt;
use rustc_hash::{FxHashMap, FxHashSet};

use rolldown_common::{
ChunkIdx, ChunkKind, FileNameRenderOptions, ImportMetaRolldownAssetReplacer, Module,
PreliminaryFilename,
ChunkIdx, ChunkKind, CssAssetNameReplacer, FileNameRenderOptions,
ImportMetaRolldownAssetReplacer, Module, PreliminaryFilename,
};
use rolldown_plugin::SharedPluginDriver;
use rolldown_utils::{
Expand Down Expand Up @@ -293,13 +293,33 @@ impl<'a> GenerateStage<'a> {

pub fn patch_asset_modules(&mut self, chunk_graph: &ChunkGraph) {
chunk_graph.chunk_table.iter().for_each(|chunk| {
let mut module_idx_to_filenames = FxHashMap::default();
// replace asset name in ecma view
chunk.asset_preliminary_filenames.iter().for_each(|(module_idx, preliminary)| {
let Module::Normal(module) = &mut self.link_output.module_table.modules[*module_idx] else {
return;
};
let asset_filename: ArcStr = preliminary.as_str().into();
module.ecma_view.mutations.push(Box::new(ImportMetaRolldownAssetReplacer {
asset_filename: preliminary.to_string(),
asset_filename: asset_filename.clone(),
}));
module_idx_to_filenames.insert(module_idx, asset_filename);
});
// replace asset name in css view
chunk.modules.iter().for_each(|module_idx| {
let module = &mut self.link_output.module_table.modules[*module_idx];
if let Some(css_view) =
module.as_normal_mut().and_then(|normal_module| normal_module.css_view.as_mut())
{
for (idx, record) in css_view.import_records.iter_enumerated() {
if let Some(asset_filename) = module_idx_to_filenames.get(&record.resolved_module) {
let span = css_view.record_idx_to_span[idx];
css_view
.mutations
.push(Box::new(CssAssetNameReplacer { span, asset_name: asset_filename.clone() }));
}
}
}
});
});
}
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 @@ -227,6 +227,9 @@ impl<'a> LinkStage<'a> {
ImportKind::AtImport => {
unreachable!("A Js module would never import a CSS module via `@import`");
}
ImportKind::UrlImport => {
unreachable!("A Js module would never import a CSS module via `url()`");
}
}
});

Expand Down Expand Up @@ -455,6 +458,9 @@ impl<'a> LinkStage<'a> {
ImportKind::AtImport => {
unreachable!("A Js module would never import a CSS module via `@import`");
}
ImportKind::UrlImport => {
unreachable!("A Js module would never import a CSS module via `url()`");
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@
"name": "entry",
"import": "entry.css"
}
]
],
"moduleTypes": {
".css": "css"
}
},
"expectExecuted": false
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,21 @@ snapshot_kind: text
```
## UNRESOLVED_IMPORT

```text
[UNRESOLVED_IMPORT] Warning: Could not resolve ./file2 in entry.css
```
## UNRESOLVED_IMPORT

```text
[UNRESOLVED_IMPORT] Warning: Could not resolve pkg in entry.css
```
## UNRESOLVED_IMPORT

```text
[UNRESOLVED_IMPORT] Warning: Could not resolve pkg2 in entry.css
```
# Assets

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@
"name": "entry",
"import": "entry.css"
}
]
],
"moduleTypes": {
".css": "css",
".file": "asset",
".svg": "dataurl"
}
},
"expectExecuted": false
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,34 @@ snapshot_kind: text
```text
[UNRESOLVED_IMPORT] Warning: Could not resolve extern.css in entry.css
```
## UNRESOLVED_IMPORT

```text
[UNRESOLVED_IMPORT] Warning: Could not resolve extern.png in entry.css
```
# Assets

## assets/file-JEFwJEkD.file

## copy.js

```js
//#region file.file
var file_default = "assets/file-JEFwJEkD.file";
//#endregion
//#region copy.copy
var require_copy = __commonJS({ "copy.copy"() {
copy;
} });
//#endregion
export { __commonJS, __require, __toESM, file_default, require_copy };
```
## dynamic.js

```js
Expand All @@ -49,28 +74,16 @@ export { dynamic_default as default };
## entry.js

```js
import { __commonJS, __require, __toESM, file_default, require_copy } from "./copy.js";
import a from "extern-esm";
//#region esm.js
var esm_default = 1;
//#endregion
//#region <data:application/json,2>
var json_2_default = 2;
//#endregion
//#region file.file
var require_file = __commonJS({ "file.file"() {
file;
} });
//#endregion
//#region copy.copy
var require_copy = __commonJS({ "copy.copy"() {
copy;
} });
//#endregion
//#region cjs.js
var require_cjs = __commonJS({ "cjs.js"(exports, module) {
Expand All @@ -79,9 +92,8 @@ var require_cjs = __commonJS({ "cjs.js"(exports, module) {
//#endregion
//#region entry.js
var import_file = __toESM(require_file());
var import_copy = __toESM(require_copy());
console.log(a, esm_default, json_2_default, import_file.default, import_copy.default, __require("extern-cjs"), require_cjs(), import("./dynamic.js"));
console.log(a, esm_default, json_2_default, file_default, import_copy.default, __require("extern-cjs"), require_cjs(), import("./dynamic.js"));
let exported;
//#endregion
Expand All @@ -99,5 +111,6 @@ d { background: url(extern.png) }
## entry2.js

```js
import "./copy.js";
```
Loading

0 comments on commit 829d80a

Please sign in to comment.