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

7 files changed, 29 insertions, 15 deletions

diff --git a/cli/graph_util.rs b/cli/graph_util.rs
index cfd79d491..b919d5042 100644
--- a/cli/graph_util.rs
+++ b/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"
-        && NpmPackageReference::from_specifier(&specifier).is_ok()
-      {
+      if 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:")
-              && NpmPackageReference::from_str(dep_specifier).is_ok()
-            {
+            if NpmPackageReference::from_str(dep_specifier).is_ok() {
               continue;
             }
 
diff --git a/cli/npm/resolution/specifier.rs b/cli/npm/resolution/specifier.rs
index c529d393b..efcea9d99 100644
--- a/cli/npm/resolution/specifier.rs
+++ b/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!(
-          "Not an npm specifier: {}",
-          specifier
-        )));
+        // don't allocate a string here and instead use a static string
+        // because this is hit a lot when a url is not an npm 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);
-        }
+      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
diff --git a/cli/proc_state.rs b/cli/proc_state.rs
index e50a4bdba..1558f5865 100644
--- a/cli/proc_state.rs
+++ b/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,
diff --git a/cli/tests/integration/npm_tests.rs b/cli/tests/integration/npm_tests.rs
index ced9ad8a3..07460d990 100644
--- a/cli/tests/integration/npm_tests.rs
+++ b/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",
diff --git a/cli/tests/testdata/npm/remote_npm_specifier/main.out b/cli/tests/testdata/npm/remote_npm_specifier/main.out
new file mode 100644
index 000000000..0cb08b7bc
--- /dev/null
+++ b/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)
diff --git a/cli/tests/testdata/npm/remote_npm_specifier/main.ts b/cli/tests/testdata/npm/remote_npm_specifier/main.ts
new file mode 100644
index 000000000..20a99b688
--- /dev/null
+++ b/cli/tests/testdata/npm/remote_npm_specifier/main.ts
@@ -0,0 +1 @@
+import "http://localhost:4545/npm/remote_npm_specifier/remote.ts";
diff --git a/cli/tests/testdata/npm/remote_npm_specifier/remote.ts b/cli/tests/testdata/npm/remote_npm_specifier/remote.ts
new file mode 100644
index 000000000..923ed3ed8
--- /dev/null
+++ b/cli/tests/testdata/npm/remote_npm_specifier/remote.ts
@@ -0,0 +1,3 @@
+import chalk from "npm:chalk";
+
+console.log(chalk.green("test"));