Skip to content

Commit

Permalink
fix: deep merge module resolve options (#7470)
Browse files Browse the repository at this point in the history
  • Loading branch information
ahabhgk authored Aug 6, 2024
1 parent 258c050 commit c0ea8ac
Show file tree
Hide file tree
Showing 13 changed files with 92 additions and 30 deletions.
22 changes: 12 additions & 10 deletions crates/rspack_binding_options/src/options/raw_module/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -769,21 +769,23 @@ impl TryFrom<RawModuleRule> 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()?,
scheme: value.scheme.map(|raw| raw.try_into()).transpose()?,
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,
},
})
}
}
Expand Down
30 changes: 17 additions & 13 deletions crates/rspack_core/src/normal_module_factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -623,7 +623,7 @@ impl NormalModuleFactory {
dependency: &dyn Dependency,
issuer: Option<&'a str>,
issuer_layer: Option<&'a str>,
) -> Result<Vec<&'a ModuleRule>> {
) -> Result<Vec<&'a ModuleRuleEffect>> {
let mut rules = Vec::new();
module_rules_matcher(
&self.options.module.rules,
Expand All @@ -638,17 +638,21 @@ impl NormalModuleFactory {
Ok(rules)
}

fn calculate_resolve_options(&self, module_rules: &[&ModuleRule]) -> Option<Box<Resolve>> {
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<Box<Resolve>> {
let mut resolved: Option<Resolve> = 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<bool> {
fn calculate_side_effects(&self, module_rules: &[&ModuleRuleEffect]) -> Option<bool> {
let mut side_effect_res = None;
// side_effects from module rule has higher priority
module_rules.iter().for_each(|rule| {
Expand All @@ -661,7 +665,7 @@ impl NormalModuleFactory {

fn calculate_parser_and_generator_options(
&self,
module_rules: &[&ModuleRule],
module_rules: &[&ModuleRuleEffect],
) -> (Option<ParserOptions>, Option<GeneratorOptions>) {
let mut resolved_parser = None;
let mut resolved_generator = None;
Expand Down Expand Up @@ -727,7 +731,7 @@ impl NormalModuleFactory {
fn calculate_module_type(
&self,
matched_module_type: Option<ModuleType>,
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| {
Expand All @@ -742,7 +746,7 @@ impl NormalModuleFactory {
fn calculate_module_layer(
&self,
issuer_layer: Option<&ModuleLayer>,
module_rules: &[&ModuleRule],
module_rules: &[&ModuleRuleEffect],
) -> Option<ModuleLayer> {
let mut resolved_module_layer = issuer_layer;
module_rules.iter().for_each(|module_rule| {
Expand Down
12 changes: 9 additions & 3 deletions crates/rspack_core/src/options/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,7 @@ pub struct ModuleRuleUseLoader {
pub type FnUse =
Box<dyn Fn(FuncUseCtx) -> BoxFuture<'static, Result<Vec<ModuleRuleUseLoader>>> + Sync + Send>;

#[derive(Derivative, Default)]
#[derive(Derivative)]
#[derivative(Debug)]
pub struct ModuleRule {
/// A conditional match matching an absolute path + query + fragment.
Expand All @@ -659,6 +659,14 @@ pub struct ModuleRule {
pub mimetype: Option<RuleSetCondition>,
pub description_data: Option<DescriptionData>,
pub with: Option<With>,
pub one_of: Option<Vec<ModuleRule>>,
pub rules: Option<Vec<ModuleRule>>,
pub effect: ModuleRuleEffect,
}

#[derive(Derivative)]
#[derivative(Debug)]
pub struct ModuleRuleEffect {
pub side_effects: Option<bool>,
/// The `ModuleType` to use for the matched resource.
pub r#type: Option<ModuleType>,
Expand All @@ -668,8 +676,6 @@ pub struct ModuleRule {
pub parser: Option<ParserOptions>,
pub generator: Option<GeneratorOptions>,
pub resolve: Option<Resolve>,
pub one_of: Option<Vec<ModuleRule>>,
pub rules: Option<Vec<ModuleRule>>,
pub enforce: ModuleRuleEnforce,
}

Expand Down
8 changes: 4 additions & 4 deletions crates/rspack_core/src/utils/module_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand All @@ -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(
Expand All @@ -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<bool> {
if let Some(test_rule) = &module_rule.rspack_resource
&& !test_rule
Expand Down Expand Up @@ -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)
}
Original file line number Diff line number Diff line change
@@ -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");
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "lib",
"exports": {
"server": "./right.js",
"default": "./wrong.js"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const lib = "lib"
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
throw new Error("wrong conditionNames")
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "server-lib",
"exports": {
"server": "./right.js",
"default": "./wrong.js"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const lib = "lib2"
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
throw new Error("wrong conditionNames")
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from "server-lib";
Original file line number Diff line number Diff line change
@@ -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"]
}
};

2 comments on commit c0ea8ac

@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 ❌ failure
_selftest ✅ success
nx ❌ failure
rspress ✅ success
rsbuild ❌ failure
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-08-06 9347ecc) Current Change
10000_development-mode + exec 2.32 s ± 38 ms 2.33 s ± 23 ms +0.65 %
10000_development-mode_hmr + exec 708 ms ± 18 ms 713 ms ± 7 ms +0.69 %
10000_production-mode + exec 2.86 s ± 26 ms 2.88 s ± 36 ms +0.90 %
arco-pro_development-mode + exec 1.86 s ± 68 ms 1.88 s ± 62 ms +0.91 %
arco-pro_development-mode_hmr + exec 434 ms ± 0.91 ms 434 ms ± 4 ms -0.12 %
arco-pro_production-mode + exec 3.44 s ± 91 ms 3.44 s ± 87 ms -0.12 %
threejs_development-mode_10x + exec 1.7 s ± 15 ms 1.72 s ± 19 ms +1.34 %
threejs_development-mode_10x_hmr + exec 808 ms ± 9.6 ms 827 ms ± 3.9 ms +2.32 %
threejs_production-mode_10x + exec 5.49 s ± 21 ms 5.53 s ± 32 ms +0.72 %

Please sign in to comment.