From c0ea8acaf1e72f8a5e0ad2ab7f1296053e3bb468 Mon Sep 17 00:00:00 2001 From: Gengkun Date: Tue, 6 Aug 2024 17:53:38 +0800 Subject: [PATCH] fix: deep merge module resolve options (#7470) --- .../src/options/raw_module/mod.rs | 22 +++++++------- .../rspack_core/src/normal_module_factory.rs | 30 +++++++++++-------- crates/rspack_core/src/options/module.rs | 12 ++++++-- crates/rspack_core/src/utils/module_rules.rs | 8 ++--- .../configCases/module/merge-resolve/index.js | 7 +++++ .../merge-resolve/modules/lib/package.json | 7 +++++ .../module/merge-resolve/modules/lib/right.js | 1 + .../module/merge-resolve/modules/lib/wrong.js | 1 + .../merge-resolve/modules/lib2/package.json | 7 +++++ .../merge-resolve/modules/lib2/right.js | 1 + .../merge-resolve/modules/lib2/wrong.js | 1 + .../module/merge-resolve/reexports.js | 1 + .../module/merge-resolve/rspack.config.js | 24 +++++++++++++++ 13 files changed, 92 insertions(+), 30 deletions(-) create mode 100644 packages/rspack-test-tools/tests/configCases/module/merge-resolve/index.js create mode 100644 packages/rspack-test-tools/tests/configCases/module/merge-resolve/modules/lib/package.json create mode 100644 packages/rspack-test-tools/tests/configCases/module/merge-resolve/modules/lib/right.js create mode 100644 packages/rspack-test-tools/tests/configCases/module/merge-resolve/modules/lib/wrong.js create mode 100644 packages/rspack-test-tools/tests/configCases/module/merge-resolve/modules/lib2/package.json create mode 100644 packages/rspack-test-tools/tests/configCases/module/merge-resolve/modules/lib2/right.js create mode 100644 packages/rspack-test-tools/tests/configCases/module/merge-resolve/modules/lib2/wrong.js create mode 100644 packages/rspack-test-tools/tests/configCases/module/merge-resolve/reexports.js create mode 100644 packages/rspack-test-tools/tests/configCases/module/merge-resolve/rspack.config.js diff --git a/crates/rspack_binding_options/src/options/raw_module/mod.rs b/crates/rspack_binding_options/src/options/raw_module/mod.rs index 4b015f7967a..d2d88aecb15 100644 --- a/crates/rspack_binding_options/src/options/raw_module/mod.rs +++ b/crates/rspack_binding_options/src/options/raw_module/mod.rs @@ -15,8 +15,8 @@ use rspack_core::{ DynamicImportMode, ExportPresenceMode, FuncUseCtx, GeneratorOptions, GeneratorOptionsByModuleType, JavascriptParserOptions, JavascriptParserOrder, JavascriptParserUrl, ModuleNoParseRule, ModuleNoParseRules, ModuleNoParseTestFn, ModuleOptions, - ModuleRule, ModuleRuleEnforce, ModuleRuleUse, ModuleRuleUseLoader, ModuleType, OverrideStrict, - ParserOptions, ParserOptionsByModuleType, + ModuleRule, ModuleRuleEffect, ModuleRuleEnforce, ModuleRuleUse, ModuleRuleUseLoader, ModuleType, + OverrideStrict, ParserOptions, ParserOptionsByModuleType, }; use rspack_error::error; use rspack_napi::regexp::{JsRegExp, JsRegExpExt}; @@ -769,13 +769,6 @@ impl TryFrom for ModuleRule { resource: value.resource.map(|raw| raw.try_into()).transpose()?, description_data, with, - r#use: uses.transpose()?.unwrap_or_default(), - r#type: module_type, - layer: value.layer, - parser: value.parser.map(|raw| raw.into()), - generator: value.generator.map(|raw| raw.into()), - resolve: value.resolve.map(|raw| raw.try_into()).transpose()?, - side_effects: value.side_effects, issuer: value.issuer.map(|raw| raw.try_into()).transpose()?, issuer_layer: value.issuer_layer.map(|raw| raw.try_into()).transpose()?, dependency: value.dependency.map(|raw| raw.try_into()).transpose()?, @@ -783,7 +776,16 @@ impl TryFrom for ModuleRule { mimetype: value.mimetype.map(|raw| raw.try_into()).transpose()?, one_of, rules, - enforce, + effect: ModuleRuleEffect { + r#use: uses.transpose()?.unwrap_or_default(), + r#type: module_type, + layer: value.layer, + parser: value.parser.map(|raw| raw.into()), + generator: value.generator.map(|raw| raw.into()), + resolve: value.resolve.map(|raw| raw.try_into()).transpose()?, + side_effects: value.side_effects, + enforce, + }, }) } } diff --git a/crates/rspack_core/src/normal_module_factory.rs b/crates/rspack_core/src/normal_module_factory.rs index cd008bcb237..0639d4eb951 100644 --- a/crates/rspack_core/src/normal_module_factory.rs +++ b/crates/rspack_core/src/normal_module_factory.rs @@ -13,7 +13,7 @@ use crate::{ diagnostics::EmptyDependency, module_rules_matcher, parse_resource, resolve, stringify_loaders_and_resource, BoxLoader, BoxModule, CompilerOptions, Context, Dependency, DependencyCategory, FuncUseCtx, GeneratorOptions, ModuleExt, ModuleFactory, - ModuleFactoryCreateData, ModuleFactoryResult, ModuleIdentifier, ModuleLayer, ModuleRule, + ModuleFactoryCreateData, ModuleFactoryResult, ModuleIdentifier, ModuleLayer, ModuleRuleEffect, ModuleRuleEnforce, ModuleRuleUse, ModuleRuleUseLoader, ModuleType, NormalModule, ParserAndGenerator, ParserOptions, RawModule, Resolve, ResolveArgs, ResolveOptionsWithDependencyType, ResolveResult, Resolver, ResolverFactory, ResourceData, @@ -623,7 +623,7 @@ impl NormalModuleFactory { dependency: &dyn Dependency, issuer: Option<&'a str>, issuer_layer: Option<&'a str>, - ) -> Result> { + ) -> Result> { let mut rules = Vec::new(); module_rules_matcher( &self.options.module.rules, @@ -638,17 +638,21 @@ impl NormalModuleFactory { Ok(rules) } - fn calculate_resolve_options(&self, module_rules: &[&ModuleRule]) -> Option> { - let mut resolved = None; - module_rules.iter().for_each(|rule| { - if let Some(resolve) = rule.resolve.as_ref() { - resolved = Some(Box::new(resolve.to_owned())); + fn calculate_resolve_options(&self, module_rules: &[&ModuleRuleEffect]) -> Option> { + let mut resolved: Option = None; + for rule in module_rules { + if let Some(rule_resolve) = &rule.resolve { + if let Some(r) = resolved { + resolved = Some(r.merge(rule_resolve.to_owned())); + } else { + resolved = Some(rule_resolve.to_owned()); + } } - }); - resolved + } + resolved.map(Box::new) } - fn calculate_side_effects(&self, module_rules: &[&ModuleRule]) -> Option { + fn calculate_side_effects(&self, module_rules: &[&ModuleRuleEffect]) -> Option { let mut side_effect_res = None; // side_effects from module rule has higher priority module_rules.iter().for_each(|rule| { @@ -661,7 +665,7 @@ impl NormalModuleFactory { fn calculate_parser_and_generator_options( &self, - module_rules: &[&ModuleRule], + module_rules: &[&ModuleRuleEffect], ) -> (Option, Option) { let mut resolved_parser = None; let mut resolved_generator = None; @@ -727,7 +731,7 @@ impl NormalModuleFactory { fn calculate_module_type( &self, matched_module_type: Option, - module_rules: &[&ModuleRule], + module_rules: &[&ModuleRuleEffect], ) -> ModuleType { let mut resolved_module_type = matched_module_type.unwrap_or(ModuleType::JsAuto); module_rules.iter().for_each(|module_rule| { @@ -742,7 +746,7 @@ impl NormalModuleFactory { fn calculate_module_layer( &self, issuer_layer: Option<&ModuleLayer>, - module_rules: &[&ModuleRule], + module_rules: &[&ModuleRuleEffect], ) -> Option { let mut resolved_module_layer = issuer_layer; module_rules.iter().for_each(|module_rule| { diff --git a/crates/rspack_core/src/options/module.rs b/crates/rspack_core/src/options/module.rs index 9f136c04b47..43fbb641689 100644 --- a/crates/rspack_core/src/options/module.rs +++ b/crates/rspack_core/src/options/module.rs @@ -635,7 +635,7 @@ pub struct ModuleRuleUseLoader { pub type FnUse = Box BoxFuture<'static, Result>> + Sync + Send>; -#[derive(Derivative, Default)] +#[derive(Derivative)] #[derivative(Debug)] pub struct ModuleRule { /// A conditional match matching an absolute path + query + fragment. @@ -659,6 +659,14 @@ pub struct ModuleRule { pub mimetype: Option, pub description_data: Option, pub with: Option, + pub one_of: Option>, + pub rules: Option>, + pub effect: ModuleRuleEffect, +} + +#[derive(Derivative)] +#[derivative(Debug)] +pub struct ModuleRuleEffect { pub side_effects: Option, /// The `ModuleType` to use for the matched resource. pub r#type: Option, @@ -668,8 +676,6 @@ pub struct ModuleRule { pub parser: Option, pub generator: Option, pub resolve: Option, - pub one_of: Option>, - pub rules: Option>, pub enforce: ModuleRuleEnforce, } diff --git a/crates/rspack_core/src/utils/module_rules.rs b/crates/rspack_core/src/utils/module_rules.rs index cd5bc4f91dd..ada9c92903a 100644 --- a/crates/rspack_core/src/utils/module_rules.rs +++ b/crates/rspack_core/src/utils/module_rules.rs @@ -4,7 +4,7 @@ use async_recursion::async_recursion; use rspack_error::Result; use rspack_loader_runner::ResourceData; -use crate::{DependencyCategory, ImportAttributes, ModuleRule}; +use crate::{DependencyCategory, ImportAttributes, ModuleRule, ModuleRuleEffect}; pub async fn module_rules_matcher<'a>( rules: &'a [ModuleRule], @@ -13,7 +13,7 @@ pub async fn module_rules_matcher<'a>( issuer_layer: Option<&'a str>, dependency: &DependencyCategory, attributes: Option<&ImportAttributes>, - matched_rules: &mut Vec<&'a ModuleRule>, + matched_rules: &mut Vec<&'a ModuleRuleEffect>, ) -> Result<()> { for rule in rules { module_rule_matcher( @@ -39,7 +39,7 @@ pub async fn module_rule_matcher<'a>( issuer_layer: Option<&'a str>, dependency: &DependencyCategory, attributes: Option<&ImportAttributes>, - matched_rules: &mut Vec<&'a ModuleRule>, + matched_rules: &mut Vec<&'a ModuleRuleEffect>, ) -> Result { if let Some(test_rule) = &module_rule.rspack_resource && !test_rule @@ -222,6 +222,6 @@ pub async fn module_rule_matcher<'a>( return Ok(false); } } - matched_rules.push(module_rule); + matched_rules.push(&module_rule.effect); Ok(true) } diff --git a/packages/rspack-test-tools/tests/configCases/module/merge-resolve/index.js b/packages/rspack-test-tools/tests/configCases/module/merge-resolve/index.js new file mode 100644 index 00000000000..ea2802a1be9 --- /dev/null +++ b/packages/rspack-test-tools/tests/configCases/module/merge-resolve/index.js @@ -0,0 +1,7 @@ +import { lib } from "lib"; +import * as reexports from "./reexports"; + +it("should use correct resolve options", () => { + expect(lib).toBe("lib"); + expect(reexports.lib).toBe("lib2"); +}) \ No newline at end of file diff --git a/packages/rspack-test-tools/tests/configCases/module/merge-resolve/modules/lib/package.json b/packages/rspack-test-tools/tests/configCases/module/merge-resolve/modules/lib/package.json new file mode 100644 index 00000000000..a0887bee019 --- /dev/null +++ b/packages/rspack-test-tools/tests/configCases/module/merge-resolve/modules/lib/package.json @@ -0,0 +1,7 @@ +{ + "name": "lib", + "exports": { + "server": "./right.js", + "default": "./wrong.js" + } +} \ No newline at end of file diff --git a/packages/rspack-test-tools/tests/configCases/module/merge-resolve/modules/lib/right.js b/packages/rspack-test-tools/tests/configCases/module/merge-resolve/modules/lib/right.js new file mode 100644 index 00000000000..1664b920942 --- /dev/null +++ b/packages/rspack-test-tools/tests/configCases/module/merge-resolve/modules/lib/right.js @@ -0,0 +1 @@ +export const lib = "lib" \ No newline at end of file diff --git a/packages/rspack-test-tools/tests/configCases/module/merge-resolve/modules/lib/wrong.js b/packages/rspack-test-tools/tests/configCases/module/merge-resolve/modules/lib/wrong.js new file mode 100644 index 00000000000..2e8efdee486 --- /dev/null +++ b/packages/rspack-test-tools/tests/configCases/module/merge-resolve/modules/lib/wrong.js @@ -0,0 +1 @@ +throw new Error("wrong conditionNames") diff --git a/packages/rspack-test-tools/tests/configCases/module/merge-resolve/modules/lib2/package.json b/packages/rspack-test-tools/tests/configCases/module/merge-resolve/modules/lib2/package.json new file mode 100644 index 00000000000..3487fe46961 --- /dev/null +++ b/packages/rspack-test-tools/tests/configCases/module/merge-resolve/modules/lib2/package.json @@ -0,0 +1,7 @@ +{ + "name": "server-lib", + "exports": { + "server": "./right.js", + "default": "./wrong.js" + } +} \ No newline at end of file diff --git a/packages/rspack-test-tools/tests/configCases/module/merge-resolve/modules/lib2/right.js b/packages/rspack-test-tools/tests/configCases/module/merge-resolve/modules/lib2/right.js new file mode 100644 index 00000000000..43614b7ddac --- /dev/null +++ b/packages/rspack-test-tools/tests/configCases/module/merge-resolve/modules/lib2/right.js @@ -0,0 +1 @@ +export const lib = "lib2" \ No newline at end of file diff --git a/packages/rspack-test-tools/tests/configCases/module/merge-resolve/modules/lib2/wrong.js b/packages/rspack-test-tools/tests/configCases/module/merge-resolve/modules/lib2/wrong.js new file mode 100644 index 00000000000..2e8efdee486 --- /dev/null +++ b/packages/rspack-test-tools/tests/configCases/module/merge-resolve/modules/lib2/wrong.js @@ -0,0 +1 @@ +throw new Error("wrong conditionNames") diff --git a/packages/rspack-test-tools/tests/configCases/module/merge-resolve/reexports.js b/packages/rspack-test-tools/tests/configCases/module/merge-resolve/reexports.js new file mode 100644 index 00000000000..438cf455d39 --- /dev/null +++ b/packages/rspack-test-tools/tests/configCases/module/merge-resolve/reexports.js @@ -0,0 +1 @@ +export * from "server-lib"; diff --git a/packages/rspack-test-tools/tests/configCases/module/merge-resolve/rspack.config.js b/packages/rspack-test-tools/tests/configCases/module/merge-resolve/rspack.config.js new file mode 100644 index 00000000000..83985e20db6 --- /dev/null +++ b/packages/rspack-test-tools/tests/configCases/module/merge-resolve/rspack.config.js @@ -0,0 +1,24 @@ +/** @type {import("@rspack/core").Configuration} */ +module.exports = { + module: { + rules: [ + { + test: /\.js/, + resolve: { + conditionNames: ["server"], + } + }, + { + test: /reexports\.js/, + resolve: { + alias: { + "server-lib": "lib2", + } + } + }, + ] + }, + resolve: { + modules: ["modules"] + } +};