fix: better handling of npm resolution occurring on workers (#24094)

Closes https://github.com/denoland/deno/issues/24063

8 files changed, 92 insertions, 77 deletions

diff --git a/cli/lsp/analysis.rs b/cli/lsp/analysis.rs
index f60f2e044..62feeb602 100644
--- a/cli/lsp/analysis.rs
+++ b/cli/lsp/analysis.rs
@@ -724,8 +724,8 @@ impl CodeActionCollection {
     &mut self,
     specifier: &ModuleSpecifier,
     diagnostic: &lsp::Diagnostic,
-    maybe_text_info: Option<SourceTextInfo>,
-    maybe_parsed_source: Option<deno_ast::ParsedSource>,
+    maybe_text_info: Option<&SourceTextInfo>,
+    maybe_parsed_source: Option<&deno_ast::ParsedSource>,
   ) -> Result<(), AnyError> {
     if let Some(data_quick_fixes) = diagnostic
       .data
@@ -774,8 +774,8 @@ impl CodeActionCollection {
     &mut self,
     specifier: &ModuleSpecifier,
     diagnostic: &lsp::Diagnostic,
-    maybe_text_info: Option<SourceTextInfo>,
-    maybe_parsed_source: Option<deno_ast::ParsedSource>,
+    maybe_text_info: Option<&SourceTextInfo>,
+    maybe_parsed_source: Option<&deno_ast::ParsedSource>,
   ) -> Result<(), AnyError> {
     let code = diagnostic
       .code
@@ -830,7 +830,7 @@ impl CodeActionCollection {
       .push(CodeActionKind::DenoLint(ignore_error_action));
 
     // Disable a lint error for the entire file.
-    let maybe_ignore_comment = maybe_parsed_source.clone().and_then(|ps| {
+    let maybe_ignore_comment = maybe_parsed_source.and_then(|ps| {
       // Note: we can use ps.get_leading_comments() but it doesn't
       // work when shebang is present at the top of the file.
       ps.comments().get_vec().iter().find_map(|c| {
@@ -861,9 +861,8 @@ impl CodeActionCollection {
     if let Some(ignore_comment) = maybe_ignore_comment {
       new_text = format!(" {code}");
       // Get the end position of the comment.
-      let line = maybe_parsed_source
+      let line = maybe_text_info
         .unwrap()
-        .text_info()
         .line_and_column_index(ignore_comment.end());
       let position = lsp::Position {
         line: line.line_index as u32,
diff --git a/cli/lsp/code_lens.rs b/cli/lsp/code_lens.rs
index d7e9e7052..c7e0c59bb 100644
--- a/cli/lsp/code_lens.rs
+++ b/cli/lsp/code_lens.rs
@@ -67,7 +67,7 @@ impl DenoTestCollector {
 
   fn add_code_lenses<N: AsRef<str>>(&mut self, name: N, range: &SourceRange) {
     let range =
-      source_range_to_lsp_range(range, self.parsed_source.text_info());
+      source_range_to_lsp_range(range, self.parsed_source.text_info_lazy());
     self.add_code_lens(&name, range, "▶\u{fe0e} Run Test", false);
     self.add_code_lens(&name, range, "Debug", true);
   }
@@ -406,7 +406,7 @@ pub async fn resolve_code_lens(
 
 pub fn collect_test(
   specifier: &ModuleSpecifier,
-  parsed_source: ParsedSource,
+  parsed_source: &ParsedSource,
 ) -> Result<Vec<lsp::CodeLens>, AnyError> {
   let mut collector =
     DenoTestCollector::new(specifier.clone(), parsed_source.clone());
@@ -537,7 +537,6 @@ pub fn collect_tsc(
 #[cfg(test)]
 mod tests {
   use deno_ast::MediaType;
-  use deno_ast::SourceTextInfo;
 
   use super::*;
 
@@ -562,7 +561,7 @@ mod tests {
     "#;
     let parsed_module = deno_ast::parse_module(deno_ast::ParseParams {
       specifier: specifier.clone(),
-      text_info: SourceTextInfo::new(source.into()),
+      text: source.into(),
       media_type: MediaType::TypeScript,
       capture_tokens: true,
       scope_analysis: true,
diff --git a/cli/lsp/completions.rs b/cli/lsp/completions.rs
index 4e0e4b46b..f02ba6409 100644
--- a/cli/lsp/completions.rs
+++ b/cli/lsp/completions.rs
@@ -164,7 +164,7 @@ pub async fn get_import_completions(
   let document = documents.get(specifier)?;
   let file_referrer = document.file_referrer();
   let (text, _, range) = document.get_maybe_dependency(position)?;
-  let range = to_narrow_lsp_range(&document.text_info(), &range);
+  let range = to_narrow_lsp_range(document.text_info(), &range);
   let resolved = resolver
     .as_graph_resolver(file_referrer)
     .resolve(
diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs
index 9e54c8106..6504c38fe 100644
--- a/cli/lsp/diagnostics.rs
+++ b/cli/lsp/diagnostics.rs
@@ -854,7 +854,7 @@ fn generate_document_lint_diagnostics(
   match document.maybe_parsed_source() {
     Some(Ok(parsed_source)) => {
       if let Ok(references) =
-        analysis::get_lint_references(&parsed_source, lint_rules, lint_config)
+        analysis::get_lint_references(parsed_source, lint_rules, lint_config)
       {
         references
           .into_iter()
diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs
index 7cf839a69..c7323d0c8 100644
--- a/cli/lsp/documents.rs
+++ b/cli/lsp/documents.rs
@@ -155,7 +155,7 @@ impl AssetOrDocument {
   pub fn text(&self) -> Arc<str> {
     match self {
       AssetOrDocument::Asset(a) => a.text(),
-      AssetOrDocument::Document(d) => d.text_info.text(),
+      AssetOrDocument::Document(d) => d.text.clone(),
     }
   }
 
@@ -191,7 +191,7 @@ impl AssetOrDocument {
 
   pub fn maybe_parsed_source(
     &self,
-  ) -> Option<Result<deno_ast::ParsedSource, deno_ast::ParseDiagnostic>> {
+  ) -> Option<&Result<deno_ast::ParsedSource, deno_ast::ParseDiagnostic>> {
     self.document().and_then(|d| d.maybe_parsed_source())
   }
 
@@ -243,7 +243,7 @@ fn get_maybe_test_module_fut(
   let handle = tokio::task::spawn_blocking(move || {
     let mut collector = TestCollector::new(
       parsed_source.specifier().clone(),
-      parsed_source.text_info().clone(),
+      parsed_source.text_info_lazy().clone(),
     );
     parsed_source.module().visit_with(&mut collector);
     Arc::new(collector.take())
@@ -283,7 +283,8 @@ pub struct Document {
   open_data: Option<DocumentOpenData>,
   resolver: Arc<LspResolver>,
   specifier: ModuleSpecifier,
-  text_info: SourceTextInfo,
+  text: Arc<str>,
+  text_info_cell: once_cell::sync::OnceCell<SourceTextInfo>,
 }
 
 impl Document {
@@ -291,7 +292,7 @@ impl Document {
   #[allow(clippy::too_many_arguments)]
   fn new(
     specifier: ModuleSpecifier,
-    content: Arc<str>,
+    text: Arc<str>,
     maybe_lsp_version: Option<i32>,
     maybe_language_id: Option<LanguageId>,
     maybe_headers: Option<HashMap<String, String>>,
@@ -300,7 +301,6 @@ impl Document {
     cache: &Arc<LspCache>,
     file_referrer: Option<ModuleSpecifier>,
   ) -> Arc<Self> {
-    let text_info = SourceTextInfo::new(content);
     let media_type = resolve_media_type(
       &specifier,
       maybe_headers.as_ref(),
@@ -311,7 +311,7 @@ impl Document {
       if media_type_is_diagnosable(media_type) {
         parse_and_analyze_module(
           specifier.clone(),
-          text_info.clone(),
+          text.clone(),
           maybe_headers.as_ref(),
           media_type,
           file_referrer.as_ref(),
@@ -328,7 +328,7 @@ impl Document {
     let maybe_types_dependency = maybe_module
       .as_ref()
       .and_then(|m| Some(Arc::new(m.maybe_types_dependency.clone()?)));
-    let line_index = Arc::new(LineIndex::new(text_info.text_str()));
+    let line_index = Arc::new(LineIndex::new(text.as_ref()));
     let maybe_test_module_fut =
       get_maybe_test_module_fut(maybe_parsed_source.as_ref(), &config);
     Arc::new(Self {
@@ -350,7 +350,8 @@ impl Document {
       }),
       resolver,
       specifier,
-      text_info,
+      text,
+      text_info_cell: once_cell::sync::OnceCell::new(),
     })
   }
 
@@ -370,11 +371,8 @@ impl Document {
     let maybe_parsed_source;
     let maybe_test_module_fut;
     if media_type != self.media_type {
-      let parsed_source_result = parse_source(
-        self.specifier.clone(),
-        self.text_info.clone(),
-        media_type,
-      );
+      let parsed_source_result =
+        parse_source(self.specifier.clone(), self.text.clone(), media_type);
       let maybe_module = analyze_module(
         self.specifier.clone(),
         &parsed_source_result,
@@ -397,7 +395,7 @@ impl Document {
       let graph_resolver =
         resolver.as_graph_resolver(self.file_referrer.as_ref());
       let npm_resolver =
-        resolver.as_graph_npm_resolver(self.file_referrer.as_ref());
+        resolver.create_graph_npm_resolver(self.file_referrer.as_ref());
       dependencies = Arc::new(
         self
           .dependencies
@@ -409,7 +407,7 @@ impl Document {
                 s,
                 &CliJsrUrlProvider,
                 Some(graph_resolver),
-                Some(npm_resolver),
+                Some(&npm_resolver),
               ),
             )
           })
@@ -419,10 +417,10 @@ impl Document {
         Arc::new(d.with_new_resolver(
           &CliJsrUrlProvider,
           Some(graph_resolver),
-          Some(npm_resolver),
+          Some(&npm_resolver),
         ))
       });
-      maybe_parsed_source = self.maybe_parsed_source();
+      maybe_parsed_source = self.maybe_parsed_source().cloned();
       maybe_test_module_fut = self
         .maybe_test_module_fut
         .clone()
@@ -450,7 +448,8 @@ impl Document {
       }),
       resolver,
       specifier: self.specifier.clone(),
-      text_info: self.text_info.clone(),
+      text: self.text.clone(),
+      text_info_cell: once_cell::sync::OnceCell::new(),
     })
   }
 
@@ -459,7 +458,7 @@ impl Document {
     version: i32,
     changes: Vec<lsp::TextDocumentContentChangeEvent>,
   ) -> Result<Arc<Self>, AnyError> {
-    let mut content = self.text_info.text_str().to_string();
+    let mut content = self.text.to_string();
     let mut line_index = self.line_index.clone();
     let mut index_valid = IndexValid::All;
     for change in changes {
@@ -475,7 +474,7 @@ impl Document {
         index_valid = IndexValid::UpTo(0);
       }
     }
-    let text_info = SourceTextInfo::from_string(content);
+    let text: Arc<str> = content.into();
     let media_type = self.media_type;
     let (maybe_parsed_source, maybe_module) = if self
       .maybe_language_id
@@ -485,7 +484,7 @@ impl Document {
     {
       parse_and_analyze_module(
         self.specifier.clone(),
-        text_info.clone(),
+        text.clone(),
         self.maybe_headers.as_ref(),
         media_type,
         self.file_referrer.as_ref(),
@@ -506,7 +505,7 @@ impl Document {
     let line_index = if index_valid == IndexValid::All {
       line_index
     } else {
-      Arc::new(LineIndex::new(text_info.text_str()))
+      Arc::new(LineIndex::new(text.as_ref()))
     };
     let maybe_test_module_fut =
       get_maybe_test_module_fut(maybe_parsed_source.as_ref(), &self.config);
@@ -518,7 +517,8 @@ impl Document {
       maybe_language_id: self.maybe_language_id,
       dependencies,
       maybe_types_dependency,
-      text_info,
+      text,
+      text_info_cell: once_cell::sync::OnceCell::new(),
       line_index,
       maybe_headers: self.maybe_headers.clone(),
       maybe_navigation_tree: Mutex::new(None),
@@ -542,7 +542,8 @@ impl Document {
       maybe_language_id: self.maybe_language_id,
       dependencies: self.dependencies.clone(),
       maybe_types_dependency: self.maybe_types_dependency.clone(),
-      text_info: self.text_info.clone(),
+      text: self.text.clone(),
+      text_info_cell: once_cell::sync::OnceCell::new(),
       line_index: self.line_index.clone(),
       maybe_headers: self.maybe_headers.clone(),
       maybe_navigation_tree: Mutex::new(
@@ -564,7 +565,8 @@ impl Document {
       maybe_language_id: self.maybe_language_id,
       dependencies: self.dependencies.clone(),
       maybe_types_dependency: self.maybe_types_dependency.clone(),
-      text_info: self.text_info.clone(),
+      text: self.text.clone(),
+      text_info_cell: once_cell::sync::OnceCell::new(),
       line_index: self.line_index.clone(),
       maybe_headers: self.maybe_headers.clone(),
       maybe_navigation_tree: Mutex::new(
@@ -585,12 +587,22 @@ impl Document {
     self.file_referrer.as_ref()
   }
 
-  pub fn content(&self) -> Arc<str> {
-    self.text_info.text()
+  pub fn content(&self) -> &Arc<str> {
+    &self.text
   }
 
-  pub fn text_info(&self) -> SourceTextInfo {
-    self.text_info.clone()
+  pub fn text_info(&self) -> &SourceTextInfo {
+    // try to get the text info from the parsed source and if
+    // not then create one in the cell
+    self
+      .maybe_parsed_source()
+      .and_then(|p| p.as_ref().ok())
+      .map(|p| p.text_info_lazy())
+      .unwrap_or_else(|| {
+        self
+          .text_info_cell
+          .get_or_init(|| SourceTextInfo::new(self.text.clone()))
+      })
   }
 
   pub fn line_index(&self) -> Arc<LineIndex> {
@@ -647,8 +659,8 @@ impl Document {
 
   pub fn maybe_parsed_source(
     &self,
-  ) -> Option<Result<deno_ast::ParsedSource, deno_ast::ParseDiagnostic>> {
-    self.open_data.as_ref()?.maybe_parsed_source.clone()
+  ) -> Option<&Result<deno_ast::ParsedSource, deno_ast::ParseDiagnostic>> {
+    self.open_data.as_ref()?.maybe_parsed_source.as_ref()
   }
 
   pub async fn maybe_test_module(&self) -> Option<Arc<TestModule>> {
@@ -1421,7 +1433,7 @@ impl<'a> OpenDocumentsGraphLoader<'a> {
       if let Some(doc) = self.open_docs.get(specifier) {
         return Some(
           future::ready(Ok(Some(deno_graph::source::LoadResponse::Module {
-            content: Arc::from(doc.content()),
+            content: Arc::from(doc.content().clone()),
             specifier: doc.specifier().clone(),
             maybe_headers: None,
           })))
@@ -1459,14 +1471,13 @@ impl<'a> deno_graph::source::Loader for OpenDocumentsGraphLoader<'a> {
 
 fn parse_and_analyze_module(
   specifier: ModuleSpecifier,
-  text_info: SourceTextInfo,
+  text: Arc<str>,
   maybe_headers: Option<&HashMap<String, String>>,
   media_type: MediaType,
   file_referrer: Option<&ModuleSpecifier>,
   resolver: &LspResolver,
 ) -> (Option<ParsedSourceResult>, Option<ModuleResult>) {
-  let parsed_source_result =
-    parse_source(specifier.clone(), text_info, media_type);
+  let parsed_source_result = parse_source(specifier.clone(), text, media_type);
   let module_result = analyze_module(
     specifier,
     &parsed_source_result,
@@ -1479,12 +1490,12 @@ fn parse_and_analyze_module(
 
 fn parse_source(
   specifier: ModuleSpecifier,
-  text_info: SourceTextInfo,
+  text: Arc<str>,
   media_type: MediaType,
 ) -> ParsedSourceResult {
   deno_ast::parse_module(deno_ast::ParseParams {
     specifier,
-    text_info,
+    text,
     media_type,
     capture_tokens: true,
     scope_analysis: true,
@@ -1500,20 +1511,23 @@ fn analyze_module(
   resolver: &LspResolver,
 ) -> ModuleResult {
   match parsed_source_result {
-    Ok(parsed_source) => Ok(deno_graph::parse_module_from_ast(
-      deno_graph::ParseModuleFromAstOptions {
-        graph_kind: deno_graph::GraphKind::TypesOnly,
-        specifier,
-        maybe_headers,
-        parsed_source,
-        // use a null file system because there's no need to bother resolving
-        // dynamic imports like import(`./dir/${something}`) in the LSP
-        file_system: &deno_graph::source::NullFileSystem,
-        jsr_url_provider: &CliJsrUrlProvider,
-        maybe_resolver: Some(resolver.as_graph_resolver(file_referrer)),
-        maybe_npm_resolver: Some(resolver.as_graph_npm_resolver(file_referrer)),
-      },
-    )),
+    Ok(parsed_source) => {
+      let npm_resolver = resolver.create_graph_npm_resolver(file_referrer);
+      Ok(deno_graph::parse_module_from_ast(
+        deno_graph::ParseModuleFromAstOptions {
+          graph_kind: deno_graph::GraphKind::TypesOnly,
+          specifier,
+          maybe_headers,
+          parsed_source,
+          // use a null file system because there's no need to bother resolving
+          // dynamic imports like import(`./dir/${something}`) in the LSP
+          file_system: &deno_graph::source::NullFileSystem,
+          jsr_url_provider: &CliJsrUrlProvider,
+          maybe_resolver: Some(resolver.as_graph_resolver(file_referrer)),
+          maybe_npm_resolver: Some(&npm_resolver),
+        },
+      ))
+    }
     Err(err) => Err(deno_graph::ModuleGraphError::ModuleError(
       deno_graph::ModuleError::ParseErr(specifier, err.clone()),
     )),
@@ -1602,7 +1616,7 @@ console.log(b);
       )
       .unwrap();
     assert_eq!(
-      &*documents.get(&specifier).unwrap().content(),
+      documents.get(&specifier).unwrap().content().as_ref(),
       r#"import * as b from "./b.ts";
 console.log(b, "hello deno");
 "#
diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs
index e362a9e7e..466c5b430 100644
--- a/cli/lsp/language_server.rs
+++ b/cli/lsp/language_server.rs
@@ -1317,7 +1317,7 @@ impl Inner {
       move || {
         let format_result = match document.maybe_parsed_source() {
           Some(Ok(parsed_source)) => {
-            format_parsed_source(&parsed_source, &fmt_options)
+            format_parsed_source(parsed_source, &fmt_options)
           }
           Some(Err(err)) => Err(anyhow!("{:#}", err)),
           None => {
@@ -1330,12 +1330,12 @@ impl Inner {
               .map(|ext| file_path.with_extension(ext))
               .unwrap_or(file_path);
             // it's not a js/ts file, so attempt to format its contents
-            format_file(&file_path, &document.content(), &fmt_options)
+            format_file(&file_path, document.content(), &fmt_options)
           }
         };
         match format_result {
           Ok(Some(new_text)) => Some(text::get_edits(
-            &document.content(),
+            document.content(),
             &new_text,
             document.line_index().as_ref(),
           )),
@@ -1605,7 +1605,9 @@ impl Inner {
               &specifier,
               diagnostic,
               asset_or_doc.document().map(|d| d.text_info()),
-              asset_or_doc.maybe_parsed_source().and_then(|r| r.ok()),
+              asset_or_doc
+                .maybe_parsed_source()
+                .and_then(|r| r.as_ref().ok()),
             )
             .map_err(|err| {
               error!("Unable to fix lint error: {:#}", err);
diff --git a/cli/lsp/resolver.rs b/cli/lsp/resolver.rs
index 599db4876..2b465f695 100644
--- a/cli/lsp/resolver.rs
+++ b/cli/lsp/resolver.rs
@@ -20,6 +20,7 @@ use crate::resolver::CliGraphResolver;
 use crate::resolver::CliGraphResolverOptions;
 use crate::resolver::CliNodeResolver;
 use crate::resolver::SloppyImportsResolver;
+use crate::resolver::WorkerCliNpmGraphResolver;
 use crate::util::progress_bar::ProgressBar;
 use crate::util::progress_bar::ProgressBarStyle;
 use dashmap::DashMap;
@@ -27,7 +28,6 @@ use deno_ast::MediaType;
 use deno_cache_dir::HttpCache;
 use deno_core::error::AnyError;
 use deno_core::url::Url;
-use deno_graph::source::NpmResolver;
 use deno_graph::source::Resolver;
 use deno_graph::GraphImport;
 use deno_graph::ModuleSpecifier;
@@ -105,6 +105,7 @@ impl LspResolver {
     let redirect_resolver = Some(Arc::new(RedirectResolver::new(
       cache.root_vendor_or_global(),
     )));
+    let npm_graph_resolver = graph_resolver.create_graph_npm_resolver();
     let graph_imports = config_data
       .and_then(|d| d.config_file.as_ref())
       .and_then(|cf| cf.to_maybe_imports().ok())
@@ -118,7 +119,7 @@ impl LspResolver {
                 imports,
                 &CliJsrUrlProvider,
                 Some(graph_resolver.as_ref()),
-                Some(graph_resolver.as_ref()),
+                Some(&npm_graph_resolver),
               );
               (referrer, graph_import)
             })
@@ -180,11 +181,11 @@ impl LspResolver {
     self.graph_resolver.as_ref()
   }
 
-  pub fn as_graph_npm_resolver(
+  pub fn create_graph_npm_resolver(
     &self,
     _file_referrer: Option<&ModuleSpecifier>,
-  ) -> &dyn NpmResolver {
-    self.graph_resolver.as_ref()
+  ) -> WorkerCliNpmGraphResolver {
+    self.graph_resolver.create_graph_npm_resolver()
   }
 
   pub fn maybe_managed_npm_resolver(
diff --git a/cli/lsp/testing/collectors.rs b/cli/lsp/testing/collectors.rs
index 508e50f9b..7b00228df 100644
--- a/cli/lsp/testing/collectors.rs
+++ b/cli/lsp/testing/collectors.rs
@@ -641,14 +641,14 @@ pub mod tests {
 
     let parsed_module = deno_ast::parse_module(deno_ast::ParseParams {
       specifier: specifier.clone(),
-      text_info: deno_ast::SourceTextInfo::new(source.into()),
+      text: source.into(),
       media_type: deno_ast::MediaType::TypeScript,
       capture_tokens: true,
       scope_analysis: true,
       maybe_syntax: None,
     })
     .unwrap();
-    let text_info = parsed_module.text_info().clone();
+    let text_info = parsed_module.text_info_lazy().clone();
     let mut collector = TestCollector::new(specifier, text_info);
     parsed_module.module().visit_with(&mut collector);
     collector.take()