Turbopack: don't look into fallback import map twice (#82786)

Fixes a panic where we tried to lookup a `Pattern::Alternatives` in a `fallback_import_map.lookup()` (but the API is that the caller is in charge of unwrapping the alternative and calling lookup multiple times). We can completely skip this in this case, because the individual requests inside the `Request::Alternatives` already looked at the `fallback_import_map`, so no need to check again for the whole alternative as well.

This was a somewhat recent regression:
- #77954 made sure that `var` variables always have a `<dynamic>` variant added, due to their weird scope rules (or lack thereof).
- #81541 then added a `fallback_import_map`

Author: mischnic

turbopack/crates/turbopack-core/src/resolve/mod.rs

@@ -2094,27 +2094,31 @@ async fn resolve_internal_inline(
             }
         };
 
-        // Apply fallback import mappings if provided
-        if let Some(import_map) = &options_value.fallback_import_map
-            && *result.is_unresolvable().await?
-        {
-            let result = import_map
-                .await?
-                .lookup(lookup_path.clone(), request)
-                .await?;
-            let resolved_result = resolve_import_map_result(
-                &result,
-                lookup_path.clone(),
-                lookup_path.clone(),
-                request,
-                options,
-                request.query().owned().await?,
-            )
-            .await?;
-            if let Some(result) = resolved_result
-                && !*result.is_unresolvable().await?
+        // The individual variants inside the alternative already looked at the fallback import
+        // map in the recursive `resolve_internal_inline` calls
+        if !matches!(*request_value, Request::Alternatives { .. }) {
+            // Apply fallback import mappings if provided
+            if let Some(import_map) = &options_value.fallback_import_map
+                && *result.is_unresolvable().await?
             {
-                return Ok(result);
+                let result = import_map
+                    .await?
+                    .lookup(lookup_path.clone(), request)
+                    .await?;
+                let resolved_result = resolve_import_map_result(
+                    &result,
+                    lookup_path.clone(),
+                    lookup_path.clone(),
+                    request,
+                    options,
+                    request.query().owned().await?,
+                )
+                .await?;
+                if let Some(result) = resolved_result
+                    && !*result.is_unresolvable().await?
+                {
+                    return Ok(result);
+                }
             }
         }
 

turbopack/crates/turbopack-tests/tests/execution.rs

@@ -392,6 +392,17 @@ async fn run_test_operation(prepared_test: ResolvedVc<PreparedTest>) -> Result<V
             .resolved_cell(),
     );
 
+    let mut fallback_import_map = ImportMap::empty();
+    fallback_import_map.insert_exact_alias(
+        rcstr!("fallback"),
+        ImportMapping::External(
+            None,
+            ExternalType::EcmaScriptModule,
+            ExternalTraced::Untraced,
+        )
+        .resolved_cell(),
+    );
+
     let remove_unused_exports = options.remove_unused_exports.unwrap_or(true);
 
     let asset_context: Vc<Box<dyn AssetContext>> = Vc::upcast(ModuleAssetContext::new(
@@ -435,6 +446,7 @@ async fn run_test_operation(prepared_test: ResolvedVc<PreparedTest>) -> Result<V
             browser: true,
             module: true,
             import_map: Some(import_map.resolved_cell()),
+            fallback_import_map: Some(fallback_import_map.resolved_cell()),
             ..Default::default()
         }
         .cell(),

turbopack/crates/turbopack-tests/tests/execution/turbopack/resolving/dynamic/input/index.js

@@ -0,0 +1,19 @@
+let names = ['cannot-resolve']
+if (globalThis.foo != undefined) {
+  // ensure `<dynamic>` is part of the array
+  names.push(globalThis.foo)
+}
+
+let result
+for (let name of names) {
+  try {
+    result = require(name).Iconv
+    return
+  } catch {}
+}
+
+console.log(result)
+
+it('should correctly handle potentially completely dynamic requests', () => {
+  expect(result).toBeUndefined()
+})

turbopack/crates/turbopack-tests/tests/execution/turbopack/resolving/dynamic/issues/__l___Module not found____c__ Can't resolve __c_('-fa7799.txt

@@ -0,0 +1,21 @@
+warning - [resolve] /turbopack/crates/turbopack-tests/tests/execution/turbopack/resolving/dynamic/input/index.js:10:13  Module not found: Can't resolve ('cannot-resolve' | <dynamic>)
+  
+       6 | 
+       7 | let result
+       8 | for (let name of names) {
+       9 |   try {
+         +              v-----------v
+      10 +     result = require(name).Iconv
+         +              ^-----------^
+      11 |     return
+      12 |   } catch {}
+      13 | }
+      14 | 
+  
+  
+  
+  | It was not possible to find the requested file.
+  | Parsed request as written in source code: module "'cannot-resolve'" or dynamic
+  | Path where resolving has started: [project]/turbopack/crates/turbopack-tests/tests/execution/turbopack/resolving/dynamic/input/index.js
+  | Type of request: commonjs request
+  |