From 36cce3f287ac3e6358b653bb5850ce3813bdf46b Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 24 Jul 2023 15:35:13 -0400 Subject: [PATCH] refactor(ext/node): CjsCodeAnalyzer - analyze_cjs optionally pass source text (#19896) --- cli/factory.rs | 3 ++- cli/node.rs | 18 ++++++++++++---- cli/standalone/mod.rs | 3 ++- ext/node/analyze.rs | 48 ++++++++++++++++++++++--------------------- 4 files changed, 43 insertions(+), 29 deletions(-) diff --git a/cli/factory.rs b/cli/factory.rs index cb835181c1..330865744f 100644 --- a/cli/factory.rs +++ b/cli/factory.rs @@ -475,7 +475,8 @@ impl CliFactory { let caches = self.caches()?; let node_analysis_cache = NodeAnalysisCache::new(caches.node_analysis_db()); - let cjs_esm_analyzer = CliCjsCodeAnalyzer::new(node_analysis_cache); + let cjs_esm_analyzer = + CliCjsCodeAnalyzer::new(node_analysis_cache, self.fs().clone()); Ok(Arc::new(NodeCodeTranslator::new( cjs_esm_analyzer, diff --git a/cli/node.rs b/cli/node.rs index 75e0d9ef91..2a9a84ef95 100644 --- a/cli/node.rs +++ b/cli/node.rs @@ -1,9 +1,12 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. +use std::borrow::Cow; + use deno_ast::CjsAnalysis; use deno_ast::MediaType; use deno_ast::ModuleSpecifier; use deno_core::error::AnyError; +use deno_runtime::deno_fs; use deno_runtime::deno_node::analyze::CjsAnalysis as ExtNodeCjsAnalysis; use deno_runtime::deno_node::analyze::CjsCodeAnalyzer; use deno_runtime::deno_node::analyze::NodeCodeTranslator; @@ -34,11 +37,12 @@ pub fn resolve_specifier_into_node_modules( pub struct CliCjsCodeAnalyzer { cache: NodeAnalysisCache, + fs: deno_fs::FileSystemRc, } impl CliCjsCodeAnalyzer { - pub fn new(cache: NodeAnalysisCache) -> Self { - Self { cache } + pub fn new(cache: NodeAnalysisCache, fs: deno_fs::FileSystemRc) -> Self { + Self { cache, fs } } fn inner_cjs_analysis( @@ -83,9 +87,15 @@ impl CjsCodeAnalyzer for CliCjsCodeAnalyzer { fn analyze_cjs( &self, specifier: &ModuleSpecifier, - source: &str, + source: Option<&str>, ) -> Result { - let analysis = self.inner_cjs_analysis(specifier, source)?; + let source = match source { + Some(source) => Cow::Borrowed(source), + None => { + Cow::Owned(self.fs.read_to_string(&specifier.to_file_path().unwrap())?) + } + }; + let analysis = self.inner_cjs_analysis(specifier, &source)?; Ok(ExtNodeCjsAnalysis { exports: analysis.exports, reexports: analysis.reexports, diff --git a/cli/standalone/mod.rs b/cli/standalone/mod.rs index d08df4b12a..dfa71cf6f7 100644 --- a/cli/standalone/mod.rs +++ b/cli/standalone/mod.rs @@ -366,7 +366,8 @@ pub async fn run( let cjs_resolutions = Arc::new(CjsResolutionStore::default()); let cache_db = Caches::new(deno_dir_provider.clone()); let node_analysis_cache = NodeAnalysisCache::new(cache_db.node_analysis_db()); - let cjs_esm_code_analyzer = CliCjsCodeAnalyzer::new(node_analysis_cache); + let cjs_esm_code_analyzer = + CliCjsCodeAnalyzer::new(node_analysis_cache, fs.clone()); let node_code_translator = Arc::new(NodeCodeTranslator::new( cjs_esm_code_analyzer, fs.clone(), diff --git a/ext/node/analyze.rs b/ext/node/analyze.rs index d811122d5c..cf292d82fa 100644 --- a/ext/node/analyze.rs +++ b/ext/node/analyze.rs @@ -29,10 +29,15 @@ pub struct CjsAnalysis { pub trait CjsCodeAnalyzer { /// Analyzes CommonJs code for exports and reexports, which is /// then used to determine the wrapper ESM module exports. + /// + /// Note that the source is provided by the caller when the caller + /// already has it. If the source is needed by the implementation, + /// then it can use the provided source, or otherwise load it if + /// necessary. fn analyze_cjs( &self, specifier: &ModuleSpecifier, - source: &str, + maybe_source: Option<&str>, ) -> Result; } @@ -73,7 +78,9 @@ impl NodeCodeTranslator { let mut temp_var_count = 0; let mut handled_reexports: HashSet = HashSet::default(); - let analysis = self.cjs_code_analyzer.analyze_cjs(specifier, source)?; + let analysis = self + .cjs_code_analyzer + .analyze_cjs(specifier, Some(source))?; let mut source = vec![ r#"import {createRequire as __internalCreateRequire} from "node:module"; @@ -100,7 +107,7 @@ impl NodeCodeTranslator { handled_reexports.insert(reexport.to_string()); - // First, resolve relate reexport specifier + // First, resolve the reexport specifier let resolved_reexport = self.resolve( &reexport, &referrer, @@ -110,35 +117,30 @@ impl NodeCodeTranslator { NodeResolutionMode::Execution, permissions, )?; - // Second, read the source code from disk + + // Second, resolve its exports and re-exports let reexport_specifier = ModuleSpecifier::from_file_path(&resolved_reexport).unwrap(); - let reexport_file_text = self - .fs - .read_to_string(&resolved_reexport) - .map_err(AnyError::from) + let analysis = self + .cjs_code_analyzer + .analyze_cjs(&reexport_specifier, None) .with_context(|| { format!( - "Could not find '{}' ({}) referenced from {}", + "Could not load '{}' ({}) referenced from {}", reexport, reexport_specifier, referrer ) })?; - { - let analysis = self - .cjs_code_analyzer - .analyze_cjs(&reexport_specifier, &reexport_file_text)?; - for reexport in analysis.reexports { - reexports_to_handle.push_back((reexport, reexport_specifier.clone())); - } - - all_exports.extend( - analysis - .exports - .into_iter() - .filter(|e| e.as_str() != "default"), - ); + for reexport in analysis.reexports { + reexports_to_handle.push_back((reexport, reexport_specifier.clone())); } + + all_exports.extend( + analysis + .exports + .into_iter() + .filter(|e| e.as_str() != "default"), + ); } source.push(format!(