fix(lsp): improved cjs tracking (#23374)

Our cjs tracking was a bit broken. It was marking stuff as esm that was
actually cjs leading to type checking errors.

Cargo.lock

@@ -2641,10 +2641,11 @@ checksum = "c007b1ae3abe1cb6f85a16305acd418b7ca6343b953633fee2b76d8f108b830f"
 
 [[package]]
 name = "file_test_runner"
-version = "0.2.0"
+version = "0.4.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "e13bacb20a988be248a13adc6011bce718648f78d4684d4dd0444d05256f4a7b"
+checksum = "b66e9ef00f9f6b82b030b7a9d659030f73498921d4c021b0772e75dfd7090d80"
 dependencies = [
+ "anyhow",
  "crossbeam-channel",
  "deno_terminal",
  "parking_lot 0.12.1",

cli/lsp/documents.rs

@@ -12,6 +12,7 @@ use super::tsc::AssetDocument;
 use crate::args::package_json;
 use crate::cache::HttpCache;
 use crate::jsr::JsrCacheResolver;
+use crate::lsp::logging::lsp_warn;
 use crate::npm::CliNpmResolver;
 use crate::resolver::CliGraphResolver;
 use crate::resolver::CliGraphResolverOptions;
@@ -43,7 +44,6 @@ use deno_semver::jsr::JsrPackageReqReference;
 use deno_semver::npm::NpmPackageReqReference;
 use deno_semver::package::PackageReq;
 use indexmap::IndexMap;
-use once_cell::sync::Lazy;
 use package_json::PackageJsonDepsProvider;
 use std::borrow::Cow;
 use std::collections::BTreeSet;
@@ -57,36 +57,6 @@ use std::sync::atomic::Ordering;
 use std::sync::Arc;
 use tower_lsp::lsp_types as lsp;
 
-static JS_HEADERS: Lazy<HashMap<String, String>> = Lazy::new(|| {
-  ([(
-    "content-type".to_string(),
-    "application/javascript".to_string(),
-  )])
-  .into_iter()
-  .collect()
-});
-
-static JSX_HEADERS: Lazy<HashMap<String, String>> = Lazy::new(|| {
-  ([("content-type".to_string(), "text/jsx".to_string())])
-    .into_iter()
-    .collect()
-});
-
-static TS_HEADERS: Lazy<HashMap<String, String>> = Lazy::new(|| {
-  ([(
-    "content-type".to_string(),
-    "application/typescript".to_string(),
-  )])
-  .into_iter()
-  .collect()
-});
-
-static TSX_HEADERS: Lazy<HashMap<String, String>> = Lazy::new(|| {
-  ([("content-type".to_string(), "text/tsx".to_string())])
-    .into_iter()
-    .collect()
-});
-
 pub const DOCUMENT_SCHEMES: [&str; 5] =
   ["data", "blob", "file", "http", "https"];
 
@@ -103,18 +73,6 @@ pub enum LanguageId {
 }
 
 impl LanguageId {
-  pub fn as_media_type(&self) -> MediaType {
-    match self {
-      LanguageId::JavaScript => MediaType::JavaScript,
-      LanguageId::Jsx => MediaType::Jsx,
-      LanguageId::TypeScript => MediaType::TypeScript,
-      LanguageId::Tsx => MediaType::Tsx,
-      LanguageId::Json => MediaType::Json,
-      LanguageId::JsonC => MediaType::Json,
-      LanguageId::Markdown | LanguageId::Unknown => MediaType::Unknown,
-    }
-  }
-
   pub fn as_extension(&self) -> Option<&'static str> {
     match self {
       LanguageId::JavaScript => Some("js"),
@@ -128,13 +86,15 @@ impl LanguageId {
     }
   }
 
-  fn as_headers(&self) -> Option<&HashMap<String, String>> {
+  pub fn as_content_type(&self) -> Option<&'static str> {
     match self {
-      Self::JavaScript => Some(&JS_HEADERS),
-      Self::Jsx => Some(&JSX_HEADERS),
-      Self::TypeScript => Some(&TS_HEADERS),
-      Self::Tsx => Some(&TSX_HEADERS),
-      _ => None,
+      LanguageId::JavaScript => Some("application/javascript"),
+      LanguageId::Jsx => Some("text/jsx"),
+      LanguageId::TypeScript => Some("application/typescript"),
+      LanguageId::Tsx => Some("text/tsx"),
+      LanguageId::Json | LanguageId::JsonC => Some("application/json"),
+      LanguageId::Markdown => Some("text/markdown"),
+      LanguageId::Unknown => None,
     }
   }
 
@@ -291,6 +251,7 @@ pub struct Document {
   // so having a mutex to hold it is ok
   maybe_navigation_tree: Mutex<Option<Arc<tsc::NavigationTree>>>,
   maybe_parsed_source: Option<ParsedSourceResult>,
+  media_type: MediaType,
   specifier: ModuleSpecifier,
   text_info: SourceTextInfo,
 }
@@ -302,15 +263,23 @@ impl Document {
     maybe_headers: Option<HashMap<String, String>>,
     text_info: SourceTextInfo,
     resolver: &dyn deno_graph::source::Resolver,
+    maybe_node_resolver: Option<&CliNodeResolver>,
     npm_resolver: &dyn deno_graph::source::NpmResolver,
   ) -> Arc<Self> {
     // we only ever do `Document::new` on disk resources that are supposed to
     // be diagnosable, unlike `Document::open`, so it is safe to unconditionally
     // parse the module.
+    let media_type = resolve_media_type(
+      &specifier,
+      maybe_headers.as_ref(),
+      None,
+      maybe_node_resolver,
+    );
     let (maybe_parsed_source, maybe_module) = parse_and_analyze_module(
       &specifier,
       text_info.clone(),
       maybe_headers.as_ref(),
+      media_type,
       resolver,
       npm_resolver,
     );
@@ -328,6 +297,7 @@ impl Document {
       maybe_navigation_tree: Mutex::new(None),
       maybe_parsed_source: maybe_parsed_source
         .filter(|_| specifier.scheme() == "file"),
+      media_type,
       text_info,
       specifier,
     })
@@ -336,12 +306,27 @@ impl Document {
   fn maybe_with_new_resolver(
     &self,
     resolver: &dyn deno_graph::source::Resolver,
+    maybe_node_resolver: Option<&CliNodeResolver>,
     npm_resolver: &dyn deno_graph::source::NpmResolver,
   ) -> Option<Arc<Self>> {
-    let parsed_source_result = match &self.maybe_parsed_source {
+    let mut parsed_source_result = match &self.maybe_parsed_source {
       Some(parsed_source_result) => parsed_source_result.clone(),
       None => return None, // nothing to change
     };
+    let media_type = resolve_media_type(
+      &self.specifier,
+      self.maybe_headers.as_ref(),
+      self.maybe_language_id,
+      maybe_node_resolver,
+    );
+    // reparse if the media type has changed
+    if let Ok(parsed_source) = &parsed_source_result {
+      if parsed_source.media_type() != media_type {
+        parsed_source_result =
+          parse_source(&self.specifier, self.text_info.clone(), media_type);
+      }
+    }
+
     let maybe_module = Some(analyze_module(
       &self.specifier,
       &parsed_source_result,
@@ -363,27 +348,37 @@ impl Document {
       maybe_headers: self.maybe_headers.clone(),
       maybe_language_id: self.maybe_language_id,
       maybe_lsp_version: self.maybe_lsp_version,
+      media_type,
       text_info: self.text_info.clone(),
       specifier: self.specifier.clone(),
     }))
   }
 
+  #[allow(clippy::too_many_arguments)]
   fn open(
     specifier: ModuleSpecifier,
     version: i32,
     language_id: LanguageId,
+    maybe_headers: Option<HashMap<String, String>>,
     content: Arc<str>,
     cache: &Arc<dyn HttpCache>,
     resolver: &dyn deno_graph::source::Resolver,
+    maybe_node_resolver: Option<&CliNodeResolver>,
     npm_resolver: &dyn deno_graph::source::NpmResolver,
   ) -> Arc<Self> {
-    let maybe_headers = language_id.as_headers();
     let text_info = SourceTextInfo::new(content);
+    let media_type = resolve_media_type(
+      &specifier,
+      None,
+      Some(language_id),
+      maybe_node_resolver,
+    );
     let (maybe_parsed_source, maybe_module) = if language_id.is_diagnosable() {
       parse_and_analyze_module(
         &specifier,
         text_info.clone(),
-        maybe_headers,
+        maybe_headers.as_ref(),
+        media_type,
         resolver,
         npm_resolver,
       )
@@ -400,11 +395,12 @@ impl Document {
       line_index,
       maybe_language_id: Some(language_id),
       maybe_lsp_version: Some(version),
-      maybe_headers: maybe_headers.map(ToOwned::to_owned),
+      maybe_headers,
       maybe_module,
       maybe_navigation_tree: Mutex::new(None),
       maybe_parsed_source: maybe_parsed_source
         .filter(|_| specifier.scheme() == "file"),
+      media_type,
       text_info,
       specifier,
     })
@@ -434,20 +430,18 @@ impl Document {
       }
     }
     let text_info = SourceTextInfo::from_string(content);
+    let media_type = self.media_type;
     let (maybe_parsed_source, maybe_module) = if self
       .maybe_language_id
       .as_ref()
       .map(|li| li.is_diagnosable())
       .unwrap_or(false)
     {
-      let maybe_headers = self
-        .maybe_language_id
-        .as_ref()
-        .and_then(|li| li.as_headers());
       parse_and_analyze_module(
         &self.specifier,
         text_info.clone(),
-        maybe_headers,
+        self.maybe_headers.as_ref(),
+        media_type,
         resolver,
         npm_resolver,
       )
@@ -477,6 +471,7 @@ impl Document {
         .filter(|_| self.specifier.scheme() == "file"),
       maybe_lsp_version: Some(version),
       maybe_navigation_tree: Mutex::new(None),
+      media_type,
     }))
   }
 
@@ -494,6 +489,7 @@ impl Document {
       maybe_parsed_source: self.maybe_parsed_source.clone(),
       maybe_lsp_version: self.maybe_lsp_version,
       maybe_navigation_tree: Mutex::new(None),
+      media_type: self.media_type,
     })
   }
 
@@ -554,18 +550,7 @@ impl Document {
   }
 
   pub fn media_type(&self) -> MediaType {
-    if let Some(Ok(module)) = &self.maybe_module {
-      return module.media_type;
-    }
-    let specifier_media_type = MediaType::from_specifier(&self.specifier);
-    if specifier_media_type != MediaType::Unknown {
-      return specifier_media_type;
-    }
-
-    self
-      .maybe_language_id
-      .map(|id| id.as_media_type())
-      .unwrap_or(MediaType::Unknown)
+    self.media_type
   }
 
   pub fn maybe_language_id(&self) -> Option<LanguageId> {
@@ -629,6 +614,39 @@ impl Document {
   }
 }
 
+fn resolve_media_type(
+  specifier: &ModuleSpecifier,
+  maybe_headers: Option<&HashMap<String, String>>,
+  maybe_language_id: Option<LanguageId>,
+  maybe_node_resolver: Option<&CliNodeResolver>,
+) -> MediaType {
+  if let Some(node_resolver) = maybe_node_resolver {
+    if node_resolver.in_npm_package(specifier) {
+      match node_resolver.url_to_node_resolution(specifier.clone()) {
+        Ok(resolution) => {
+          let (_, media_type) =
+            NodeResolution::into_specifier_and_media_type(Some(resolution));
+          return media_type;
+        }
+        Err(err) => {
+          lsp_warn!("Node resolution failed for '{}': {}", specifier, err);
+        }
+      }
+    }
+  }
+
+  if maybe_headers.is_some() {
+    return MediaType::from_specifier_and_headers(specifier, maybe_headers);
+  }
+
+  // LanguageId is a subset of MediaType, so get its content type and
+  // also use the specifier to inform its media type
+  MediaType::from_specifier_and_content_type(
+    specifier,
+    maybe_language_id.and_then(|id| id.as_content_type()),
+  )
+}
+
 pub fn to_lsp_range(range: &deno_graph::Range) -> lsp::Range {
   lsp::Range {
     start: lsp::Position {
@@ -712,6 +730,7 @@ impl FileSystemDocuments {
     cache: &Arc<dyn HttpCache>,
     resolver: &dyn deno_graph::source::Resolver,
     specifier: &ModuleSpecifier,
+    maybe_node_resolver: Option<&CliNodeResolver>,
     npm_resolver: &dyn deno_graph::source::NpmResolver,
   ) -> Option<Arc<Document>> {
     let fs_version = if specifier.scheme() == "data" {
@@ -724,7 +743,13 @@ impl FileSystemDocuments {
       != fs_version
     {
       // attempt to update the file on the file system
-      self.refresh_document(cache, resolver, specifier, npm_resolver)
+      self.refresh_document(
+        cache,
+        resolver,
+        specifier,
+        maybe_node_resolver,
+        npm_resolver,
+      )
     } else {
       file_system_doc
     }
@@ -737,6 +762,7 @@ impl FileSystemDocuments {
     cache: &Arc<dyn HttpCache>,
     resolver: &dyn deno_graph::source::Resolver,
     specifier: &ModuleSpecifier,
+    maybe_node_resolver: Option<&CliNodeResolver>,
     npm_resolver: &dyn deno_graph::source::NpmResolver,
   ) -> Option<Arc<Document>> {
     let doc = if specifier.scheme() == "file" {
@@ -751,6 +777,7 @@ impl FileSystemDocuments {
         None,
         SourceTextInfo::from_string(content),
         resolver,
+        maybe_node_resolver,
         npm_resolver,
       )
     } else if specifier.scheme() == "data" {
@@ -764,6 +791,7 @@ impl FileSystemDocuments {
         None,
         SourceTextInfo::from_string(source),
         resolver,
+        maybe_node_resolver,
         npm_resolver,
       )
     } else {
@@ -791,6 +819,7 @@ impl FileSystemDocuments {
         maybe_headers,
         SourceTextInfo::from_string(content),
         resolver,
+        maybe_node_resolver,
         npm_resolver,
       )
     };
@@ -837,6 +866,8 @@ pub struct Documents {
   /// Any imports to the context supplied by configuration files. This is like
   /// the imports into the a module graph in CLI.
   imports: Arc<IndexMap<ModuleSpecifier, GraphImport>>,
+  /// Resolver for node_modules.
+  maybe_node_resolver: Option<Arc<CliNodeResolver>>,
   /// A resolver that takes into account currently loaded import map and JSX
   /// settings.
   resolver: Arc<CliGraphResolver>,
@@ -861,6 +892,7 @@ impl Documents {
       open_docs: HashMap::default(),
       file_system_docs: Default::default(),
       imports: Default::default(),
+      maybe_node_resolver: None,
       resolver: Arc::new(CliGraphResolver::new(CliGraphResolverOptions {
         node_resolver: None,
         npm_resolver: None,
@@ -905,9 +937,14 @@ impl Documents {
       specifier.clone(),
       version,
       language_id,
+      // todo(dsherret): don't we want to pass in the headers from
+      // the cache for remote modules here in order to get the
+      // x-typescript-types?
+      None,
       content,
       &self.cache,
       resolver,
+      self.maybe_node_resolver.as_deref(),
       npm_resolver,
     );
 
@@ -1102,6 +1139,7 @@ impl Documents {
         &self.cache,
         self.get_resolver(),
         &specifier,
+        self.maybe_node_resolver.as_deref(),
         self.get_npm_resolver(),
       )
     }
@@ -1281,6 +1319,7 @@ impl Documents {
   ) {
     let config_data = config.tree.root_data();
     let config_file = config_data.and_then(|d| d.config_file.as_deref());
+    self.maybe_node_resolver = node_resolver.clone();
     self.resolver = Arc::new(CliGraphResolver::new(CliGraphResolverOptions {
       node_resolver,
       npm_resolver,
@@ -1352,9 +1391,11 @@ impl Documents {
         if !config.specifier_enabled(doc.specifier()) {
           continue;
         }
-        if let Some(new_doc) =
-          doc.maybe_with_new_resolver(resolver, npm_resolver)
-        {
+        if let Some(new_doc) = doc.maybe_with_new_resolver(
+          resolver,
+          self.maybe_node_resolver.as_deref(),
+          npm_resolver,
+        ) {
           *doc = new_doc;
         }
       }
@@ -1362,9 +1403,11 @@ impl Documents {
         if !config.specifier_enabled(doc.specifier()) {
           continue;
         }
-        if let Some(new_doc) =
-          doc.maybe_with_new_resolver(resolver, npm_resolver)
-        {
+        if let Some(new_doc) = doc.maybe_with_new_resolver(
+          resolver,
+          self.maybe_node_resolver.as_deref(),
+          npm_resolver,
+        ) {
           *doc.value_mut() = new_doc;
         }
       }
@@ -1385,6 +1428,7 @@ impl Documents {
             &self.cache,
             resolver,
             specifier,
+            self.maybe_node_resolver.as_deref(),
             npm_resolver,
           );
         }
@@ -1628,10 +1672,11 @@ fn parse_and_analyze_module(
   specifier: &ModuleSpecifier,
   text_info: SourceTextInfo,
   maybe_headers: Option<&HashMap<String, String>>,
+  media_type: MediaType,
   resolver: &dyn deno_graph::source::Resolver,
   npm_resolver: &dyn deno_graph::source::NpmResolver,
 ) -> (Option<ParsedSourceResult>, Option<ModuleResult>) {
-  let parsed_source_result = parse_source(specifier, text_info, maybe_headers);
+  let parsed_source_result = parse_source(specifier, text_info, media_type);
   let module_result = analyze_module(
     specifier,
     &parsed_source_result,
@@ -1645,12 +1690,12 @@ fn parse_and_analyze_module(
 fn parse_source(
   specifier: &ModuleSpecifier,
   text_info: SourceTextInfo,
-  maybe_headers: Option<&HashMap<String, String>>,
+  media_type: MediaType,
 ) -> ParsedSourceResult {
   deno_ast::parse_module(deno_ast::ParseParams {
     specifier: specifier.clone(),
     text_info,
-    media_type: MediaType::from_specifier_and_headers(specifier, maybe_headers),
+    media_type,
     capture_tokens: true,
     scope_analysis: true,
     maybe_syntax: None,

cli/lsp/tsc.rs

@@ -3996,6 +3996,7 @@ struct LoadResponse {
   data: Arc<str>,
   script_kind: i32,
   version: Option<String>,
+  is_cjs: bool,
 }
 
 #[op2]
@@ -4018,6 +4019,10 @@ fn op_load<'s>(
         data: doc.text(),
         script_kind: crate::tsc::as_ts_script_kind(doc.media_type()),
         version: state.script_version(&specifier),
+        is_cjs: matches!(
+          doc.media_type(),
+          MediaType::Cjs | MediaType::Cts | MediaType::Dcts
+        ),
       })
     };
 

cli/resolver.rs

@@ -223,6 +223,13 @@ impl CliNodeResolver {
     Ok(specifier)
   }
 
+  pub fn url_to_node_resolution(
+    &self,
+    specifier: ModuleSpecifier,
+  ) -> Result<NodeResolution, AnyError> {
+    self.node_resolver.url_to_node_resolution(specifier)
+  }
+
   fn handle_node_resolve_result(
     &self,
     result: Result<Option<NodeResolution>, AnyError>,

cli/tsc/99_main_compiler.js

@@ -135,12 +135,16 @@ delete Object.prototype.__proto__;
     #cache = new Set();
 
     /** @param {[string, ts.Extension]} param */
-    add([specifier, ext]) {
+    maybeAdd([specifier, ext]) {
       if (ext === ".cjs" || ext === ".d.cts" || ext === ".cts") {
         this.#cache.add(specifier);
       }
     }
 
+    add(specifier) {
+      this.#cache.add(specifier);
+    }
+
     /** @param specifier {string} */
     has(specifier) {
       return this.#cache.has(specifier);
@@ -603,22 +607,30 @@ delete Object.prototype.__proto__;
         return sourceFile;
       }
 
-      /** @type {{ data: string; scriptKind: ts.ScriptKind; version: string; }} */
+      /** @type {{ data: string; scriptKind: ts.ScriptKind; version: string; isCjs: boolean }} */
       const fileInfo = ops.op_load(specifier);
       if (!fileInfo) {
         return undefined;
       }
-      const { data, scriptKind, version } = fileInfo;
+      let { data, scriptKind, version, isCjs } = fileInfo;
       assert(
         data != null,
         `"data" is unexpectedly null for "${specifier}".`,
       );
+
+      // use the cache for non-lsp
+      if (isCjs == null) {
+        isCjs = isCjsCache.has(specifier);
+      } else if (isCjs) {
+        isCjsCache.add(specifier);
+      }
+
       sourceFile = ts.createSourceFile(
         specifier,
         data,
         {
           ...getCreateSourceFileOptions(languageVersion),
-          impliedNodeFormat: isCjsCache.has(specifier)
+          impliedNodeFormat: isCjs
             ? ts.ModuleKind.CommonJS
             : ts.ModuleKind.ESNext,
           // no need to parse docs for `deno check`
@@ -688,7 +700,7 @@ delete Object.prototype.__proto__;
             base: containingFilePath,
           })?.[0];
           if (resolved) {
-            isCjsCache.add(resolved);
+            isCjsCache.maybeAdd(resolved);
             return {
               primary: true,
               resolvedFileName: resolved[0],
@@ -723,7 +735,7 @@ delete Object.prototype.__proto__;
       if (resolved) {
         const result = resolved.map((item) => {
           if (item) {
-            isCjsCache.add(item);
+            isCjsCache.maybeAdd(item);
             const [resolvedFileName, extension] = item;
             return {
               resolvedFileName,

tests/Cargo.toml

@@ -43,7 +43,7 @@ deno_lockfile.workspace = true
 deno_terminal.workspace = true
 deno_tls.workspace = true
 fastwebsockets = { workspace = true, features = ["upgrade", "unstable-split"] }
-file_test_runner = "0.2.0"
+file_test_runner = "0.4.0"
 flaky_test = "=0.1.0"
 http.workspace = true
 http-body-util.workspace = true

tests/integration/lsp_tests.rs

@@ -12287,3 +12287,52 @@ fn lsp_uses_lockfile_for_npm_initialization() {
   assert_eq!(skipping_count, 1);
   client.shutdown();
 }
+
+#[test]
+fn lsp_cjs_internal_types_default_export() {
+  let context = TestContextBuilder::new()
+    .use_http_server()
+    .use_temp_cwd()
+    .add_npm_env_vars()
+    .env("DENO_FUTURE", "1")
+    .build();
+  let temp_dir = context.temp_dir();
+  temp_dir.write("deno.json", r#"{}"#);
+  temp_dir.write(
+    "package.json",
+    r#"{
+  "dependencies": {
+    "@denotest/cjs-internal-types-default-export": "*"
+  }
+}"#,
+  );
+  context.run_npm("install");
+
+  let mut client = context.new_lsp_command().build();
+  client.initialize_default();
+  // this was previously being resolved as ESM and not correctly as CJS
+  let node_modules_index_d_ts = temp_dir.path().join(
+    "node_modules/@denotest/cjs-internal-types-default-export/index.d.ts",
+  );
+  client.did_open(json!({
+    "textDocument": {
+      "uri": node_modules_index_d_ts.uri_file(),
+      "languageId": "typescript",
+      "version": 1,
+      "text": node_modules_index_d_ts.read_to_string(),
+    }
+  }));
+  let main_url = temp_dir.path().join("main.ts").uri_file();
+  let diagnostics = client.did_open(
+    json!({
+      "textDocument": {
+        "uri": main_url,
+        "languageId": "typescript",
+        "version": 1,
+        "text": "import * as mod from '@denotest/cjs-internal-types-default-export';\nmod.add(1, 2);",
+      }
+    }),
+  );
+  // previously, diagnostic about `add` not being callable
+  assert_eq!(json!(diagnostics.all()), json!([]));
+}

tests/specs/mod.rs

@@ -9,6 +9,10 @@ use std::sync::Arc;
 
 use deno_core::anyhow::Context;
 use deno_core::serde_json;
+use file_test_runner::collection::collect_tests_or_exit;
+use file_test_runner::collection::strategies::TestPerDirectoryCollectionStrategy;
+use file_test_runner::collection::CollectOptions;
+use file_test_runner::collection::CollectedTest;
 use serde::Deserialize;
 use test_util::tests_path;
 use test_util::PathRef;
@@ -69,6 +73,7 @@ struct StepMetaData {
   pub clean_deno_dir: bool,
   pub args: VecOrString,
   pub cwd: Option<String>,
+  pub command_name: Option<String>,
   #[serde(default)]
   pub envs: HashMap<String, String>,
   pub output: String,
@@ -77,15 +82,13 @@ struct StepMetaData {
 }
 
 pub fn main() {
-  let root_category =
-    file_test_runner::collect_tests_or_exit(file_test_runner::CollectOptions {
-      base: tests_path().join("specs").to_path_buf(),
-      strategy: file_test_runner::FileCollectionStrategy::TestPerDirectory {
-        file_name: MANIFEST_FILE_NAME.to_string(),
-      },
-      root_category_name: "specs".to_string(),
-      filter_override: None,
-    });
+  let root_category = collect_tests_or_exit(CollectOptions {
+    base: tests_path().join("specs").to_path_buf(),
+    strategy: Box::new(TestPerDirectoryCollectionStrategy {
+      file_name: MANIFEST_FILE_NAME.to_string(),
+    }),
+    filter_override: None,
+  });
 
   if root_category.is_empty() {
     return; // all tests filtered out
@@ -111,15 +114,13 @@ pub fn main() {
           output.extend(panic_output);
           file_test_runner::TestResult::Failed { output }
         }
+        file_test_runner::TestResult::Steps(_) => unreachable!(),
       }
     }),
   );
 }
 
-fn run_test(
-  test: &file_test_runner::CollectedTest,
-  diagnostic_logger: Rc<RefCell<Vec<u8>>>,
-) {
+fn run_test(test: &CollectedTest, diagnostic_logger: Rc<RefCell<Vec<u8>>>) {
   let metadata_path = PathRef::new(&test.path);
   let metadata_value = metadata_path.read_jsonc_value();
   // checking for "steps" leads to a more targeted error message
@@ -182,6 +183,10 @@ fn run_test(
       Some(cwd) => command.current_dir(cwd),
       None => command,
     };
+    let command = match &step.command_name {
+      Some(command_name) => command.name(command_name),
+      None => command,
+    };
     let output = command.run();
     if step.output.ends_with(".out") {
       let test_output_path = cwd.join(&step.output);

tests/specs/npm/cjs_internal_types_default_export/__test__.jsonc

@@ -0,0 +1,15 @@
+{
+  "tempDir": true,
+  "envs": {
+    "DENO_FUTURE": "1"
+  },
+  "steps": [{
+    "commandName": "npm",
+    "args": "install",
+    "output": "[WILDCARD]"
+  }, {
+    "args": "check main.ts",
+    "exitCode": 1,
+    "output": "main.out"
+  }]
+}

tests/specs/npm/cjs_internal_types_default_export/main.out

@@ -0,0 +1,5 @@
+Check file:///[WILDLINE]/main.ts
+error: TS2345 [ERROR]: Argument of type 'string' is not assignable to parameter of type 'number'.
+add(1, "test"); // should error
+       ~~~~~~
+    at file:///[WILDLINE]/main.ts:3:8

tests/specs/npm/cjs_internal_types_default_export/main.ts

@@ -0,0 +1,3 @@
+import { add } from "@denotest/cjs-internal-types-default-export";
+
+add(1, "test"); // should error

tests/specs/npm/cjs_internal_types_default_export/package.json

@@ -0,0 +1,5 @@
+{
+  "dependencies": {
+    "@denotest/cjs-internal-types-default-export": "*"
+  }
+}

tests/testdata/npm/registry/@denotest/cjs-internal-types-default-export/1.0.0/add.d.ts

@@ -0,0 +1,3 @@
+const _default: (a: number, b: number) => number;
+
+export default _default;

tests/testdata/npm/registry/@denotest/cjs-internal-types-default-export/1.0.0/index.d.ts

@@ -0,0 +1 @@
+export { default as add } from './add';

tests/testdata/npm/registry/@denotest/cjs-internal-types-default-export/1.0.0/index.js

@@ -0,0 +1 @@
+module.exports.add = (a, b) => a + b;

tests/testdata/npm/registry/@denotest/cjs-internal-types-default-export/1.0.0/package.json

@@ -0,0 +1,4 @@
+{
+  "name": "@denotest/cjs-internal-types-default-export",
+  "version": "1.0.0"
+}