feat(npm): require --unstable for npm specifiers in remote modules (#16612)

cli/graph_util.rs

@@ -80,9 +80,7 @@ impl GraphData {
     let mut has_npm_specifier_in_graph = false;
 
     for (specifier, result) in graph.specifiers() {
-      if specifier.scheme() == "npm"
+      if NpmPackageReference::from_specifier(&specifier).is_ok() {
-        && NpmPackageReference::from_specifier(&specifier).is_ok()
-      {
         has_npm_specifier_in_graph = true;
         continue;
       }
@@ -230,9 +228,7 @@ impl GraphData {
           for (dep_specifier, dep) in dependencies.iter().rev() {
             // todo(dsherret): ideally there would be a way to skip external dependencies
             // in the graph here rather than specifically npm package references
-            if dep_specifier.starts_with("npm:")
+            if NpmPackageReference::from_str(dep_specifier).is_ok() {
-              && NpmPackageReference::from_str(dep_specifier).is_ok()
-            {
               continue;
             }
 

cli/npm/resolution/specifier.rs

@@ -35,10 +35,9 @@ impl NpmPackageReference {
     let specifier = match specifier.strip_prefix("npm:") {
       Some(s) => s,
       None => {
-        return Err(generic_error(format!(
+        // don't allocate a string here and instead use a static string
-          "Not an npm specifier: {}",
+        // because this is hit a lot when a url is not an npm specifier
-          specifier
+        return Err(generic_error("Not an npm specifier"));
-        )));
       }
     };
     let parts = specifier.split('/').collect::<Vec<_>>();
@@ -244,11 +243,8 @@ pub fn resolve_npm_package_reqs(graph: &ModuleGraph) -> Vec<NpmPackageReq> {
 
     // fill this leaf's information
     for specifier in &specifiers {
-      if specifier.scheme() == "npm" {
-        // this will error elsewhere if not the case
       if let Ok(npm_ref) = NpmPackageReference::from_specifier(specifier) {
         leaf.reqs.insert(npm_ref.req);
-        }
       } else if !specifier.as_str().starts_with(&parent_specifier.as_str()) {
         leaf
           .dependencies

cli/proc_state.rs

@@ -524,6 +524,15 @@ impl ProcState {
         Some(Resolved::Ok { specifier, .. }) => {
           if let Ok(reference) = NpmPackageReference::from_specifier(specifier)
           {
+            if !self.options.unstable()
+              && matches!(found_referrer.scheme(), "http" | "https")
+            {
+              return Err(custom_error(
+                "NotSupported",
+                format!("importing npm specifiers in remote modules requires the --unstable flag (referrer: {})", found_referrer),
+              ));
+            }
+
             return self
               .handle_node_resolve_result(node::node_resolve_npm_reference(
                 &reference,

cli/tests/integration/npm_tests.rs

@@ -179,6 +179,14 @@ itest!(sub_paths {
   http_server: true,
 });
 
+itest!(remote_npm_specifier {
+  args: "run --quiet npm/remote_npm_specifier/main.ts",
+  output: "npm/remote_npm_specifier/main.out",
+  envs: env_vars(),
+  http_server: true,
+  exit_code: 1,
+});
+
 itest!(tarball_with_global_header {
   args: "run -A --quiet npm/tarball_with_global_header/main.js",
   output: "npm/tarball_with_global_header/main.out",

cli/tests/testdata/npm/remote_npm_specifier/main.out

@@ -0,0 +1 @@
+error: importing npm specifiers in remote modules requires the --unstable flag (referrer: http://localhost:4545/npm/remote_npm_specifier/remote.ts)

cli/tests/testdata/npm/remote_npm_specifier/main.ts

@@ -0,0 +1 @@
+import "http://localhost:4545/npm/remote_npm_specifier/remote.ts";

cli/tests/testdata/npm/remote_npm_specifier/remote.ts

@@ -0,0 +1,3 @@
+import chalk from "npm:chalk";
+
+console.log(chalk.green("test"));