Skip to content

Commit

Permalink
fix: support lazy css using css-extract (#6747)
Browse files Browse the repository at this point in the history
  • Loading branch information
JSerFeng committed Jun 6, 2024
1 parent 354cc6f commit 7e0ec16
Show file tree
Hide file tree
Showing 9 changed files with 93 additions and 14 deletions.
4 changes: 2 additions & 2 deletions crates/rspack_core/src/compiler/module_executor/ctrl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ impl Task<MakeTaskContext> for CtrlTask {
Event::ExecuteModule(param, execute_task) => {
let dep_id = match &param {
EntryParam::DependencyId(id, _) => *id,
EntryParam::EntryDependency(dep) => *dep.id(),
EntryParam::Entry(dep) => *dep.id(),
};
self.execute_task_map.insert(dep_id, execute_task);
return Ok(vec![Box::new(EntryTask { param }), self]);
Expand Down Expand Up @@ -238,7 +238,7 @@ impl Task<MakeTaskContext> for FinishModuleTask {
Event::ExecuteModule(param, execute_task) => {
let dep_id = match &param {
EntryParam::DependencyId(id, _) => *id,
EntryParam::EntryDependency(dep) => *dep.id(),
EntryParam::Entry(dep) => *dep.id(),
};
ctrl_task.execute_task_map.insert(dep_id, execute_task);
res.push(Box::new(EntryTask { param }));
Expand Down
13 changes: 9 additions & 4 deletions crates/rspack_core/src/compiler/module_executor/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ use super::ctrl::Event;
use crate::{
compiler::make::repair::{factorize::FactorizeTask, MakeTaskContext},
utils::task_loop::{Task, TaskResult, TaskType},
Dependency, DependencyId, EntryDependency, ModuleProfile,
Dependency, DependencyId, LoaderImportDependency, ModuleProfile,
};

#[derive(Debug)]
pub enum EntryParam {
DependencyId(DependencyId, UnboundedSender<Event>),
EntryDependency(Box<EntryDependency>),
Entry(Box<LoaderImportDependency>),
}

#[derive(Debug)]
Expand Down Expand Up @@ -39,14 +39,19 @@ impl Task<MakeTaskContext> for EntryTask {
}
Ok(vec![])
}
EntryParam::EntryDependency(dep) => {
EntryParam::Entry(dep) => {
let dep_id = *dep.id();
module_graph.add_dependency(dep.clone());
Ok(vec![Box::new(FactorizeTask {
module_factory: context
.dependency_factories
.get(dep.dependency_type())
.expect("should have dependency_factories")
.unwrap_or_else(|| {
panic!(
"should have dependency_factories for dependency_type: {}",
dep.dependency_type()
)
})
.clone(),
original_module_identifier: None,
original_module_source: None,
Expand Down
7 changes: 3 additions & 4 deletions crates/rspack_core/src/compiler/module_executor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use self::{
use super::make::{repair::MakeTaskContext, update_module_graph, MakeArtifact, MakeParam};
use crate::{
task_loop::run_task_loop_with_event, Compilation, CompilationAsset, Context, Dependency,
DependencyId, EntryDependency,
DependencyId, LoaderImportDependency,
};

#[derive(Debug, Default)]
Expand Down Expand Up @@ -123,14 +123,13 @@ impl ModuleExecutor {
.expect("should have event sender");
let (param, dep_id) = match self.request_dep_map.entry(request.clone()) {
Entry::Vacant(v) => {
let dep = EntryDependency::new(
let dep = LoaderImportDependency::new(
request.clone(),
original_module_context.unwrap_or(Context::from("")),
false,
);
let dep_id = *dep.id();
v.insert(dep_id);
(EntryParam::EntryDependency(Box::new(dep)), dep_id)
(EntryParam::Entry(Box::new(dep)), dep_id)
}
Entry::Occupied(v) => {
let dep_id = *v.get();
Expand Down
52 changes: 52 additions & 0 deletions crates/rspack_core/src/dependency/loader_import.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
use crate::{
AsContextDependency, AsDependencyTemplate, Context, Dependency, DependencyCategory, DependencyId,
DependencyType, ModuleDependency,
};

#[derive(Debug, Hash, PartialEq, Eq, Clone)]
pub struct LoaderImportDependency {
id: DependencyId,
context: Context,
request: String,
}

impl LoaderImportDependency {
pub fn new(request: String, context: Context) -> Self {
Self {
request,
context,
id: DependencyId::new(),
}
}
}

impl AsDependencyTemplate for LoaderImportDependency {}
impl AsContextDependency for LoaderImportDependency {}

impl Dependency for LoaderImportDependency {
fn id(&self) -> &DependencyId {
&self.id
}

fn category(&self) -> &DependencyCategory {
&DependencyCategory::LoaderImport
}

fn dependency_type(&self) -> &DependencyType {
&DependencyType::LoaderImport
}
}

impl ModuleDependency for LoaderImportDependency {
fn request(&self) -> &str {
&self.request
}

fn user_request(&self) -> &str {
&self.request
}

fn set_request(&mut self, request: String) {
self.request = request;
}
}
2 changes: 2 additions & 0 deletions crates/rspack_core/src/dependency/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ mod dependency_trait;
mod dependency_type;
mod entry;
mod import_dependency_trait;
mod loader_import;
mod module_dependency;
mod runtime_requirements_dependency;
mod runtime_template;
Expand All @@ -29,6 +30,7 @@ pub use dependency_trait::*;
pub use dependency_type::DependencyType;
pub use entry::*;
pub use import_dependency_trait::ImportDependencyTrait;
pub use loader_import::*;
pub use module_dependency::*;
pub use runtime_requirements_dependency::RuntimeRequirementsDependency;
pub use runtime_template::*;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,11 @@ async fn compilation(
DependencyType::Provided,
params.normal_module_factory.clone(),
);
// ImportModule
compilation.set_dependency_factory(
DependencyType::LoaderImport,
params.normal_module_factory.clone(),
);
// other
compilation.set_dependency_factory(
DependencyType::WebpackIsIncluded,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,9 @@ import { expect, test } from "@/fixtures";
test("should load success", async ({ page }) => {
await page.getByText("Click me").click();
await expect(page).toHaveURL(/success/);
const body = await page.$("body");
const backgroundColor = await body!.evaluate(
el => window.getComputedStyle(el).backgroundColor
);
expect(backgroundColor, "blue");
});
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,24 @@ module.exports = {
main: [
// Will trigger the issue.
'data:text/javascript,import "core-js";',
"./src/index.css",
"./src/index.js"
]
},
stats: "none",
mode: "development",
plugins: [new rspack.HtmlRspackPlugin()],
module: {
rules: [
{
test: /\.css$/,
use: [rspack.CssExtractRspackPlugin.loader, "css-loader"]
}
]
},
plugins: [new rspack.HtmlRspackPlugin(), new rspack.CssExtractRspackPlugin()],
experiments: {
lazyCompilation: {
entries: true
}
css: false,
lazyCompilation: true
},
devServer: {
hot: true
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
html {
background-color: blue;
}

2 comments on commit 7e0ec16

@rspack-bot
Copy link

Choose a reason for hiding this comment

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

📝 Benchmark detail: Open

Name Base (2024-06-06 6ace5d3) Current Change
10000_development-mode + exec 2.23 s ± 27 ms 2.23 s ± 22 ms -0.14 %
10000_development-mode_hmr + exec 734 ms ± 12 ms 738 ms ± 10 ms +0.45 %
10000_production-mode + exec 2.57 s ± 29 ms 2.59 s ± 28 ms +0.84 %
arco-pro_development-mode + exec 1.92 s ± 60 ms 1.92 s ± 72 ms +0.10 %
arco-pro_development-mode_hmr + exec 441 ms ± 2.6 ms 441 ms ± 2.1 ms +0.04 %
arco-pro_production-mode + exec 3.55 s ± 53 ms 3.49 s ± 82 ms -1.86 %
threejs_development-mode_10x + exec 1.4 s ± 20 ms 1.41 s ± 14 ms +0.56 %
threejs_development-mode_10x_hmr + exec 817 ms ± 3.7 ms 810 ms ± 8.7 ms -0.88 %
threejs_production-mode_10x + exec 4.71 s ± 23 ms 4.72 s ± 17 ms +0.28 %

@rspack-bot
Copy link

Choose a reason for hiding this comment

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

📝 Ran ecosystem CI: Open

suite result
modernjs ✅ success
_selftest ✅ success
nx ✅ success
rspress ✅ success
rsbuild ✅ success
compat ✅ success
examples ✅ success

Please sign in to comment.