diff --git a/cli/args/mod.rs b/cli/args/mod.rs index 434a4fa6af4cf..0a0f7d7043dda 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -12,6 +12,7 @@ use self::package_json::PackageJsonDeps; use ::import_map::ImportMap; use deno_ast::SourceMapOption; use deno_core::resolve_url_or_path; +use deno_graph::GraphKind; use deno_npm::resolution::ValidSerializedNpmResolutionSnapshot; use deno_npm::NpmSystemInfo; use deno_runtime::deno_tls::RootCertStoreProvider; @@ -873,6 +874,14 @@ impl CliOptions { self.maybe_config_file.as_ref().map(|f| f.specifier.clone()) } + pub fn graph_kind(&self) -> GraphKind { + match self.sub_command() { + DenoSubcommand::Cache(_) => GraphKind::All, + DenoSubcommand::Check(_) => GraphKind::TypesOnly, + _ => self.type_check_mode().as_graph_kind(), + } + } + pub fn ts_type_lib_window(&self) -> TsTypeLib { TsTypeLib::DenoWindow } diff --git a/cli/factory.rs b/cli/factory.rs index 074eaa1e4ceba..8a9d20970a1c0 100644 --- a/cli/factory.rs +++ b/cli/factory.rs @@ -21,9 +21,9 @@ use crate::cache::NodeAnalysisCache; use crate::cache::ParsedSourceCache; use crate::emit::Emitter; use crate::file_fetcher::FileFetcher; +use crate::graph_container::MainModuleGraphContainer; use crate::graph_util::FileWatcherReporter; use crate::graph_util::ModuleGraphBuilder; -use crate::graph_util::ModuleGraphContainer; use crate::graph_util::ModuleGraphCreator; use crate::http_util::HttpClient; use crate::module_loader::CliModuleLoaderFactory; @@ -60,7 +60,6 @@ use deno_core::futures::FutureExt; use deno_core::parking_lot::Mutex; use deno_core::FeatureChecker; -use deno_graph::GraphKind; use deno_lockfile::WorkspaceMemberConfig; use deno_runtime::deno_fs; use deno_runtime::deno_node::analyze::NodeCodeTranslator; @@ -157,7 +156,7 @@ struct CliFactoryServices { emit_cache: Deferred, emitter: Deferred>, fs: Deferred>, - graph_container: Deferred>, + main_graph_container: Deferred>, lockfile: Deferred>>>, maybe_import_map: Deferred>>, maybe_inspector_server: Deferred>>, @@ -673,17 +672,19 @@ impl CliFactory { .await } - pub fn graph_container(&self) -> &Arc { - self.services.graph_container.get_or_init(|| { - let graph_kind = match self.options.sub_command() { - // todo(dsherret): ideally the graph container would not be used - // for deno cache because it doesn't dynamically load modules - DenoSubcommand::Cache(_) => GraphKind::All, - DenoSubcommand::Check(_) => GraphKind::TypesOnly, - _ => self.options.type_check_mode().as_graph_kind(), - }; - Arc::new(ModuleGraphContainer::new(graph_kind)) - }) + pub async fn main_module_graph_container( + &self, + ) -> Result<&Arc, AnyError> { + self + .services + .main_graph_container + .get_or_try_init_async(async { + Ok(Arc::new(MainModuleGraphContainer::new( + self.cli_options().clone(), + self.module_load_preparer().await?.clone(), + ))) + }) + .await } pub fn maybe_inspector_server( @@ -706,7 +707,6 @@ impl CliFactory { .get_or_try_init_async(async { Ok(Arc::new(ModuleLoadPreparer::new( self.options.clone(), - self.graph_container().clone(), self.maybe_lockfile().clone(), self.module_graph_builder().await?.clone(), self.text_only_progress_bar().clone(), @@ -791,11 +791,15 @@ impl CliFactory { self.blob_store().clone(), Box::new(CliModuleLoaderFactory::new( &self.options, + if self.options.code_cache_enabled() { + Some(self.code_cache()?.clone()) + } else { + None + }, self.emitter()?.clone(), - self.graph_container().clone(), + self.main_module_graph_container().await?.clone(), + self.module_info_cache()?.clone(), self.module_load_preparer().await?.clone(), - self.parsed_source_cache().clone(), - self.resolver().await?.clone(), cli_node_resolver.clone(), NpmModuleLoader::new( self.cjs_resolutions().clone(), @@ -803,12 +807,8 @@ impl CliFactory { fs.clone(), cli_node_resolver.clone(), ), - if self.options.code_cache_enabled() { - Some(self.code_cache()?.clone()) - } else { - None - }, - self.module_info_cache()?.clone(), + self.parsed_source_cache().clone(), + self.resolver().await?.clone(), )), self.root_cert_store_provider().clone(), self.fs().clone(), diff --git a/cli/graph_container.rs b/cli/graph_container.rs new file mode 100644 index 0000000000000..ec18ffaab45d4 --- /dev/null +++ b/cli/graph_container.rs @@ -0,0 +1,157 @@ +// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. + +use std::sync::Arc; + +use deno_ast::ModuleSpecifier; +use deno_core::error::AnyError; +use deno_core::parking_lot::RwLock; +use deno_core::resolve_url_or_path; +use deno_graph::ModuleGraph; +use deno_runtime::colors; +use deno_runtime::permissions::PermissionsContainer; + +use crate::args::CliOptions; +use crate::module_loader::ModuleLoadPreparer; + +pub trait ModuleGraphContainer: Clone + 'static { + /// Acquires a permit to modify the module graph without other code + /// having the chance to modify it. In the meantime, other code may + /// still read from the existing module graph. + async fn acquire_update_permit(&self) -> impl ModuleGraphUpdatePermit; + /// Gets a copy of the graph. + fn graph(&self) -> Arc; +} + +/// A permit for updating the module graph. When complete and +/// everything looks fine, calling `.commit()` will store the +/// new graph in the ModuleGraphContainer. +pub trait ModuleGraphUpdatePermit { + /// Gets the module graph for mutation. + fn graph_mut(&mut self) -> &mut ModuleGraph; + /// Saves the mutated module graph in the container. + fn commit(self); +} + +/// Holds the `ModuleGraph` for the main worker. +#[derive(Clone)] +pub struct MainModuleGraphContainer { + // Allow only one request to update the graph data at a time, + // but allow other requests to read from it at any time even + // while another request is updating the data. + update_queue: Arc, + inner: Arc>>, + cli_options: Arc, + module_load_preparer: Arc, +} + +impl MainModuleGraphContainer { + pub fn new( + cli_options: Arc, + module_load_preparer: Arc, + ) -> Self { + Self { + update_queue: Default::default(), + inner: Arc::new(RwLock::new(Arc::new(ModuleGraph::new( + cli_options.graph_kind(), + )))), + cli_options, + module_load_preparer, + } + } + + pub async fn check_specifiers( + &self, + specifiers: &[ModuleSpecifier], + ) -> Result<(), AnyError> { + let mut graph_permit = self.acquire_update_permit().await; + let graph = graph_permit.graph_mut(); + self + .module_load_preparer + .prepare_module_load( + graph, + specifiers, + false, + self.cli_options.ts_type_lib_window(), + PermissionsContainer::allow_all(), + ) + .await?; + graph_permit.commit(); + Ok(()) + } + + /// Helper around prepare_module_load that loads and type checks + /// the provided files. + pub async fn load_and_type_check_files( + &self, + files: &[String], + ) -> Result<(), AnyError> { + let specifiers = self.collect_specifiers(files)?; + + if specifiers.is_empty() { + log::warn!("{} No matching files found.", colors::yellow("Warning")); + } + + self.check_specifiers(&specifiers).await + } + + pub fn collect_specifiers( + &self, + files: &[String], + ) -> Result, AnyError> { + let excludes = self.cli_options.resolve_config_excludes()?; + Ok( + files + .iter() + .filter_map(|file| { + let file_url = + resolve_url_or_path(file, self.cli_options.initial_cwd()).ok()?; + if file_url.scheme() != "file" { + return Some(file_url); + } + // ignore local files that match any of files listed in `exclude` option + let file_path = file_url.to_file_path().ok()?; + if excludes.matches_path(&file_path) { + None + } else { + Some(file_url) + } + }) + .collect::>(), + ) + } +} + +impl ModuleGraphContainer for MainModuleGraphContainer { + async fn acquire_update_permit(&self) -> impl ModuleGraphUpdatePermit { + let permit = self.update_queue.acquire().await; + MainModuleGraphUpdatePermit { + permit, + inner: self.inner.clone(), + graph: (**self.inner.read()).clone(), + } + } + + fn graph(&self) -> Arc { + self.inner.read().clone() + } +} + +/// A permit for updating the module graph. When complete and +/// everything looks fine, calling `.commit()` will store the +/// new graph in the ModuleGraphContainer. +pub struct MainModuleGraphUpdatePermit<'a> { + permit: crate::util::sync::TaskQueuePermit<'a>, + inner: Arc>>, + graph: ModuleGraph, +} + +impl<'a> ModuleGraphUpdatePermit for MainModuleGraphUpdatePermit<'a> { + fn graph_mut(&mut self) -> &mut ModuleGraph { + &mut self.graph + } + + fn commit(self) { + *self.inner.write() = Arc::new(self.graph); + drop(self.permit); // explicit drop for clarity + } +} diff --git a/cli/graph_util.rs b/cli/graph_util.rs index 375096f988a49..ed56cf9f7a120 100644 --- a/cli/graph_util.rs +++ b/cli/graph_util.rs @@ -18,8 +18,6 @@ use crate::tools::check; use crate::tools::check::TypeChecker; use crate::util::file_watcher::WatcherCommunicator; use crate::util::fs::canonicalize_path; -use crate::util::sync::TaskQueue; -use crate::util::sync::TaskQueuePermit; use deno_runtime::fs_util::specifier_to_file_path; use deno_config::WorkspaceMemberConfig; @@ -27,7 +25,6 @@ use deno_core::anyhow::bail; use deno_core::error::custom_error; use deno_core::error::AnyError; use deno_core::parking_lot::Mutex; -use deno_core::parking_lot::RwLock; use deno_core::ModuleSpecifier; use deno_graph::source::Loader; use deno_graph::source::ResolutionMode; @@ -762,40 +759,6 @@ fn get_resolution_error_bare_specifier( } } -/// Holds the `ModuleGraph` and what parts of it are type checked. -pub struct ModuleGraphContainer { - // Allow only one request to update the graph data at a time, - // but allow other requests to read from it at any time even - // while another request is updating the data. - update_queue: Arc, - inner: Arc>>, -} - -impl ModuleGraphContainer { - pub fn new(graph_kind: GraphKind) -> Self { - Self { - update_queue: Default::default(), - inner: Arc::new(RwLock::new(Arc::new(ModuleGraph::new(graph_kind)))), - } - } - - /// Acquires a permit to modify the module graph without other code - /// having the chance to modify it. In the meantime, other code may - /// still read from the existing module graph. - pub async fn acquire_update_permit(&self) -> ModuleGraphUpdatePermit { - let permit = self.update_queue.acquire().await; - ModuleGraphUpdatePermit { - permit, - inner: self.inner.clone(), - graph: (**self.inner.read()).clone(), - } - } - - pub fn graph(&self) -> Arc { - self.inner.read().clone() - } -} - /// Gets if any of the specified root's "file:" dependents are in the /// provided changed set. pub fn has_graph_root_local_dependent_changed( @@ -828,31 +791,6 @@ pub fn has_graph_root_local_dependent_changed( false } -/// A permit for updating the module graph. When complete and -/// everything looks fine, calling `.commit()` will store the -/// new graph in the ModuleGraphContainer. -pub struct ModuleGraphUpdatePermit<'a> { - permit: TaskQueuePermit<'a>, - inner: Arc>>, - graph: ModuleGraph, -} - -impl<'a> ModuleGraphUpdatePermit<'a> { - /// Gets the module graph for mutation. - pub fn graph_mut(&mut self) -> &mut ModuleGraph { - &mut self.graph - } - - /// Saves the mutated module graph in the container - /// and returns an Arc to the new module graph. - pub fn commit(self) -> Arc { - let graph = Arc::new(self.graph); - *self.inner.write() = graph.clone(); - drop(self.permit); // explicit drop for clarity - graph - } -} - #[derive(Clone, Debug)] pub struct FileWatcherReporter { watcher_communicator: Arc, diff --git a/cli/lsp/testing/execution.rs b/cli/lsp/testing/execution.rs index ae4b62ea8833c..29b6a4f190ebf 100644 --- a/cli/lsp/testing/execution.rs +++ b/cli/lsp/testing/execution.rs @@ -219,10 +219,10 @@ impl TestRun { // file would have impact on other files, which is undesirable. let permissions = Permissions::from_options(&factory.cli_options().permissions_options()?)?; + let main_graph_container = factory.main_module_graph_container().await?; test::check_specifiers( - factory.cli_options(), factory.file_fetcher()?, - factory.module_load_preparer().await?, + main_graph_container, self .queue .iter() diff --git a/cli/main.rs b/cli/main.rs index 4f866ee21bc21..099bf060cc831 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -8,6 +8,7 @@ mod emit; mod errors; mod factory; mod file_fetcher; +mod graph_container; mod graph_util; mod http_util; mod js; @@ -30,6 +31,7 @@ use crate::args::flags_from_vec; use crate::args::DenoSubcommand; use crate::args::Flags; use crate::args::DENO_FUTURE; +use crate::graph_container::ModuleGraphContainer; use crate::util::display; use crate::util::v8::get_v8_flags_from_env; use crate::util::v8::init_v8_flags; @@ -112,18 +114,19 @@ async fn run_subcommand(flags: Flags) -> Result { }), DenoSubcommand::Cache(cache_flags) => spawn_subcommand(async move { let factory = CliFactory::from_flags(flags)?; - let module_load_preparer = factory.module_load_preparer().await?; let emitter = factory.emitter()?; - let graph_container = factory.graph_container(); - module_load_preparer + let main_graph_container = + factory.main_module_graph_container().await?; + main_graph_container .load_and_type_check_files(&cache_flags.files) .await?; - emitter.cache_module_emits(&graph_container.graph()) + emitter.cache_module_emits(&main_graph_container.graph()) }), DenoSubcommand::Check(check_flags) => spawn_subcommand(async move { let factory = CliFactory::from_flags(flags)?; - let module_load_preparer = factory.module_load_preparer().await?; - module_load_preparer + let main_graph_container = + factory.main_module_graph_container().await?; + main_graph_container .load_and_type_check_files(&check_flags.files) .await }), diff --git a/cli/module_loader.rs b/cli/module_loader.rs index 7d8cb130b296c..9a8441ccd9979 100644 --- a/cli/module_loader.rs +++ b/cli/module_loader.rs @@ -1,5 +1,12 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. +use std::borrow::Cow; +use std::cell::RefCell; +use std::pin::Pin; +use std::rc::Rc; +use std::str; +use std::sync::Arc; + use crate::args::jsr_url; use crate::args::CliOptions; use crate::args::DenoSubcommand; @@ -9,10 +16,12 @@ use crate::cache::ModuleInfoCache; use crate::cache::ParsedSourceCache; use crate::emit::Emitter; use crate::factory::CliFactory; +use crate::graph_container::MainModuleGraphContainer; +use crate::graph_container::ModuleGraphContainer; +use crate::graph_container::ModuleGraphUpdatePermit; use crate::graph_util::graph_lock_or_exit; use crate::graph_util::CreateGraphOptions; use crate::graph_util::ModuleGraphBuilder; -use crate::graph_util::ModuleGraphContainer; use crate::node; use crate::resolver::CliGraphResolver; use crate::resolver::CliNodeResolver; @@ -23,6 +32,7 @@ use crate::tools::check::TypeChecker; use crate::util::progress_bar::ProgressBar; use crate::util::text_encoding::code_without_source_map; use crate::util::text_encoding::source_map_from_code; +use crate::worker::ModuleLoaderAndSourceMapGetter; use crate::worker::ModuleLoaderFactory; use deno_ast::MediaType; @@ -36,7 +46,6 @@ use deno_core::futures::future::FutureExt; use deno_core::futures::Future; use deno_core::parking_lot::Mutex; use deno_core::resolve_url; -use deno_core::resolve_url_or_path; use deno_core::ModuleCodeString; use deno_core::ModuleLoader; use deno_core::ModuleSource; @@ -48,9 +57,11 @@ use deno_core::ResolutionKind; use deno_core::SourceMapGetter; use deno_graph::source::ResolutionMode; use deno_graph::source::Resolver; +use deno_graph::GraphKind; use deno_graph::JsModule; use deno_graph::JsonModule; use deno_graph::Module; +use deno_graph::ModuleGraph; use deno_graph::Resolution; use deno_lockfile::Lockfile; use deno_runtime::code_cache; @@ -58,12 +69,6 @@ use deno_runtime::deno_node::NodeResolutionMode; use deno_runtime::fs_util::code_timestamp; use deno_runtime::permissions::PermissionsContainer; use deno_semver::npm::NpmPackageReqReference; -use deno_terminal::colors; -use std::borrow::Cow; -use std::pin::Pin; -use std::rc::Rc; -use std::str; -use std::sync::Arc; pub async fn load_top_level_deps(factory: &CliFactory) -> Result<(), AnyError> { let npm_resolver = factory.npm_resolver().await?; @@ -83,12 +88,19 @@ pub async fn load_top_level_deps(factory: &CliFactory) -> Result<(), AnyError> { entry.value.cloned() } }) - .collect(); + .collect::>(); + let mut graph_permit = factory + .main_module_graph_container() + .await? + .acquire_update_permit() + .await; + let graph = graph_permit.graph_mut(); factory .module_load_preparer() .await? .prepare_module_load( - roots, + graph, + &roots, false, factory.cli_options().ts_type_lib_window(), deno_runtime::permissions::PermissionsContainer::allow_all(), @@ -101,7 +113,6 @@ pub async fn load_top_level_deps(factory: &CliFactory) -> Result<(), AnyError> { pub struct ModuleLoadPreparer { options: Arc, - graph_container: Arc, lockfile: Option>>, module_graph_builder: Arc, progress_bar: ProgressBar, @@ -112,7 +123,6 @@ impl ModuleLoadPreparer { #[allow(clippy::too_many_arguments)] pub fn new( options: Arc, - graph_container: Arc, lockfile: Option>>, module_graph_builder: Arc, progress_bar: ProgressBar, @@ -120,7 +130,6 @@ impl ModuleLoadPreparer { ) -> Self { Self { options, - graph_container, lockfile, module_graph_builder, progress_bar, @@ -135,7 +144,8 @@ impl ModuleLoadPreparer { #[allow(clippy::too_many_arguments)] pub async fn prepare_module_load( &self, - roots: Vec, + graph: &mut ModuleGraph, + roots: &[ModuleSpecifier], is_dynamic: bool, lib: TsTypeLib, permissions: PermissionsContainer, @@ -144,10 +154,7 @@ impl ModuleLoadPreparer { let _pb_clear_guard = self.progress_bar.clear_guard(); let mut cache = self.module_graph_builder.create_fetch_cacher(permissions); - log::debug!("Creating module graph."); - let mut graph_update_permit = - self.graph_container.acquire_update_permit().await; - let graph = graph_update_permit.graph_mut(); + log::debug!("Building module graph."); let has_type_checked = !graph.roots.is_empty(); self @@ -157,13 +164,13 @@ impl ModuleLoadPreparer { CreateGraphOptions { is_dynamic, graph_kind: graph.graph_kind(), - roots: roots.clone(), + roots: roots.to_vec(), loader: Some(&mut cache), }, ) .await?; - self.module_graph_builder.graph_roots_valid(graph, &roots)?; + self.module_graph_builder.graph_roots_valid(graph, roots)?; // If there is a lockfile... if let Some(lockfile) = &self.lockfile { @@ -174,9 +181,6 @@ impl ModuleLoadPreparer { lockfile.write().context("Failed writing lockfile.")?; } - // save the graph and get a reference to the new graph - let graph = graph_update_permit.commit(); - drop(_pb_clear_guard); // type check if necessary @@ -188,7 +192,7 @@ impl ModuleLoadPreparer { // created, we could avoid the clone of the graph here by providing // the actual graph on the first run and then getting the Arc // back from the return value. - (*graph).clone(), + graph.clone(), check::CheckOptions { build_fast_check_graph: true, lib, @@ -204,154 +208,23 @@ impl ModuleLoadPreparer { Ok(()) } - - /// Helper around prepare_module_load that loads and type checks - /// the provided files. - pub async fn load_and_type_check_files( - &self, - files: &[String], - ) -> Result<(), AnyError> { - let lib = self.options.ts_type_lib_window(); - - let specifiers = self.collect_specifiers(files)?; - - if specifiers.is_empty() { - log::warn!("{} No matching files found.", colors::yellow("Warning")); - } - - self - .prepare_module_load( - specifiers, - false, - lib, - PermissionsContainer::allow_all(), - ) - .await - } - - fn collect_specifiers( - &self, - files: &[String], - ) -> Result, AnyError> { - let excludes = self.options.resolve_config_excludes()?; - Ok( - files - .iter() - .filter_map(|file| { - let file_url = - resolve_url_or_path(file, self.options.initial_cwd()).ok()?; - if file_url.scheme() != "file" { - return Some(file_url); - } - // ignore local files that match any of files listed in `exclude` option - let file_path = file_url.to_file_path().ok()?; - if excludes.matches_path(&file_path) { - None - } else { - Some(file_url) - } - }) - .collect::>(), - ) - } -} - -struct PreparedModuleLoader { - emitter: Arc, - graph_container: Arc, - parsed_source_cache: Arc, -} - -impl PreparedModuleLoader { - pub fn load_prepared_module( - &self, - specifier: &ModuleSpecifier, - maybe_referrer: Option<&ModuleSpecifier>, - ) -> Result { - if specifier.scheme() == "node" { - unreachable!(); // Node built-in modules should be handled internally. - } - - let graph = self.graph_container.graph(); - match graph.get(specifier) { - Some(deno_graph::Module::Json(JsonModule { - source, - media_type, - specifier, - .. - })) => Ok(ModuleCodeStringSource { - code: source.clone().into(), - found_url: specifier.clone(), - media_type: *media_type, - }), - Some(deno_graph::Module::Js(JsModule { - source, - media_type, - specifier, - .. - })) => { - let code: ModuleCodeString = match media_type { - MediaType::JavaScript - | MediaType::Unknown - | MediaType::Cjs - | MediaType::Mjs - | MediaType::Json => source.clone().into(), - MediaType::Dts | MediaType::Dcts | MediaType::Dmts => { - Default::default() - } - MediaType::TypeScript - | MediaType::Mts - | MediaType::Cts - | MediaType::Jsx - | MediaType::Tsx => { - // get emit text - self - .emitter - .emit_parsed_source(specifier, *media_type, source)? - } - MediaType::TsBuildInfo | MediaType::Wasm | MediaType::SourceMap => { - panic!("Unexpected media type {media_type} for {specifier}") - } - }; - - // at this point, we no longer need the parsed source in memory, so free it - self.parsed_source_cache.free(specifier); - - Ok(ModuleCodeStringSource { - code, - found_url: specifier.clone(), - media_type: *media_type, - }) - } - Some( - deno_graph::Module::External(_) - | deno_graph::Module::Node(_) - | deno_graph::Module::Npm(_), - ) - | None => { - let mut msg = format!("Loading unprepared module: {specifier}"); - if let Some(referrer) = maybe_referrer { - msg = format!("{}, imported from: {}", msg, referrer.as_str()); - } - Err(anyhow!(msg)) - } - } - } } struct SharedCliModuleLoaderState { + graph_kind: GraphKind, lib_window: TsTypeLib, lib_worker: TsTypeLib, is_inspecting: bool, is_repl: bool, - graph_container: Arc, + code_cache: Option>, + emitter: Arc, + main_module_graph_container: Arc, + module_info_cache: Arc, module_load_preparer: Arc, - prepared_module_loader: PreparedModuleLoader, - resolver: Arc, node_resolver: Arc, npm_module_loader: NpmModuleLoader, - code_cache: Option>, - module_info_cache: Arc, + parsed_source_cache: Arc, + resolver: Arc, } pub struct CliModuleLoaderFactory { @@ -362,18 +235,19 @@ impl CliModuleLoaderFactory { #[allow(clippy::too_many_arguments)] pub fn new( options: &CliOptions, + code_cache: Option>, emitter: Arc, - graph_container: Arc, + main_module_graph_container: Arc, + module_info_cache: Arc, module_load_preparer: Arc, - parsed_source_cache: Arc, - resolver: Arc, node_resolver: Arc, npm_module_loader: NpmModuleLoader, - code_cache: Option>, - module_info_cache: Arc, + parsed_source_cache: Arc, + resolver: Arc, ) -> Self { Self { shared: Arc::new(SharedCliModuleLoaderState { + graph_kind: options.graph_kind(), lib_window: options.ts_type_lib_window(), lib_worker: options.ts_type_lib_worker(), is_inspecting: options.is_inspecting(), @@ -381,34 +255,39 @@ impl CliModuleLoaderFactory { options.sub_command(), DenoSubcommand::Repl(_) | DenoSubcommand::Jupyter(_) ), - prepared_module_loader: PreparedModuleLoader { - emitter, - graph_container: graph_container.clone(), - parsed_source_cache, - }, - graph_container, + code_cache, + emitter, + main_module_graph_container, + module_info_cache, module_load_preparer, - resolver, node_resolver, npm_module_loader, - code_cache, - module_info_cache, + parsed_source_cache, + resolver, }), } } - fn create_with_lib( + fn create_with_lib( &self, + graph_container: TGraphContainer, lib: TsTypeLib, root_permissions: PermissionsContainer, dynamic_permissions: PermissionsContainer, - ) -> Rc { - Rc::new(CliModuleLoader { + ) -> ModuleLoaderAndSourceMapGetter { + let loader = Rc::new(CliModuleLoader { lib, root_permissions, dynamic_permissions, + graph_container, + emitter: self.shared.emitter.clone(), + parsed_source_cache: self.shared.parsed_source_cache.clone(), shared: self.shared.clone(), - }) + }); + ModuleLoaderAndSourceMapGetter { + module_loader: loader.clone(), + source_map_getter: Some(loader), + } } } @@ -417,8 +296,9 @@ impl ModuleLoaderFactory for CliModuleLoaderFactory { &self, root_permissions: PermissionsContainer, dynamic_permissions: PermissionsContainer, - ) -> Rc { + ) -> ModuleLoaderAndSourceMapGetter { self.create_with_lib( + (*self.shared.main_module_graph_container).clone(), self.shared.lib_window, root_permissions, dynamic_permissions, @@ -429,22 +309,20 @@ impl ModuleLoaderFactory for CliModuleLoaderFactory { &self, root_permissions: PermissionsContainer, dynamic_permissions: PermissionsContainer, - ) -> Rc { + ) -> ModuleLoaderAndSourceMapGetter { self.create_with_lib( + // create a fresh module graph for the worker + WorkerModuleGraphContainer::new(Arc::new(ModuleGraph::new( + self.shared.graph_kind, + ))), self.shared.lib_worker, root_permissions, dynamic_permissions, ) } - - fn create_source_map_getter(&self) -> Option> { - Some(Rc::new(CliSourceMapGetter { - shared: self.shared.clone(), - })) - } } -struct CliModuleLoader { +struct CliModuleLoader { lib: TsTypeLib, /// The initial set of permissions used to resolve the static imports in the /// worker. These are "allow all" for main worker, and parent thread @@ -454,9 +332,12 @@ struct CliModuleLoader { /// "root permissions" for Web Worker. dynamic_permissions: PermissionsContainer, shared: Arc, + emitter: Arc, + parsed_source_cache: Arc, + graph_container: TGraphContainer, } -impl CliModuleLoader { +impl CliModuleLoader { fn load_sync( &self, specifier: &ModuleSpecifier, @@ -476,10 +357,7 @@ impl CliModuleLoader { { result? } else { - self - .shared - .prepared_module_loader - .load_prepared_module(specifier, maybe_referrer)? + self.load_prepared_module(specifier, maybe_referrer)? }; let code = if self.shared.is_inspecting { // we need the code with the source map in order for @@ -581,7 +459,7 @@ impl CliModuleLoader { }; } - let graph = self.shared.graph_container.graph(); + let graph = self.graph_container.graph(); let maybe_resolved = match graph.get(referrer) { Some(Module::Js(module)) => { module.dependencies.get(specifier).map(|d| &d.maybe_code) @@ -695,9 +573,86 @@ impl CliModuleLoader { .map(|timestamp| timestamp.to_string())?; Ok(Some(timestamp)) } + + fn load_prepared_module( + &self, + specifier: &ModuleSpecifier, + maybe_referrer: Option<&ModuleSpecifier>, + ) -> Result { + if specifier.scheme() == "node" { + unreachable!(); // Node built-in modules should be handled internally. + } + + let graph = self.graph_container.graph(); + match graph.get(specifier) { + Some(deno_graph::Module::Json(JsonModule { + source, + media_type, + specifier, + .. + })) => Ok(ModuleCodeStringSource { + code: source.clone().into(), + found_url: specifier.clone(), + media_type: *media_type, + }), + Some(deno_graph::Module::Js(JsModule { + source, + media_type, + specifier, + .. + })) => { + let code: ModuleCodeString = match media_type { + MediaType::JavaScript + | MediaType::Unknown + | MediaType::Cjs + | MediaType::Mjs + | MediaType::Json => source.clone().into(), + MediaType::Dts | MediaType::Dcts | MediaType::Dmts => { + Default::default() + } + MediaType::TypeScript + | MediaType::Mts + | MediaType::Cts + | MediaType::Jsx + | MediaType::Tsx => { + // get emit text + self + .emitter + .emit_parsed_source(specifier, *media_type, source)? + } + MediaType::TsBuildInfo | MediaType::Wasm | MediaType::SourceMap => { + panic!("Unexpected media type {media_type} for {specifier}") + } + }; + + // at this point, we no longer need the parsed source in memory, so free it + self.parsed_source_cache.free(specifier); + + Ok(ModuleCodeStringSource { + code, + found_url: specifier.clone(), + media_type: *media_type, + }) + } + Some( + deno_graph::Module::External(_) + | deno_graph::Module::Node(_) + | deno_graph::Module::Npm(_), + ) + | None => { + let mut msg = format!("Loading unprepared module: {specifier}"); + if let Some(referrer) = maybe_referrer { + msg = format!("{}, imported from: {}", msg, referrer.as_str()); + } + Err(anyhow!(msg)) + } + } + } } -impl ModuleLoader for CliModuleLoader { +impl ModuleLoader + for CliModuleLoader +{ fn resolve( &self, specifier: &str, @@ -747,13 +702,12 @@ impl ModuleLoader for CliModuleLoader { _maybe_referrer: Option, is_dynamic: bool, ) -> Pin>>> { - if let Some(result) = - self.shared.npm_module_loader.maybe_prepare_load(specifier) - { - return Box::pin(deno_core::futures::future::ready(result)); + if self.shared.node_resolver.in_npm_package(specifier) { + return Box::pin(deno_core::futures::future::ready(Ok(()))); } let specifier = specifier.clone(); + let graph_container = self.graph_container.clone(); let module_load_preparer = self.shared.module_load_preparer.clone(); let root_permissions = if is_dynamic { @@ -764,9 +718,19 @@ impl ModuleLoader for CliModuleLoader { let lib = self.lib; async move { + let mut update_permit = graph_container.acquire_update_permit().await; + let graph = update_permit.graph_mut(); module_load_preparer - .prepare_module_load(vec![specifier], is_dynamic, lib, root_permissions) - .await + .prepare_module_load( + graph, + &[specifier], + is_dynamic, + lib, + root_permissions, + ) + .await?; + update_permit.commit(); + Ok(()) } .boxed_local() } @@ -795,15 +759,13 @@ impl ModuleLoader for CliModuleLoader { ); } } - async {}.boxed_local() + std::future::ready(()).boxed_local() } } -struct CliSourceMapGetter { - shared: Arc, -} - -impl SourceMapGetter for CliSourceMapGetter { +impl SourceMapGetter + for CliModuleLoader +{ fn get_source_map(&self, file_name: &str) -> Option> { let specifier = resolve_url(file_name).ok()?; match specifier.scheme() { @@ -812,11 +774,7 @@ impl SourceMapGetter for CliSourceMapGetter { "wasm" | "file" | "http" | "https" | "data" | "blob" => (), _ => return None, } - let source = self - .shared - .prepared_module_loader - .load_prepared_module(&specifier, None) - .ok()?; + let source = self.load_prepared_module(&specifier, None).ok()?; source_map_from_code(&source.code) } @@ -825,7 +783,7 @@ impl SourceMapGetter for CliSourceMapGetter { file_name: &str, line_number: usize, ) -> Option { - let graph = self.shared.graph_container.graph(); + let graph = self.graph_container.graph(); let code = match graph.get(&resolve_url(file_name).ok()?) { Some(deno_graph::Module::Js(module)) => &module.source, Some(deno_graph::Module::Json(module)) => &module.source, @@ -844,3 +802,54 @@ impl SourceMapGetter for CliSourceMapGetter { } } } + +/// Holds the `ModuleGraph` in workers. +#[derive(Clone)] +struct WorkerModuleGraphContainer { + // Allow only one request to update the graph data at a time, + // but allow other requests to read from it at any time even + // while another request is updating the data. + update_queue: Rc, + inner: Rc>>, +} + +impl WorkerModuleGraphContainer { + pub fn new(module_graph: Arc) -> Self { + Self { + update_queue: Default::default(), + inner: Rc::new(RefCell::new(module_graph)), + } + } +} + +impl ModuleGraphContainer for WorkerModuleGraphContainer { + async fn acquire_update_permit(&self) -> impl ModuleGraphUpdatePermit { + let permit = self.update_queue.acquire().await; + WorkerModuleGraphUpdatePermit { + permit, + inner: self.inner.clone(), + graph: (**self.inner.borrow()).clone(), + } + } + + fn graph(&self) -> Arc { + self.inner.borrow().clone() + } +} + +struct WorkerModuleGraphUpdatePermit { + permit: deno_core::unsync::TaskQueuePermit, + inner: Rc>>, + graph: ModuleGraph, +} + +impl ModuleGraphUpdatePermit for WorkerModuleGraphUpdatePermit { + fn graph_mut(&mut self) -> &mut ModuleGraph { + &mut self.graph + } + + fn commit(self) { + *self.inner.borrow_mut() = Arc::new(self.graph); + drop(self.permit); // explicit drop for clarity + } +} diff --git a/cli/resolver.rs b/cli/resolver.rs index a11a12b5d76c0..4b5c99292e6cb 100644 --- a/cli/resolver.rs +++ b/cli/resolver.rs @@ -91,8 +91,8 @@ impl CliNodeResolver { } } - pub fn in_npm_package(&self, referrer: &ModuleSpecifier) -> bool { - self.npm_resolver.in_npm_package(referrer) + pub fn in_npm_package(&self, specifier: &ModuleSpecifier) -> bool { + self.npm_resolver.in_npm_package(specifier) } pub fn get_closest_package_json( @@ -249,6 +249,7 @@ impl CliNodeResolver { } } +#[derive(Clone)] pub struct NpmModuleLoader { cjs_resolutions: Arc, node_code_translator: Arc, @@ -271,18 +272,6 @@ impl NpmModuleLoader { } } - pub fn maybe_prepare_load( - &self, - specifier: &ModuleSpecifier, - ) -> Option> { - if self.node_resolver.in_npm_package(specifier) { - // nothing to prepare - Some(Ok(())) - } else { - None - } - } - pub fn load_sync_if_in_npm_package( &self, specifier: &ModuleSpecifier, diff --git a/cli/standalone/mod.rs b/cli/standalone/mod.rs index 4b7962a5f0c0c..37720bd541b33 100644 --- a/cli/standalone/mod.rs +++ b/cli/standalone/mod.rs @@ -27,6 +27,7 @@ use crate::util::progress_bar::ProgressBarStyle; use crate::util::v8::construct_v8_flags; use crate::worker::CliMainWorkerFactory; use crate::worker::CliMainWorkerOptions; +use crate::worker::ModuleLoaderAndSourceMapGetter; use crate::worker::ModuleLoaderFactory; use deno_ast::MediaType; use deno_core::anyhow::Context; @@ -282,30 +283,30 @@ impl ModuleLoaderFactory for StandaloneModuleLoaderFactory { &self, root_permissions: PermissionsContainer, dynamic_permissions: PermissionsContainer, - ) -> Rc { - Rc::new(EmbeddedModuleLoader { - shared: self.shared.clone(), - root_permissions, - dynamic_permissions, - }) + ) -> ModuleLoaderAndSourceMapGetter { + ModuleLoaderAndSourceMapGetter { + module_loader: Rc::new(EmbeddedModuleLoader { + shared: self.shared.clone(), + root_permissions, + dynamic_permissions, + }), + source_map_getter: None, + } } fn create_for_worker( &self, root_permissions: PermissionsContainer, dynamic_permissions: PermissionsContainer, - ) -> Rc { - Rc::new(EmbeddedModuleLoader { - shared: self.shared.clone(), - root_permissions, - dynamic_permissions, - }) - } - - fn create_source_map_getter( - &self, - ) -> Option> { - None + ) -> ModuleLoaderAndSourceMapGetter { + ModuleLoaderAndSourceMapGetter { + module_loader: Rc::new(EmbeddedModuleLoader { + shared: self.shared.clone(), + root_permissions, + dynamic_permissions, + }), + source_map_getter: None, + } } } diff --git a/cli/tools/bench/mod.rs b/cli/tools/bench/mod.rs index a6fb9177639c8..dd94205cb8001 100644 --- a/cli/tools/bench/mod.rs +++ b/cli/tools/bench/mod.rs @@ -8,7 +8,6 @@ use crate::display::write_json_to_stdout; use crate::factory::CliFactory; use crate::factory::CliFactoryBuilder; use crate::graph_util::has_graph_root_local_dependent_changed; -use crate::module_loader::ModuleLoadPreparer; use crate::ops; use crate::tools::test::format_test_error; use crate::tools::test::TestFilter; @@ -145,24 +144,6 @@ fn create_reporter( Box::new(ConsoleReporter::new(show_output)) } -/// Type check a collection of module and document specifiers. -async fn check_specifiers( - cli_options: &CliOptions, - module_load_preparer: &ModuleLoadPreparer, - specifiers: Vec, -) -> Result<(), AnyError> { - let lib = cli_options.ts_type_lib_window(); - module_load_preparer - .prepare_module_load( - specifiers, - false, - lib, - PermissionsContainer::allow_all(), - ) - .await?; - Ok(()) -} - /// Run a single specifier as an executable bench module. async fn bench_specifier( worker_factory: Arc, @@ -445,12 +426,8 @@ pub async fn run_benchmarks( return Err(generic_error("No bench modules found")); } - check_specifiers( - cli_options, - factory.module_load_preparer().await?, - specifiers.clone(), - ) - .await?; + let main_graph_container = factory.main_module_graph_container().await?; + main_graph_container.check_specifiers(&specifiers).await?; if bench_options.no_run { return Ok(()); @@ -507,7 +484,6 @@ pub async fn run_benchmarks_with_watch( let graph_kind = cli_options.type_check_mode().as_graph_kind(); let module_graph_creator = factory.module_graph_creator().await?; - let module_load_preparer = factory.module_load_preparer().await?; let bench_modules = collect_specifiers( bench_options.files.clone(), @@ -559,7 +535,10 @@ pub async fn run_benchmarks_with_watch( .filter(|specifier| bench_modules_to_reload.contains(specifier)) .collect::>(); - check_specifiers(cli_options, module_load_preparer, specifiers.clone()) + factory + .main_module_graph_container() + .await? + .check_specifiers(&specifiers) .await?; if bench_options.no_run { diff --git a/cli/tools/installer.rs b/cli/tools/installer.rs index 9ff17ccc30474..b13dea6fd90db 100644 --- a/cli/tools/installer.rs +++ b/cli/tools/installer.rs @@ -284,8 +284,9 @@ pub async fn install_command( }; // ensure the module is cached - CliFactory::from_flags(flags.clone())? - .module_load_preparer() + let factory = CliFactory::from_flags(flags.clone())?; + factory + .main_module_graph_container() .await? .load_and_type_check_files(&[install_flags_global.module_url.clone()]) .await?; diff --git a/cli/tools/test/mod.rs b/cli/tools/test/mod.rs index 94d4caee01ccb..2ff7203b769f9 100644 --- a/cli/tools/test/mod.rs +++ b/cli/tools/test/mod.rs @@ -10,8 +10,8 @@ use crate::factory::CliFactory; use crate::factory::CliFactoryBuilder; use crate::file_fetcher::File; use crate::file_fetcher::FileFetcher; +use crate::graph_container::MainModuleGraphContainer; use crate::graph_util::has_graph_root_local_dependent_changed; -use crate::module_loader::ModuleLoadPreparer; use crate::ops; use crate::util::file_watcher; use crate::util::fs::collect_specifiers; @@ -1305,12 +1305,10 @@ async fn fetch_inline_files( /// Type check a collection of module and document specifiers. pub async fn check_specifiers( - cli_options: &CliOptions, file_fetcher: &FileFetcher, - module_load_preparer: &ModuleLoadPreparer, + main_graph_container: &Arc, specifiers: Vec<(ModuleSpecifier, TestMode)>, ) -> Result<(), AnyError> { - let lib = cli_options.ts_type_lib_window(); let inline_files = fetch_inline_files( file_fetcher, specifiers @@ -1346,13 +1344,8 @@ pub async fn check_specifiers( } } - module_load_preparer - .prepare_module_load( - module_specifiers, - false, - lib, - PermissionsContainer::allow_all(), - ) + main_graph_container + .check_specifiers(&module_specifiers) .await?; Ok(()) @@ -1701,7 +1694,6 @@ pub async fn run_tests( let cli_options = factory.cli_options(); let test_options = cli_options.resolve_test_options(test_flags)?; let file_fetcher = factory.file_fetcher()?; - let module_load_preparer = factory.module_load_preparer().await?; // Various test files should not share the same permissions in terms of // `PermissionsContainer` - otherwise granting/revoking permissions in one // file would have impact on other files, which is undesirable. @@ -1721,10 +1713,11 @@ pub async fn run_tests( return Err(generic_error("No test modules found")); } + let main_graph_container = factory.main_module_graph_container().await?; + check_specifiers( - cli_options, file_fetcher, - module_load_preparer, + main_graph_container, specifiers_with_mode.clone(), ) .await?; @@ -1863,7 +1856,6 @@ pub async fn run_tests_with_watch( let worker_factory = Arc::new(factory.create_cli_main_worker_factory().await?); - let module_load_preparer = factory.module_load_preparer().await?; let specifiers_with_mode = fetch_specifiers_with_test_mode( &cli_options, file_fetcher, @@ -1875,10 +1867,11 @@ pub async fn run_tests_with_watch( .filter(|(specifier, _)| test_modules_to_reload.contains(specifier)) .collect::>(); + let main_graph_container = + factory.main_module_graph_container().await?; check_specifiers( - &cli_options, file_fetcher, - module_load_preparer, + main_graph_container, specifiers_with_mode.clone(), ) .await?; diff --git a/cli/worker.rs b/cli/worker.rs index a49929ca5ac54..151a4ec4f9937 100644 --- a/cli/worker.rs +++ b/cli/worker.rs @@ -58,20 +58,23 @@ use crate::util::file_watcher::WatcherCommunicator; use crate::util::file_watcher::WatcherRestartMode; use crate::version; +pub struct ModuleLoaderAndSourceMapGetter { + pub module_loader: Rc, + pub source_map_getter: Option>, +} + pub trait ModuleLoaderFactory: Send + Sync { fn create_for_main( &self, root_permissions: PermissionsContainer, dynamic_permissions: PermissionsContainer, - ) -> Rc; + ) -> ModuleLoaderAndSourceMapGetter; fn create_for_worker( &self, root_permissions: PermissionsContainer, dynamic_permissions: PermissionsContainer, - ) -> Rc; - - fn create_source_map_getter(&self) -> Option>; + ) -> ModuleLoaderAndSourceMapGetter; } #[async_trait::async_trait(?Send)] @@ -549,11 +552,12 @@ impl CliMainWorkerFactory { (main_module, false) }; - let module_loader = shared + let ModuleLoaderAndSourceMapGetter { + module_loader, + source_map_getter, + } = shared .module_loader_factory .create_for_main(PermissionsContainer::allow_all(), permissions.clone()); - let maybe_source_map_getter = - shared.module_loader_factory.create_source_map_getter(); let maybe_inspector_server = shared.maybe_inspector_server.clone(); let create_web_worker_cb = @@ -627,7 +631,7 @@ impl CliMainWorkerFactory { .clone(), root_cert_store_provider: Some(shared.root_cert_store_provider.clone()), seed: shared.options.seed, - source_map_getter: maybe_source_map_getter, + source_map_getter, format_js_error_fn: Some(Arc::new(format_js_error)), create_web_worker_cb, maybe_inspector_server, @@ -769,12 +773,13 @@ fn create_web_worker_callback( Arc::new(move |args| { let maybe_inspector_server = shared.maybe_inspector_server.clone(); - let module_loader = shared.module_loader_factory.create_for_worker( + let ModuleLoaderAndSourceMapGetter { + module_loader, + source_map_getter, + } = shared.module_loader_factory.create_for_worker( args.parent_permissions.clone(), args.permissions.clone(), ); - let maybe_source_map_getter = - shared.module_loader_factory.create_source_map_getter(); let create_web_worker_cb = create_web_worker_callback(mode, shared.clone(), stdio.clone()); @@ -839,7 +844,7 @@ fn create_web_worker_callback( seed: shared.options.seed, create_web_worker_cb, format_js_error_fn: Some(Arc::new(format_js_error)), - source_map_getter: maybe_source_map_getter, + source_map_getter, module_loader, fs: shared.fs.clone(), npm_resolver: Some(shared.npm_resolver.clone().into_npm_resolver()), diff --git a/tests/specs/node/worker_threads_cache/__test__.jsonc b/tests/specs/node/worker_threads_cache/__test__.jsonc new file mode 100644 index 0000000000000..a47fed572dcd2 --- /dev/null +++ b/tests/specs/node/worker_threads_cache/__test__.jsonc @@ -0,0 +1,5 @@ +{ + "tempDir": true, + "args": "run -A main.ts", + "output": "main.out" +} diff --git a/tests/specs/node/worker_threads_cache/main.out b/tests/specs/node/worker_threads_cache/main.out new file mode 100644 index 0000000000000..d14c028e5b751 --- /dev/null +++ b/tests/specs/node/worker_threads_cache/main.out @@ -0,0 +1,2 @@ +[Module: null prototype] { default: true } +[Module: null prototype] { default: false } diff --git a/tests/specs/node/worker_threads_cache/main.ts b/tests/specs/node/worker_threads_cache/main.ts new file mode 100644 index 0000000000000..9703ac8f63ae0 --- /dev/null +++ b/tests/specs/node/worker_threads_cache/main.ts @@ -0,0 +1,13 @@ +import fs from "node:fs/promises"; +import { isMainThread, Worker } from "node:worker_threads"; + +await fs.writeFile("mod.mjs", "export default " + isMainThread); + +const path = new URL("mod.mjs", import.meta.url); +const i = await import(path.href); +console.log(i); + +if (isMainThread) { + const worker = new Worker(new URL("main.ts", import.meta.url)); + worker.on("message", (msg) => console.log(msg)); +} diff --git a/tests/wpt/runner/expectation.json b/tests/wpt/runner/expectation.json index 866b85d52ca59..1f2dfa6847577 100644 --- a/tests/wpt/runner/expectation.json +++ b/tests/wpt/runner/expectation.json @@ -8739,9 +8739,7 @@ "blob-url.any.worker-module.html": [ "Revoking a blob URL immediately after calling import will not fail" ], - "blob-url-workers.window.html": [ - "A revoked blob URL will not resolve in a worker even if it's in the window's module graph" - ], + "blob-url-workers.window.html": [], "microtasks": { "basic.any.html": [ "import() should not drain the microtask queue if it fails during specifier resolution",