fix: add is_bare_node_specifier_enabled to NpmResolver trait (#299)

denoland · Oct 5, 2023 · 61456f1 · 61456f1
1 parent cd94491
commit 61456f1
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 22 deletions.
diff --git a/src/graph.rs b/src/graph.rs
@@ -1560,27 +1560,29 @@ fn resolve(
     resolve_import(specifier_text, &referrer_range.specifier)
       .map_err(|err| err.into())
   };
-  use import_map::ImportMapError;
-  use ResolveError::*;
-  use SpecifierError::*;
-  let res_ref = response.as_ref();
-  if matches!(res_ref, Err(Specifier(ImportPrefixMissing(_, _))))
-    || matches!(res_ref, Err(Other(e)) if matches!(e.downcast_ref::<ImportMapError>(), Some(&ImportMapError::UnmappedBareSpecifier(_, _))))
-  {
-    if let Some(npm_resolver) = maybe_npm_resolver {
-      if let Ok(specifier) =
-        ModuleSpecifier::parse(&format!("node:{}", specifier_text))
+  if let Some(npm_resolver) = maybe_npm_resolver {
+    if npm_resolver.enables_bare_builtin_node_module() {
+      use import_map::ImportMapError;
+      use ResolveError::*;
+      use SpecifierError::*;
+      let res_ref = response.as_ref();
+      if matches!(res_ref, Err(Specifier(ImportPrefixMissing(_, _))))
+        || matches!(res_ref, Err(Other(e)) if matches!(e.downcast_ref::<ImportMapError>(), Some(&ImportMapError::UnmappedBareSpecifier(_, _))))
       {
-        if npm_resolver.resolve_builtin_node_module(&specifier).is_ok() {
-          npm_resolver.on_resolve_bare_builtin_node_module(
-            specifier_text,
-            &referrer_range,
-          );
-          return Resolution::from_resolve_result(
-            Ok(specifier),
-            specifier_text,
-            referrer_range,
-          );
+        if let Ok(specifier) =
+          ModuleSpecifier::parse(&format!("node:{}", specifier_text))
+        {
+          if npm_resolver.resolve_builtin_node_module(&specifier).is_ok() {
+            npm_resolver.on_resolve_bare_builtin_node_module(
+              specifier_text,
+              &referrer_range,
+            );
+            return Resolution::from_resolve_result(
+              Ok(specifier),
+              specifier_text,
+              referrer_range,
+            );
+          }
         }
       }
     }

diff --git a/src/lib.rs b/src/lib.rs
@@ -1078,7 +1078,9 @@ console.log(a);
   }
 
   #[derive(Debug, Clone)]
-  struct MockNpmResolver {}
+  struct MockNpmResolver {
+    enables_bare_builtin_node_module: bool,
+  }
 
   impl NpmResolver for MockNpmResolver {
     fn resolve_builtin_node_module(
@@ -1122,6 +1124,10 @@ console.log(a);
     ) -> NpmPackageReqResolution {
       todo!()
     }
+
+    fn enables_bare_builtin_node_module(&self) -> bool {
+      self.enables_bare_builtin_node_module
+    }
   }
 
   #[derive(Debug, Clone)]
@@ -1201,7 +1207,9 @@ console.log(a);
       ],
       "redirects": {}
     });
-    let mock_npm_resolver = MockNpmResolver {};
+    let mock_npm_resolver = MockNpmResolver {
+      enables_bare_builtin_node_module: true,
+    };
     let mock_import_map_resolver = MockImportMapResolver {};
 
     let mut loader = setup(
@@ -1244,6 +1252,27 @@ console.log(a);
       .await;
     assert!(graph.valid().is_ok());
     assert_eq!(json!(graph), expectation);
+
+    let mock_npm_resolver = MockNpmResolver {
+      enables_bare_builtin_node_module: false,
+    };
+    let mut graph = ModuleGraph::new(GraphKind::All);
+    graph
+      .build(
+        vec![root_specifier.clone()],
+        &mut loader,
+        BuildOptions {
+          npm_resolver: Some(&mock_npm_resolver),
+          ..Default::default()
+        },
+      )
+      .await;
+    let res = graph.valid();
+    assert!(res.is_err());
+    assert_eq!(
+      res.unwrap_err().to_string(),
+      "Relative import path \"path\" not prefixed with / or ./ or ../"
+    );
   }
 
   #[tokio::test]

diff --git a/src/source.rs b/src/source.rs
@@ -243,6 +243,11 @@ pub trait NpmResolver: fmt::Debug {
 
   /// Resolves an npm package requirement to a resolved npm package name and version.
   fn resolve_npm(&self, package_req: &PackageReq) -> NpmPackageReqResolution;
+
+  /// Returns true when bare node specifier resoluion is enabled
+  fn enables_bare_builtin_node_module(&self) -> bool {
+    false
+  }
 }
 
 pub fn load_data_url(