Skip to content

Commit

Permalink
fix: extract-css should pass cacheable (#6707)
Browse files Browse the repository at this point in the history
* fix: extract-css should pass cacheable

* test: pass through cacheable (#6711)

* test: pass cachable in css extract

* test: pass cachable in css extract

---------

Co-authored-by: harpsealjs <lingyucoder@gmail.com>
  • Loading branch information
JSerFeng and LingyuCoder committed Jun 5, 2024
1 parent fdc7685 commit 9e5732c
Show file tree
Hide file tree
Showing 27 changed files with 598 additions and 11 deletions.
1 change: 1 addition & 0 deletions crates/node_binding/binding.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,7 @@ export interface JsExecuteModuleResult {
contextDependencies: Array<string>
buildDependencies: Array<string>
missingDependencies: Array<string>
cacheable: boolean
assets: Array<string>
id: number
}
Expand Down
2 changes: 2 additions & 0 deletions crates/rspack_binding_values/src/compilation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,7 @@ impl JsCompilation {
match result {
Ok(res) => {
let js_result = JsExecuteModuleResult {
cacheable: res.cacheable,
file_dependencies: res
.file_dependencies
.into_iter()
Expand Down Expand Up @@ -513,6 +514,7 @@ pub struct JsExecuteModuleResult {
pub context_dependencies: Vec<String>,
pub build_dependencies: Vec<String>,
pub missing_dependencies: Vec<String>,
pub cacheable: bool,
pub assets: Vec<String>,
pub id: u32,
}
Expand Down
16 changes: 12 additions & 4 deletions crates/rspack_core/src/compiler/module_executor/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ pub type ExecuteModuleId = u32;

#[derive(Debug, Default)]
pub struct ExecuteModuleResult {
pub cacheable: bool,
pub file_dependencies: HashSet<std::path::PathBuf>,
pub context_dependencies: HashSet<std::path::PathBuf>,
pub missing_dependencies: HashSet<std::path::PathBuf>,
Expand Down Expand Up @@ -203,9 +204,12 @@ impl Task<MakeTaskContext> for ExecuteTask {
let module_graph = compilation.get_module_graph();
let mut execute_result = match exports {
Ok(_) => {
let mut result = modules
.iter()
.fold(ExecuteModuleResult::default(), |mut res, m| {
let mut result = modules.iter().fold(
ExecuteModuleResult {
cacheable: true,
..Default::default()
},
|mut res, m| {
let module = module_graph.module_by_identifier(m).expect("unreachable");

let build_info = &module.build_info();
Expand All @@ -222,9 +226,13 @@ impl Task<MakeTaskContext> for ExecuteTask {
res
.build_dependencies
.extend(info.build_dependencies.iter().cloned());
if !info.cacheable {
res.cacheable = false;
}
}
res
});
},
);

result.id = id;

Expand Down
4 changes: 3 additions & 1 deletion crates/rspack_plugin_extract_css/src/css_dependency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub struct CssDependency {
pub(crate) order_index: u32,

resource_identifier: String,

pub(crate) cacheable: bool,
pub(crate) file_dependencies: FxHashSet<PathBuf>,
pub(crate) context_dependencies: FxHashSet<PathBuf>,
pub(crate) missing_dependencies: FxHashSet<PathBuf>,
Expand All @@ -43,6 +43,7 @@ impl CssDependency {
source_map: String,
identifier_index: u32,
order_index: u32,
cacheable: bool,
file_dependencies: FxHashSet<PathBuf>,
context_dependencies: FxHashSet<PathBuf>,
missing_dependencies: FxHashSet<PathBuf>,
Expand All @@ -60,6 +61,7 @@ impl CssDependency {
identifier_index,
order_index,
resource_identifier,
cacheable,
file_dependencies,
context_dependencies,
missing_dependencies,
Expand Down
4 changes: 3 additions & 1 deletion crates/rspack_plugin_extract_css/src/css_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ pub(crate) struct CssModule {
dependencies: Vec<DependencyId>,

identifier__: Identifier,

cacheable: bool,
file_dependencies: FxHashSet<PathBuf>,
context_dependencies: FxHashSet<PathBuf>,
missing_dependencies: FxHashSet<PathBuf>,
Expand Down Expand Up @@ -84,6 +84,7 @@ impl CssModule {
build_meta: None,
source_map_kind: rspack_util::source_map::SourceMapKind::empty(),
identifier__,
cacheable: dep.cacheable,
file_dependencies: dep.file_dependencies,
context_dependencies: dep.context_dependencies,
missing_dependencies: dep.missing_dependencies,
Expand Down Expand Up @@ -160,6 +161,7 @@ impl Module for CssModule {
Ok(BuildResult {
build_info: BuildInfo {
hash: Some(self.compute_hash(build_context.compiler_options)),
cacheable: self.cacheable,
file_dependencies: self.file_dependencies.clone(),
context_dependencies: self.context_dependencies.clone(),
missing_dependencies: self.missing_dependencies.clone(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ impl ParserAndGenerator for CssExtractParserAndGenerator {
source_map.clone(),
*identifier_index,
idx,
parse_context.build_info.cacheable,
parse_context.build_info.file_dependencies.clone(),
parse_context.build_info.context_dependencies.clone(),
parse_context.build_info.missing_dependencies.clone(),
Expand Down
2 changes: 2 additions & 0 deletions packages/rspack-test-tools/src/processor/hot-step.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,8 @@ export class HotSnapshotProcessor<
for (let [raw, replacement] of Object.entries(hashes)) {
str = str.split(raw).join(replacement);
}
// handle timestamp in css-extract
str = str.replace(/\/\/ (\d+)\s+(?=var cssReload)/, "");
return str;
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
ERROR in ./logo.svg× Module build failed:╰─▶ × Error: Failed to loadat Object.<anonymous>.module.exports (<PROJECT_ROOT>/tests/diagnosticsCases/module-build-failed/asset-module-build-failed/my-loader.js:2:8)at LOADER_EXECUTION (<RSPACK_ROOT>/dist/loader-runner/index.js:620:23)at runSyncOrAsync (<RSPACK_ROOT>/dist/loader-runner/index.js:621:11)at <RSPACK_ROOT>/dist/loader-runner/index.js:725:9at handleResult (<RSPACK_ROOT>/dist/loader-runner/loadLoader.js:125:5)at loadLoader (<RSPACK_ROOT>/dist/loader-runner/loadLoader.js:106:16)at iterateNormalLoaders (<RSPACK_ROOT>/dist/loader-runner/index.js:714:5)at <RSPACK_ROOT>/dist/loader-runner/index.js:541:13at new Promise (<anonymous>)at runLoaders (<RSPACK_ROOT>/dist/loader-runner/index.js:516:12)help: File was processed with this loader: '<PROJECT_ROOT>/tests/diagnosticsCases/module-build-failed/asset-module-build-failed/my-loader.js'
ERROR in ./logo.svg× Module build failed:╰─▶ × Error: Failed to loadat Object.<anonymous>.module.exports (<PROJECT_ROOT>/tests/diagnosticsCases/module-build-failed/asset-module-build-failed/my-loader.js:2:8)at LOADER_EXECUTION (<RSPACK_ROOT>/dist/loader-runner/index.js:626:23)at runSyncOrAsync (<RSPACK_ROOT>/dist/loader-runner/index.js:627:11)at <RSPACK_ROOT>/dist/loader-runner/index.js:731:9at handleResult (<RSPACK_ROOT>/dist/loader-runner/loadLoader.js:125:5)at loadLoader (<RSPACK_ROOT>/dist/loader-runner/loadLoader.js:106:16)at iterateNormalLoaders (<RSPACK_ROOT>/dist/loader-runner/index.js:720:5)at <RSPACK_ROOT>/dist/loader-runner/index.js:547:13at new Promise (<anonymous>)at runLoaders (<RSPACK_ROOT>/dist/loader-runner/index.js:522:12)help: File was processed with this loader: '<PROJECT_ROOT>/tests/diagnosticsCases/module-build-failed/asset-module-build-failed/my-loader.js'
Original file line number Diff line number Diff line change
@@ -1 +1 @@
ERROR in ./lib.js× Module build failed:╰─▶ × Error: Failed to loadat Object.<anonymous>.module.exports (<PROJECT_ROOT>/tests/diagnosticsCases/module-build-failed/loader-throw-error/my-loader.js:2:9)at LOADER_EXECUTION (<RSPACK_ROOT>/dist/loader-runner/index.js:620:23)at runSyncOrAsync (<RSPACK_ROOT>/dist/loader-runner/index.js:621:11)at <RSPACK_ROOT>/dist/loader-runner/index.js:725:9at handleResult (<RSPACK_ROOT>/dist/loader-runner/loadLoader.js:125:5)at loadLoader (<RSPACK_ROOT>/dist/loader-runner/loadLoader.js:106:16)at iterateNormalLoaders (<RSPACK_ROOT>/dist/loader-runner/index.js:714:5)at <RSPACK_ROOT>/dist/loader-runner/index.js:541:13at new Promise (<anonymous>)at runLoaders (<RSPACK_ROOT>/dist/loader-runner/index.js:516:12)help: File was processed with this loader: '<PROJECT_ROOT>/tests/diagnosticsCases/module-build-failed/loader-throw-error/my-loader.js'
ERROR in ./lib.js× Module build failed:╰─▶ × Error: Failed to loadat Object.<anonymous>.module.exports (<PROJECT_ROOT>/tests/diagnosticsCases/module-build-failed/loader-throw-error/my-loader.js:2:9)at LOADER_EXECUTION (<RSPACK_ROOT>/dist/loader-runner/index.js:626:23)at runSyncOrAsync (<RSPACK_ROOT>/dist/loader-runner/index.js:627:11)at <RSPACK_ROOT>/dist/loader-runner/index.js:731:9at handleResult (<RSPACK_ROOT>/dist/loader-runner/loadLoader.js:125:5)at loadLoader (<RSPACK_ROOT>/dist/loader-runner/loadLoader.js:106:16)at iterateNormalLoaders (<RSPACK_ROOT>/dist/loader-runner/index.js:720:5)at <RSPACK_ROOT>/dist/loader-runner/index.js:547:13at new Promise (<anonymous>)at runLoaders (<RSPACK_ROOT>/dist/loader-runner/index.js:522:12)help: File was processed with this loader: '<PROJECT_ROOT>/tests/diagnosticsCases/module-build-failed/loader-throw-error/my-loader.js'
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Case css-extract: Step 0

## Changed Files


## Asset Files
- Bundle: bundle.js

## Manifest

## Update
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# Case css-extract: Step 1

## Changed Files
- entry.js

## Asset Files
- Bundle: bundle.js
- Manifest: main.LAST_HASH.hot-update.json, size: 28
- Update: main.LAST_HASH.hot-update.js, size: 924

## Manifest

### main.LAST_HASH.hot-update.json

```json
{"c":["main"],"r":[],"m":[]}
```

## Update


### main.LAST_HASH.hot-update.js

#### Changed Modules
- ./entry.js
- ./index.module.css

#### Changed Runtime Modules
- webpack/runtime/get_full_hash

#### Changed Content
```js
"use strict";
self["webpackHotUpdate"]('main', {
"./entry.js": (function (module, __webpack_exports__, __webpack_require__) {
__webpack_require__.r(__webpack_exports__);
/* harmony import */var _index_module_css__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__(/*! ./index.module.css */ "./index.module.css");

module.hot.accept();
// modify
/* harmony default export */ __webpack_exports__["default"] = (_index_module_css__WEBPACK_IMPORTED_MODULE_0__["default"]);


}),
"./index.module.css": (function (__unused_webpack_module, __webpack_exports__, __webpack_require__) {
__webpack_require__.r(__webpack_exports__);
// extracted by css-extract-rspack-plugin
/* harmony default export */ __webpack_exports__["default"] = ({"bar":"Te04splqSHRbkdUMfiwa"});

}),

},function(__webpack_require__) {
// webpack/runtime/get_full_hash
(() => {
__webpack_require__.h = function () {
return "CURRENT_HASH";
};

})();

}
);
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import styles from './index.module.css'
module.hot.accept();
export default styles;
---
import styles from './index.module.css'
module.hot.accept();
// modify
export default styles;
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import styles from './index.module.css'
module.hot.accept();
export default styles;
11 changes: 11 additions & 0 deletions packages/rspack-test-tools/tests/hotCases/css/css-extract/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
const styles = require('./entry').default
require('./entry2')

it("css hmr", (done) => {
expect(styles).toHaveProperty('foo')
NEXT(require("../../update")(done, true, () => {
const updatedStyles = require('./entry').default
expect(updatedStyles).toHaveProperty('bar')
done()
}));
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.foo {
background-color: blue;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
let i = 0;
module.exports = function(content) {
this.cacheable(false);
i++;
if (i === 1) {
return content;
} else {
return `
.bar {
color: red;
}
`;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
const rspack = require("@rspack/core");

/** @type {import("@rspack/core").Configuration} */
module.exports = {
cache: true,
mode: "development",
entry: "./index",
module: {
rules: [
{
test: /\.module.css$/,
use: [
{
loader: rspack.CssExtractRspackPlugin.loader,
options: {
emit: false,
esModule: true,
}
},
"css-loader",
"./loader.js"
]
},
]
},
experiments: {
css: false,
},
plugins: [
new rspack.CssExtractRspackPlugin({
filename: "[name].css",
runtime: false,
}),
]
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Case recovery-cacheable: Step 0

## Changed Files


## Asset Files
- Bundle: bundle.js

## Manifest

## Update
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
# Case recovery-cacheable: Step 2

## Changed Files
- change.css

## Asset Files
- Bundle: bundle.js
- Manifest: main.LAST_HASH.hot-update.json, size: 28
- Update: main.LAST_HASH.hot-update.js, size: 861

## Manifest

### main.LAST_HASH.hot-update.json

```json
{"c":["main"],"r":[],"m":[]}
```

## Update


### main.LAST_HASH.hot-update.js

#### Changed Modules
- ./change.css?5dec

#### Changed Runtime Modules
- webpack/runtime/get_full_hash

#### Changed Content
```js
"use strict";
self["webpackHotUpdate"]('main', {
"./change.css?5dec": (function (module, __webpack_exports__, __webpack_require__) {
__webpack_require__.r(__webpack_exports__);
// extracted by css-extract-rspack-plugin

if(true) {
var cssReload = __webpack_require__(/*! ../../../../../rspack/dist/builtin-plugin/css-extract/hmr/hotModuleReplacement.js */ "../../../../../rspack/dist/builtin-plugin/css-extract/hmr/hotModuleReplacement.js")(module.id, {"locals":false});
module.hot.dispose(cssReload);
module.hot.accept(undefined, function(__WEBPACK_OUTDATED_DEPENDENCIES__) {
(cssReload)(__WEBPACK_OUTDATED_DEPENDENCIES__); }.bind(this));
}


}),

},function(__webpack_require__) {
// webpack/runtime/get_full_hash
(() => {
__webpack_require__.h = function () {
return "CURRENT_HASH";
};

})();

}
);
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
body {
background-color: red;
}
---
body {} {} {@}
---
body {
background-color: yellow;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import './change.css'
Loading

2 comments on commit 9e5732c

@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

@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-05 9e5732c) Current Change
10000_development-mode + exec 2.22 s ± 22 ms 2.23 s ± 21 ms +0.40 %
10000_development-mode_hmr + exec 739 ms ± 4.8 ms 739 ms ± 9.6 ms -0.01 %
10000_production-mode + exec 2.58 s ± 24 ms 2.57 s ± 16 ms -0.24 %
arco-pro_development-mode + exec 1.9 s ± 75 ms 1.91 s ± 78 ms +0.17 %
arco-pro_development-mode_hmr + exec 442 ms ± 2.2 ms 442 ms ± 3 ms -0.05 %
arco-pro_production-mode + exec 3.51 s ± 84 ms 3.51 s ± 90 ms +0.05 %
threejs_development-mode_10x + exec 1.41 s ± 19 ms 1.41 s ± 24 ms +0.08 %
threejs_development-mode_10x_hmr + exec 813 ms ± 7.3 ms 815 ms ± 11 ms +0.21 %
threejs_production-mode_10x + exec 4.72 s ± 40 ms 4.72 s ± 28 ms -0.06 %

Please sign in to comment.