Skip to content

Commit

Permalink
Automatic babel-loader followup (#3944)
Browse files Browse the repository at this point in the history
This:
* Addresses feedback from #3862
* Exempts VirtualAssets from webpack loaders. This addresses a bug where
next-hydrate.tsx could not be read.
	* Adds `ModuleRuleCondition::ResourceIsVirtualAsset` to filter these

---------

Co-authored-by: Justin Ridgewell <justin@ridgewell.name>
  • Loading branch information
wbinnssmith and jridgewell authored Feb 27, 2023
1 parent f572c30 commit fffcfa2
Show file tree
Hide file tree
Showing 13 changed files with 71 additions and 17 deletions.
12 changes: 12 additions & 0 deletions Cargo.lock

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

15 changes: 10 additions & 5 deletions crates/next-core/src/babel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use turbo_tasks::{
primitives::{BoolVc, StringVc},
Value,
};
use turbo_tasks_fs::FileSystemPathVc;
use turbo_tasks_fs::{FileSystemEntryType, FileSystemPathVc};
use turbopack::{
module_options::WebpackLoadersOptionsVc, resolve_options,
resolve_options_context::ResolveOptionsContext,
Expand Down Expand Up @@ -37,8 +37,8 @@ pub async fn maybe_add_babel_loader(
let has_babel_config = {
let mut has_babel_config = false;
for filename in BABEL_CONFIG_FILES {
let metadata = project_root.join(filename).metadata().await;
if metadata.is_ok() {
let filetype = *project_root.join(filename).get_type().await?;
if matches!(filetype, FileSystemEntryType::File) {
has_babel_config = true;
break;
}
Expand All @@ -48,6 +48,7 @@ pub async fn maybe_add_babel_loader(

if has_babel_config {
let mut options = (*webpack_options.await?).clone();
let mut has_emitted_babel_resolve_issue = false;
for ext in [".js", ".jsx", ".ts", ".tsx", ".cjs", ".mjs"] {
let configs = options.extension_to_loaders.get(ext);
let has_babel_loader = match configs {
Expand All @@ -73,7 +74,9 @@ pub async fn maybe_add_babel_loader(
};

if !has_babel_loader {
if !*(is_babel_loader_available(project_root).await?) {
if !has_emitted_babel_resolve_issue
&& !*is_babel_loader_available(project_root).await?
{
BabelIssue {
path: project_root,
title: StringVc::cell(
Expand All @@ -88,7 +91,9 @@ pub async fn maybe_add_babel_loader(
}
.cell()
.as_issue()
.emit()
.emit();

has_emitted_babel_resolve_issue = true;
}

let loader = WebpackLoaderConfigItem::LoaderName("babel-loader".to_owned());
Expand Down
1 change: 1 addition & 0 deletions crates/turbopack/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ bench_against_node_nft = []

[dependencies]
anyhow = "1.0.47"
async-recursion = "1.0.2"
indexmap = { workspace = true, features = ["serde"] }
lazy_static = "1.4.0"
regex = "1.5.4"
Expand Down
2 changes: 1 addition & 1 deletion crates/turbopack/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ async fn module(
let mut current_source = source;
let mut current_module_type = None;
for rule in options.await?.rules.iter() {
if rule.matches(&*path.await?, &reference_type) {
if rule.matches(source, &*path.await?, &reference_type).await? {
for effect in rule.effects() {
match effect {
ModuleRuleEffect::SourceTransforms(transforms) => {
Expand Down
5 changes: 4 additions & 1 deletion crates/turbopack/src/module_options/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,10 @@ impl ModuleOptionsVc {
};
for (ext, loaders) in webpack_loaders_options.extension_to_loaders.iter() {
rules.push(ModuleRule::new(
ModuleRuleCondition::ResourcePathEndsWith(ext.to_string()),
ModuleRuleCondition::All(vec![
ModuleRuleCondition::ResourcePathEndsWith(ext.to_string()),
ModuleRuleCondition::not(ModuleRuleCondition::ResourceIsVirtualAsset),
]),
vec![
ModuleRuleEffect::ModuleType(ModuleType::Ecmascript(app_transforms)),
ModuleRuleEffect::SourceTransforms(SourceTransformsVc::cell(vec![
Expand Down
13 changes: 10 additions & 3 deletions crates/turbopack/src/module_options/module_rule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ use anyhow::Result;
use serde::{Deserialize, Serialize};
use turbo_tasks::trace::TraceRawVcs;
use turbo_tasks_fs::FileSystemPath;
use turbopack_core::{reference_type::ReferenceType, source_transform::SourceTransformsVc};
use turbopack_core::{
asset::AssetVc, reference_type::ReferenceType, source_transform::SourceTransformsVc,
};
use turbopack_css::CssInputTransformsVc;
use turbopack_ecmascript::EcmascriptInputTransformsVc;

Expand All @@ -23,8 +25,13 @@ impl ModuleRule {
self.effects.iter()
}

pub fn matches(&self, path: &FileSystemPath, reference_type: &ReferenceType) -> bool {
self.condition.matches(path, reference_type)
pub async fn matches(
&self,
source: AssetVc,
path: &FileSystemPath,
reference_type: &ReferenceType,
) -> Result<bool> {
self.condition.matches(source, path, reference_type).await
}
}

Expand Down
40 changes: 33 additions & 7 deletions crates/turbopack/src/module_options/rule_condition.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
use anyhow::Result;
use async_recursion::async_recursion;
use serde::{Deserialize, Serialize};
use turbo_tasks::{primitives::Regex, trace::TraceRawVcs};
use turbo_tasks_fs::{FileSystemPath, FileSystemPathReadRef};
use turbopack_core::reference_type::ReferenceType;
use turbopack_core::{
asset::AssetVc, reference_type::ReferenceType, virtual_asset::VirtualAssetVc,
};

#[derive(Debug, Clone, Serialize, Deserialize, TraceRawVcs, PartialEq, Eq)]
pub enum ModuleRuleCondition {
All(Vec<ModuleRuleCondition>),
Any(Vec<ModuleRuleCondition>),
Not(Box<ModuleRuleCondition>),
ReferenceType(ReferenceType),
ResourceIsVirtualAsset,
ResourcePathEquals(FileSystemPathReadRef),
ResourcePathHasNoExtension,
ResourcePathEndsWith(String),
Expand All @@ -33,15 +38,33 @@ impl ModuleRuleCondition {
}

impl ModuleRuleCondition {
pub fn matches(&self, path: &FileSystemPath, reference_type: &ReferenceType) -> bool {
match self {
#[async_recursion]
pub async fn matches(
&self,
source: AssetVc,
path: &FileSystemPath,
reference_type: &ReferenceType,
) -> Result<bool> {
Ok(match self {
ModuleRuleCondition::All(conditions) => {
conditions.iter().all(|c| c.matches(path, reference_type))
for condition in conditions {
if !condition.matches(source, path, reference_type).await? {
return Ok(false);
}
}
true
}
ModuleRuleCondition::Any(conditions) => {
conditions.iter().any(|c| c.matches(path, reference_type))
for condition in conditions {
if condition.matches(source, path, reference_type).await? {
return Ok(true);
}
}
false
}
ModuleRuleCondition::Not(condition) => {
!condition.matches(source, path, reference_type).await?
}
ModuleRuleCondition::Not(condition) => !condition.matches(path, reference_type),
ModuleRuleCondition::ResourcePathEquals(other) => path == &**other,
ModuleRuleCondition::ResourcePathEndsWith(end) => path.path.ends_with(end),
ModuleRuleCondition::ResourcePathHasNoExtension => {
Expand All @@ -64,7 +87,10 @@ impl ModuleRuleCondition {
ModuleRuleCondition::ReferenceType(condition_ty) => {
condition_ty.includes(reference_type)
}
ModuleRuleCondition::ResourceIsVirtualAsset => {
VirtualAssetVc::resolve_from(source).await?.is_some()
}
_ => todo!("not implemented yet"),
}
})
}
}

0 comments on commit fffcfa2

Please sign in to comment.