Skip to content

Commit 756221b

Browse files
authored
Turbopack: fix module rules for .module.scss (#82570)
It ran `postcss(webpack(postcss(filesource)))`, which doesn't work. And changing the order does make sense, since we do want Webpack (SCSS) to run first, and only then PostCSS? #82554 didn't catch every case
1 parent bb617a8 commit 756221b

File tree

6 files changed

+97
-80
lines changed

6 files changed

+97
-80
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
.other {
2+
// some SCSS
3+
color: red;
4+
}
Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
import styles from './page.module.css'
2+
import other from './other.module.scss'
23

34
export default function Page() {
4-
return <p className={styles.main}>hello world</p>
5+
return (
6+
<div>
7+
<p className={styles.main}>hello world</p>
8+
<span className={other.other}>hello world</span>
9+
</div>
10+
)
511
}

test/e2e/app-dir/css-modules-rsc-postcss/css-modules-rsc-postcss.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ describe('css-modules-rsc-postcss', () => {
55
files: __dirname,
66
dependencies: {
77
'postcss-nested': '4.2.1',
8+
sass: 'latest',
89
},
910
})
1011

@@ -13,5 +14,8 @@ describe('css-modules-rsc-postcss', () => {
1314
expect(await browser.elementByCss('p').getComputedCss('color')).toBe(
1415
'rgb(0, 128, 0)'
1516
)
17+
expect(await browser.elementByCss('span').getComputedCss('color')).toBe(
18+
'rgb(0, 128, 0)'
19+
)
1620
})
1721
})

test/e2e/app-dir/css-order/app/base-scss.module.scss

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// this is scss
12
$base1: rgb(255, 1, 0);
23

34
.base {
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// this is scss
12
.base {
23
color: rgb(255, 3, 0);
34
}

turbopack/crates/turbopack/src/module_options/mod.rs

Lines changed: 80 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -470,6 +470,86 @@ impl ModuleOptions {
470470
),
471471
];
472472

473+
if let Some(webpack_loaders_options) = enable_webpack_loaders {
474+
let webpack_loaders_options = webpack_loaders_options.await?;
475+
let execution_context =
476+
execution_context.context("execution_context is required for webpack_loaders")?;
477+
let import_map = if let Some(loader_runner_package) =
478+
webpack_loaders_options.loader_runner_package
479+
{
480+
package_import_map_from_import_mapping(
481+
rcstr!("loader-runner"),
482+
*loader_runner_package,
483+
)
484+
} else {
485+
package_import_map_from_context(
486+
rcstr!("loader-runner"),
487+
path.clone()
488+
.context("need_path in ModuleOptions::new is incorrect")?,
489+
)
490+
};
491+
for (key, rule) in webpack_loaders_options.rules.await?.iter() {
492+
rules.push(ModuleRule::new(
493+
RuleCondition::All(vec![
494+
if key.starts_with("#") {
495+
// This is a custom marker requiring a corresponding condition entry
496+
let conditions = (*webpack_loaders_options.conditions.await?)
497+
.context(
498+
"Expected a condition entry for the webpack loader rule \
499+
matching {key}. Create a `conditions` mapping in your \
500+
next.config.js",
501+
)?
502+
.await?;
503+
504+
let condition = conditions.get(key).context(
505+
"Expected a condition entry for the webpack loader rule matching \
506+
{key}.",
507+
)?;
508+
509+
match &condition.path {
510+
ConditionPath::Glob(glob) => RuleCondition::ResourcePathGlob {
511+
base: execution_context.project_path().owned().await?,
512+
glob: Glob::new(glob.clone()).await?,
513+
},
514+
ConditionPath::Regex(regex) => {
515+
RuleCondition::ResourcePathEsRegex(regex.await?)
516+
}
517+
}
518+
} else if key.contains('/') {
519+
RuleCondition::ResourcePathGlob {
520+
base: execution_context.project_path().owned().await?,
521+
glob: Glob::new(key.clone()).await?,
522+
}
523+
} else {
524+
RuleCondition::ResourceBasePathGlob(Glob::new(key.clone()).await?)
525+
},
526+
RuleCondition::not(RuleCondition::ResourceIsVirtualSource),
527+
module_css_external_transform_conditions.clone(),
528+
]),
529+
vec![ModuleRuleEffect::SourceTransforms(ResolvedVc::cell(vec![
530+
ResolvedVc::upcast(
531+
WebpackLoaders::new(
532+
node_evaluate_asset_context(
533+
*execution_context,
534+
Some(import_map),
535+
None,
536+
Layer::new(rcstr!("webpack_loaders")),
537+
false,
538+
),
539+
*execution_context,
540+
*rule.loaders,
541+
rule.rename_as.clone(),
542+
resolve_options_context,
543+
matches!(ecmascript_source_maps, SourceMapsType::Full),
544+
)
545+
.to_resolved()
546+
.await?,
547+
),
548+
]))],
549+
));
550+
}
551+
}
552+
473553
if enable_raw_css {
474554
rules.extend([
475555
ModuleRule::new(
@@ -644,85 +724,6 @@ impl ModuleOptions {
644724
));
645725
}
646726

647-
if let Some(webpack_loaders_options) = enable_webpack_loaders {
648-
let webpack_loaders_options = webpack_loaders_options.await?;
649-
let execution_context =
650-
execution_context.context("execution_context is required for webpack_loaders")?;
651-
let import_map = if let Some(loader_runner_package) =
652-
webpack_loaders_options.loader_runner_package
653-
{
654-
package_import_map_from_import_mapping(
655-
rcstr!("loader-runner"),
656-
*loader_runner_package,
657-
)
658-
} else {
659-
package_import_map_from_context(
660-
rcstr!("loader-runner"),
661-
path.context("need_path in ModuleOptions::new is incorrect")?,
662-
)
663-
};
664-
for (key, rule) in webpack_loaders_options.rules.await?.iter() {
665-
rules.push(ModuleRule::new(
666-
RuleCondition::All(vec![
667-
if key.starts_with("#") {
668-
// This is a custom marker requiring a corresponding condition entry
669-
let conditions = (*webpack_loaders_options.conditions.await?)
670-
.context(
671-
"Expected a condition entry for the webpack loader rule \
672-
matching {key}. Create a `conditions` mapping in your \
673-
next.config.js",
674-
)?
675-
.await?;
676-
677-
let condition = conditions.get(key).context(
678-
"Expected a condition entry for the webpack loader rule matching \
679-
{key}.",
680-
)?;
681-
682-
match &condition.path {
683-
ConditionPath::Glob(glob) => RuleCondition::ResourcePathGlob {
684-
base: execution_context.project_path().owned().await?,
685-
glob: Glob::new(glob.clone()).await?,
686-
},
687-
ConditionPath::Regex(regex) => {
688-
RuleCondition::ResourcePathEsRegex(regex.await?)
689-
}
690-
}
691-
} else if key.contains('/') {
692-
RuleCondition::ResourcePathGlob {
693-
base: execution_context.project_path().owned().await?,
694-
glob: Glob::new(key.clone()).await?,
695-
}
696-
} else {
697-
RuleCondition::ResourceBasePathGlob(Glob::new(key.clone()).await?)
698-
},
699-
RuleCondition::not(RuleCondition::ResourceIsVirtualSource),
700-
module_css_external_transform_conditions.clone(),
701-
]),
702-
vec![ModuleRuleEffect::SourceTransforms(ResolvedVc::cell(vec![
703-
ResolvedVc::upcast(
704-
WebpackLoaders::new(
705-
node_evaluate_asset_context(
706-
*execution_context,
707-
Some(import_map),
708-
None,
709-
Layer::new(rcstr!("webpack_loaders")),
710-
false,
711-
),
712-
*execution_context,
713-
*rule.loaders,
714-
rule.rename_as.clone(),
715-
resolve_options_context,
716-
matches!(ecmascript_source_maps, SourceMapsType::Full),
717-
)
718-
.to_resolved()
719-
.await?,
720-
),
721-
]))],
722-
));
723-
}
724-
}
725-
726727
rules.extend(module_rules.iter().cloned());
727728

728729
Ok(ModuleOptions::cell(ModuleOptions { rules }))

0 commit comments

Comments
 (0)