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

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

1 files changed, 67 insertions, 53 deletions

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");
 "#