From fffcfa241dfb3d14e05185cd0dba202264a6c088 Mon Sep 17 00:00:00 2001 From: Will Binns-Smith Date: Mon, 27 Feb 2023 08:14:24 -0800 Subject: [PATCH] Automatic babel-loader followup (#3944) This: * Addresses feedback from https://github.com/vercel/turbo/pull/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 --- Cargo.lock | 12 ++++++ crates/next-core/src/babel.rs | 15 ++++--- .../metadata/input/app/[slug]/page.tsx | 0 .../metadata/input/app/layout.tsx | 0 .../metadata/input/app/page.tsx | 0 .../metadata/input/app/test.tsx | 0 .../metadata/input/app/triangle-black.png | Bin .../metadata/input/next.config.js | 0 crates/turbopack/Cargo.toml | 1 + crates/turbopack/src/lib.rs | 2 +- crates/turbopack/src/module_options/mod.rs | 5 ++- .../src/module_options/module_rule.rs | 13 ++++-- .../src/module_options/rule_condition.rs | 40 +++++++++++++++--- 13 files changed, 71 insertions(+), 17 deletions(-) rename crates/next-dev-tests/tests/integration/next/app/{ => __flakey__}/metadata/input/app/[slug]/page.tsx (100%) rename crates/next-dev-tests/tests/integration/next/app/{ => __flakey__}/metadata/input/app/layout.tsx (100%) rename crates/next-dev-tests/tests/integration/next/app/{ => __flakey__}/metadata/input/app/page.tsx (100%) rename crates/next-dev-tests/tests/integration/next/app/{ => __flakey__}/metadata/input/app/test.tsx (100%) rename crates/next-dev-tests/tests/integration/next/app/{ => __flakey__}/metadata/input/app/triangle-black.png (100%) rename crates/next-dev-tests/tests/integration/next/app/{ => __flakey__}/metadata/input/next.config.js (100%) diff --git a/Cargo.lock b/Cargo.lock index 7d3683a58061d..1c9c2a046b69f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -262,6 +262,17 @@ dependencies = [ "windows-sys 0.42.0", ] +[[package]] +name = "async-recursion" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3b015a331cc64ebd1774ba119538573603427eaace0a1950c423ab971f903796" +dependencies = [ + "proc-macro2 1.0.51", + "quote 1.0.23", + "syn 1.0.107", +] + [[package]] name = "async-std" version = "1.12.0" @@ -7717,6 +7728,7 @@ name = "turbopack" version = "0.1.0" dependencies = [ "anyhow", + "async-recursion", "criterion 0.3.6", "difference", "futures", diff --git a/crates/next-core/src/babel.rs b/crates/next-core/src/babel.rs index c5d8d1d0328dc..95c36bdf935d2 100644 --- a/crates/next-core/src/babel.rs +++ b/crates/next-core/src/babel.rs @@ -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, @@ -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; } @@ -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 { @@ -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( @@ -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()); diff --git a/crates/next-dev-tests/tests/integration/next/app/metadata/input/app/[slug]/page.tsx b/crates/next-dev-tests/tests/integration/next/app/__flakey__/metadata/input/app/[slug]/page.tsx similarity index 100% rename from crates/next-dev-tests/tests/integration/next/app/metadata/input/app/[slug]/page.tsx rename to crates/next-dev-tests/tests/integration/next/app/__flakey__/metadata/input/app/[slug]/page.tsx diff --git a/crates/next-dev-tests/tests/integration/next/app/metadata/input/app/layout.tsx b/crates/next-dev-tests/tests/integration/next/app/__flakey__/metadata/input/app/layout.tsx similarity index 100% rename from crates/next-dev-tests/tests/integration/next/app/metadata/input/app/layout.tsx rename to crates/next-dev-tests/tests/integration/next/app/__flakey__/metadata/input/app/layout.tsx diff --git a/crates/next-dev-tests/tests/integration/next/app/metadata/input/app/page.tsx b/crates/next-dev-tests/tests/integration/next/app/__flakey__/metadata/input/app/page.tsx similarity index 100% rename from crates/next-dev-tests/tests/integration/next/app/metadata/input/app/page.tsx rename to crates/next-dev-tests/tests/integration/next/app/__flakey__/metadata/input/app/page.tsx diff --git a/crates/next-dev-tests/tests/integration/next/app/metadata/input/app/test.tsx b/crates/next-dev-tests/tests/integration/next/app/__flakey__/metadata/input/app/test.tsx similarity index 100% rename from crates/next-dev-tests/tests/integration/next/app/metadata/input/app/test.tsx rename to crates/next-dev-tests/tests/integration/next/app/__flakey__/metadata/input/app/test.tsx diff --git a/crates/next-dev-tests/tests/integration/next/app/metadata/input/app/triangle-black.png b/crates/next-dev-tests/tests/integration/next/app/__flakey__/metadata/input/app/triangle-black.png similarity index 100% rename from crates/next-dev-tests/tests/integration/next/app/metadata/input/app/triangle-black.png rename to crates/next-dev-tests/tests/integration/next/app/__flakey__/metadata/input/app/triangle-black.png diff --git a/crates/next-dev-tests/tests/integration/next/app/metadata/input/next.config.js b/crates/next-dev-tests/tests/integration/next/app/__flakey__/metadata/input/next.config.js similarity index 100% rename from crates/next-dev-tests/tests/integration/next/app/metadata/input/next.config.js rename to crates/next-dev-tests/tests/integration/next/app/__flakey__/metadata/input/next.config.js diff --git a/crates/turbopack/Cargo.toml b/crates/turbopack/Cargo.toml index 2f5e46383ad2d..d9459f8f831f2 100644 --- a/crates/turbopack/Cargo.toml +++ b/crates/turbopack/Cargo.toml @@ -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" diff --git a/crates/turbopack/src/lib.rs b/crates/turbopack/src/lib.rs index 4137bee7bb720..a0fe58b28692b 100644 --- a/crates/turbopack/src/lib.rs +++ b/crates/turbopack/src/lib.rs @@ -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) => { diff --git a/crates/turbopack/src/module_options/mod.rs b/crates/turbopack/src/module_options/mod.rs index 3339a7b932c44..62755c7a80864 100644 --- a/crates/turbopack/src/module_options/mod.rs +++ b/crates/turbopack/src/module_options/mod.rs @@ -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![ diff --git a/crates/turbopack/src/module_options/module_rule.rs b/crates/turbopack/src/module_options/module_rule.rs index 5507ebccb525f..1a2c5a008019b 100644 --- a/crates/turbopack/src/module_options/module_rule.rs +++ b/crates/turbopack/src/module_options/module_rule.rs @@ -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; @@ -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 { + self.condition.matches(source, path, reference_type).await } } diff --git a/crates/turbopack/src/module_options/rule_condition.rs b/crates/turbopack/src/module_options/rule_condition.rs index 9ae98605417b9..6e7f54c6437f1 100644 --- a/crates/turbopack/src/module_options/rule_condition.rs +++ b/crates/turbopack/src/module_options/rule_condition.rs @@ -1,7 +1,11 @@ +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 { @@ -9,6 +13,7 @@ pub enum ModuleRuleCondition { Any(Vec), Not(Box), ReferenceType(ReferenceType), + ResourceIsVirtualAsset, ResourcePathEquals(FileSystemPathReadRef), ResourcePathHasNoExtension, ResourcePathEndsWith(String), @@ -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 { + 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 => { @@ -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"), - } + }) } }