Repository: vercel/next.js
Pull request: [turbopack] Remove some unnecessary awaits now that FileSystemPath is synchronously available.

### Remove some unnecessary awaits now that FileSystemPath is synchronously available.

### What?

* Remove `async` from `ContextCondition::matches` simplifying the implementation and eliminating a bunch of temporary allocations
* Remove some unnecessary FileSystemPath clones
* Remove recursion from `resolve::base_options` this function really just wants the FileSystem::root so require that instead
* Share the same same `cell` for most of the node externals

### Why?

Noticed this while reviewing the `resovle` codebase.  I expect them to by negligible perf wins, but wins nontheless

Closes PACK-4966

Changed files:
filename: turbopack/crates/turbopack-core/src/condition.rs
patch: @@ -1,5 +1,3 @@
-use anyhow::Result;
-use futures::{StreamExt, stream};
 use serde::{Deserialize, Serialize};
 use turbo_tasks::{NonLocalValue, trace::TraceRawVcs};
 use turbo_tasks_fs::FileSystemPath;
@@ -31,32 +29,22 @@ impl ContextCondition {
     }
 
     /// Returns true if the condition matches the context.
-    pub async fn matches(&self, path: &FileSystemPath) -> Result<bool> {
+    pub fn matches(&self, path: &FileSystemPath) -> bool {
         match self {
-            ContextCondition::All(conditions) => {
-                // False positive.
-                #[allow(clippy::manual_try_fold)]
-                stream::iter(conditions)
-                    .fold(Ok(true), |acc, c| async move {
-                        Ok(acc? && Box::pin(c.matches(path)).await?)
-                    })
-                    .await
+            ContextCondition::All(conditions) => conditions.iter().all(|c| c.matches(path)),
+            ContextCondition::Any(conditions) => conditions.iter().any(|c| c.matches(path)),
+            ContextCondition::Not(condition) => !condition.matches(path),
+            ContextCondition::InPath(other_path) => path.is_inside_or_equal_ref(other_path),
+            ContextCondition::InDirectory(dir) => {
+                // `dir` must be a substring and bracketd by either `'/'` or the end of the path.
+                if let Some(pos) = path.path.find(dir) {
+                    let end = pos + dir.len();
+                    (pos == 0 || path.path.as_bytes()[pos - 1] == b'/')
+                        && (end == path.path.len() || path.path.as_bytes()[end] == b'/')
+                } else {
+                    false
+                }
             }
-            ContextCondition::Any(conditions) => {
-                // False positive.
-                #[allow(clippy::manual_try_fold)]
-                stream::iter(conditions)
-                    .fold(Ok(false), |acc, c| async move {
-                        Ok(acc? || Box::pin(c.matches(path)).await?)
-                    })
-                    .await
-            }
-            ContextCondition::Not(condition) => Box::pin(condition.matches(path)).await.map(|b| !b),
-            ContextCondition::InPath(other_path) => Ok(path.is_inside_or_equal_ref(other_path)),
-            ContextCondition::InDirectory(dir) => Ok(path.path.starts_with(&format!("{dir}/"))
-                || path.path.contains(&format!("/{dir}/"))
-                || path.path.ends_with(&format!("/{dir}"))
-                || path.path == *dir),
         }
     }
 }

filename: turbopack/crates/turbopack-node/src/transforms/webpack.rs
patch: @@ -583,8 +583,6 @@ impl EvaluateContext for WebpackLoaderContext {
                     options,
                 );
 
-                let request_str = request.to_string().await?;
-                let lookup_path_str = lookup_path.value_to_string().await?;
                 if let Some(source) = *resolved.first_source().await? {
                     if let Some(path) = self
                         .cwd
@@ -594,12 +592,16 @@ impl EvaluateContext for WebpackLoaderContext {
                     } else {
                         bail!(
                             "Resolving {} in {} ends up on a different filesystem",
-                            request_str,
-                            lookup_path_str
+                            request.to_string().await?,
+                            lookup_path.value_to_string().await?
                         );
                     }
                 } else {
-                    bail!("Unable to resolve {} in {}", request_str, lookup_path_str);
+                    bail!(
+                        "Unable to resolve {} in {}",
+                        request.to_string().await?,
+                        lookup_path.value_to_string().await?
+                    );
                 }
             }
         }

filename: turbopack/crates/turbopack-resolve/src/resolve.rs
patch: @@ -1,6 +1,6 @@
 use anyhow::Result;
 use turbo_rcstr::rcstr;
-use turbo_tasks::Vc;
+use turbo_tasks::{ResolvedVc, Vc};
 use turbo_tasks_fs::{FileSystem, FileSystemPath};
 use turbopack_core::resolve::{
     AliasMap, AliasPattern, ExternalTraced, ExternalType, FindContextFileResult, find_context_file,
@@ -86,53 +86,48 @@ const EDGE_NODE_EXTERNALS: [&str; 5] = ["buffer", "events", "assert", "util", "a
 
 #[turbo_tasks::function]
 async fn base_resolve_options(
-    resolve_path: FileSystemPath,
+    fs: ResolvedVc<Box<dyn FileSystem>>,
     options_context: Vc<ResolveOptionsContext>,
 ) -> Result<Vc<ResolveOptions>> {
-    let parent = resolve_path.parent();
-    if parent != resolve_path {
-        return Ok(base_resolve_options(parent, options_context));
-    }
-    let resolve_path_value = resolve_path.clone();
     let opt = options_context.await?;
     let emulating = opt.emulate_environment;
-    let root = resolve_path_value.fs.root().await?.clone_value();
+    let root = fs.root().await?.clone_value();
     let mut direct_mappings = AliasMap::new();
     let node_externals = if let Some(environment) = emulating {
         environment.node_externals().owned().await?
     } else {
         opt.enable_node_externals
     };
-    if node_externals {
-        for req in NODE_EXTERNALS {
-            direct_mappings.insert(
-                AliasPattern::exact(req),
-                ImportMapping::External(None, ExternalType::CommonJs, ExternalTraced::Untraced)
-                    .resolved_cell(),
-            );
-            direct_mappings.insert(
-                AliasPattern::exact(format!("node:{req}")),
-                ImportMapping::External(None, ExternalType::CommonJs, ExternalTraced::Untraced)
-                    .resolved_cell(),
-            );
+    if node_externals || opt.enable_edge_node_externals {
+        let untraced_external_cell =
+            ImportMapping::External(None, ExternalType::CommonJs, ExternalTraced::Untraced)
+                .resolved_cell();
+
+        if node_externals {
+            for req in NODE_EXTERNALS {
+                direct_mappings.insert(AliasPattern::exact(req), untraced_external_cell);
+                direct_mappings.insert(
+                    AliasPattern::exact(format!("node:{req}")),
+                    untraced_external_cell,
+                );
+            }
         }
-    }
-    if opt.enable_edge_node_externals {
-        for req in EDGE_NODE_EXTERNALS {
-            direct_mappings.insert(
-                AliasPattern::exact(req),
-                ImportMapping::External(
-                    Some(format!("node:{req}").into()),
-                    ExternalType::CommonJs,
-                    ExternalTraced::Untraced,
-                )
-                .resolved_cell(),
-            );
-            direct_mappings.insert(
-                AliasPattern::exact(format!("node:{req}")),
-                ImportMapping::External(None, ExternalType::CommonJs, ExternalTraced::Untraced)
+        if opt.enable_edge_node_externals {
+            for req in EDGE_NODE_EXTERNALS {
+                direct_mappings.insert(
+                    AliasPattern::exact(req),
+                    ImportMapping::External(
+                        Some(format!("node:{req}").into()),
+                        ExternalType::CommonJs,
+                        ExternalTraced::Untraced,
+                    )
                     .resolved_cell(),
-            );
+                );
+                direct_mappings.insert(
+                    AliasPattern::exact(format!("node:{req}")),
+                    untraced_external_cell,
+                );
+            }
         }
     }
 
@@ -284,15 +279,14 @@ pub async fn resolve_options(
 ) -> Result<Vc<ResolveOptions>> {
     let options_context_value = options_context.await?;
     if !options_context_value.rules.is_empty() {
-        let context_value = resolve_path.clone();
         for (condition, new_options_context) in options_context_value.rules.iter() {
-            if condition.matches(&context_value).await? {
-                return Ok(resolve_options(resolve_path.clone(), **new_options_context));
+            if condition.matches(&resolve_path) {
+                return Ok(resolve_options(resolve_path, **new_options_context));
             }
         }
     }
 
-    let resolve_options = base_resolve_options(resolve_path.clone(), options_context);
+    let resolve_options = base_resolve_options(*resolve_path.fs, options_context);
 
     let resolve_options = if options_context_value.enable_typescript {
         let find_tsconfig = async || {

filename: turbopack/crates/turbopack/src/module_options/mod.rs
patch: @@ -76,10 +76,8 @@ impl ModuleOptions {
         } = *module_options_context.await?;
 
         if !rules.is_empty() {
-            let path_value = path.clone();
-
             for (condition, new_context) in rules.iter() {
-                if condition.matches(&path_value).await? {
+                if condition.matches(&path) {
                     return Ok(ModuleOptions::new(
                         path,
                         **new_context,