Skip to content

Commit

Permalink
fix(side_effects): when use path as flag can't match resolved file (#902
Browse files Browse the repository at this point in the history
)

* refactor: 🎨 change unnesssary move to ref

* fix(sideEffects-flag): 🐛 match path glob

* test: ✅ add e2e test
  • Loading branch information
stormslowly authored Jan 31, 2024
1 parent 69d8c76 commit 6df57bc
Show file tree
Hide file tree
Showing 12 changed files with 87 additions and 11 deletions.
7 changes: 7 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/mako/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ serde_json = { workspace = true }
cached = { workspace = true }
swc_core = { workspace = true, features = ["swc_ecma_quote_macros"] }
miette = { version = "5.10.0", features = ["fancy"] }
glob-match = "0.2.1"

[dev-dependencies]
insta = { version = "1.30.0", features = ["yaml"] }
Expand Down
3 changes: 1 addition & 2 deletions crates/mako/src/chunk_pot/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,7 @@ pub(crate) fn pot_to_module_object(pot: &ChunkPot, context: &Arc<Context>) -> Re
let fn_expr = to_module_fn_expr(module.0)?;

let span = Span::dummy_with_cmt();
let id = module.0.id.id.clone();
let id = relative_to_root(id, &context.root);
let id = relative_to_root(&module.0.id.id, &context.root);
// to avoid comment broken by glob=**/* for context module
let id = id.replace("*/", "*\\/");
comments.add_leading(
Expand Down
2 changes: 1 addition & 1 deletion crates/mako/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ pub fn generate_module_id(origin_module_id: String, context: &Arc<Context>) -> S
}
}

pub fn relative_to_root(module_path: String, root: &PathBuf) -> String {
pub fn relative_to_root(module_path: &String, root: &PathBuf) -> String {
let absolute_path = PathBuf::from(module_path);
let relative_path = diff_paths(&absolute_path, root).unwrap_or(absolute_path);
// diff_paths result always starts with ".."/"." or not
Expand Down
48 changes: 40 additions & 8 deletions crates/mako/src/tree_shaking/module_side_effects_flag.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use std::path::{Path, PathBuf};

use glob_match::glob_match;
use mako_core::glob::Pattern;

use crate::module::ModuleInfo;
use crate::module::{relative_to_root, ModuleInfo};
use crate::resolve::{ResolvedResource, ResolverResource};

impl ModuleInfo {
Expand All @@ -13,7 +16,12 @@ impl ModuleInfo {
let value = data.raw();
let side_effects = value.get("sideEffects".to_string());

side_effects.map(|side_effect| self.match_flag(side_effect))
let root: &Path = desc.dir().as_ref();
let root: PathBuf = root.into();

side_effects.map(|side_effect| {
Self::match_flag(side_effect, relative_to_root(&self.path, &root).as_str())
})
}
None => None,
}
Expand All @@ -36,7 +44,15 @@ impl ModuleInfo {
let side_effects = value.get("sideEffects".to_string());

match side_effects {
Some(side_effect) => self.match_flag(side_effect),
Some(side_effect) => {
let root: &Path = desc.dir().as_ref();
let root: PathBuf = root.into();

Self::match_flag(
side_effect,
relative_to_root(&self.path, &root).as_str(),
)
}
None => true,
}
}
Expand All @@ -46,13 +62,14 @@ impl ModuleInfo {
true
}
}
#[allow(dead_code)]
fn match_flag(&self, flag: &serde_json::Value) -> bool {
fn match_flag(flag: &serde_json::Value, path: &str) -> bool {
match flag {
// NOTE: 口径需要对齐这里:https://github.com/webpack/webpack/blob/main/lib/optimize/SideEffectsFlagPlugin.js#L331
serde_json::Value::Bool(flag) => *flag,
serde_json::Value::String(flag) => match_glob_pattern(flag, &self.path),
serde_json::Value::Array(flags) => flags.iter().any(|flag| self.match_flag(flag)),
serde_json::Value::String(flag) => match_glob_pattern(flag, path),
serde_json::Value::Array(flags) => {
flags.iter().any(|flag| Self::match_flag(flag, path))
}
_ => true,
}
}
Expand All @@ -66,15 +83,30 @@ fn match_glob_pattern(pattern: &str, path: &str) -> bool {
.unwrap()
.matches(path);
}
Pattern::new(pattern).unwrap().matches(path)

glob_match(pattern, path)
}

#[cfg(test)]
mod tests {
use mako_core::tokio;

use super::match_glob_pattern;
use crate::test_helper::{get_module, setup_compiler};

#[test]
fn test_path_side_effects_flag() {
assert!(match_glob_pattern("./src/index.js", "./src/index.js",));
}

#[test]
fn test_wild_effects_flag() {
assert!(match_glob_pattern(
"./src/lib/**/*.s.js",
"./src/lib/apple/pie/index.s.js",
));
}

#[tokio::test(flavor = "multi_thread")]
async fn test_side_effects_flag() {
let compiler = setup_compiler("test/build/side-effects-flag", false);
Expand Down
5 changes: 5 additions & 0 deletions e2e/fixtures/webpack.side-effects.with_path_flag/expect.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
const { injectSimpleJest, parseBuildResult } = require("../../../scripts/test-utils")
const { files } = parseBuildResult(__dirname);
injectSimpleJest()

require("./dist/index.js");
6 changes: 6 additions & 0 deletions e2e/fixtures/webpack.side-effects.with_path_flag/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { Module } from "d3";

it("should not crash", () => {
let m = new Module();
m.test();
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"entry": {
"index": "index.js"
},
"optimization": {
"skipModules": true
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 6df57bc

Please sign in to comment.