diff options
| -rw-r--r-- | cli/factory.rs | 2 | ||||
| -rw-r--r-- | cli/graph_util.rs | 80 | ||||
| -rw-r--r-- | cli/module_loader.rs | 12 | ||||
| -rw-r--r-- | cli/tools/bench/mod.rs | 15 | ||||
| -rw-r--r-- | cli/tools/lint/mod.rs | 4 | ||||
| -rw-r--r-- | cli/tools/registry/mod.rs | 6 | ||||
| -rw-r--r-- | cli/tools/test/mod.rs | 13 | ||||
| -rw-r--r-- | tests/integration/publish_tests.rs | 9 | ||||
| -rw-r--r-- | tests/testdata/publish/sloppy_imports_not_enabled.out | 2 |
9 files changed, 70 insertions, 73 deletions
diff --git a/cli/factory.rs b/cli/factory.rs index 5bd5fe149..fb4297045 100644 --- a/cli/factory.rs +++ b/cli/factory.rs @@ -651,7 +651,6 @@ impl CliFactory { .get_or_try_init_async(async { Ok(Arc::new(ModuleGraphCreator::new( self.options.clone(), - self.fs().clone(), self.npm_resolver().await?.clone(), self.module_graph_builder().await?.clone(), self.maybe_lockfile().clone(), @@ -689,7 +688,6 @@ impl CliFactory { .get_or_try_init_async(async { Ok(Arc::new(ModuleLoadPreparer::new( self.options.clone(), - self.fs().clone(), self.graph_container().clone(), self.maybe_lockfile().clone(), self.module_graph_builder().await?.clone(), diff --git a/cli/graph_util.rs b/cli/graph_util.rs index b84cb3bb6..316eb7a21 100644 --- a/cli/graph_util.rs +++ b/cli/graph_util.rs @@ -60,27 +60,6 @@ pub struct GraphValidOptions { /// Check if `roots` and their deps are available. Returns `Ok(())` if /// so. Returns `Err(_)` if there is a known module graph or resolution -/// error statically reachable from `roots` and not a dynamic import. -pub fn graph_valid_with_cli_options( - graph: &ModuleGraph, - fs: &dyn FileSystem, - roots: &[ModuleSpecifier], - options: &CliOptions, -) -> Result<(), AnyError> { - graph_valid( - graph, - fs, - roots, - GraphValidOptions { - is_vendoring: false, - follow_type_only: options.type_check_mode().is_true(), - check_js: options.check_js(), - }, - ) -} - -/// Check if `roots` and their deps are available. Returns `Ok(())` if -/// so. Returns `Err(_)` if there is a known module graph or resolution /// error statically reachable from `roots`. /// /// It is preferable to use this over using deno_graph's API directly @@ -214,7 +193,6 @@ pub struct CreateGraphOptions<'a> { pub struct ModuleGraphCreator { options: Arc<CliOptions>, - fs: Arc<dyn FileSystem>, npm_resolver: Arc<dyn CliNpmResolver>, module_graph_builder: Arc<ModuleGraphBuilder>, lockfile: Option<Arc<Mutex<Lockfile>>>, @@ -224,7 +202,6 @@ pub struct ModuleGraphCreator { impl ModuleGraphCreator { pub fn new( options: Arc<CliOptions>, - fs: Arc<dyn FileSystem>, npm_resolver: Arc<dyn CliNpmResolver>, module_graph_builder: Arc<ModuleGraphBuilder>, lockfile: Option<Arc<Mutex<Lockfile>>>, @@ -232,7 +209,6 @@ impl ModuleGraphCreator { ) -> Self { Self { options, - fs, npm_resolver, lockfile, module_graph_builder, @@ -267,9 +243,10 @@ impl ModuleGraphCreator { .await } - pub async fn create_publish_graph( + pub async fn create_and_validate_publish_graph( &self, packages: &[WorkspaceMemberConfig], + build_fast_check_graph: bool, ) -> Result<ModuleGraph, AnyError> { let mut roots = Vec::new(); for package in packages { @@ -283,15 +260,18 @@ impl ModuleGraphCreator { loader: None, }) .await?; + self.graph_valid(&graph)?; if self.options.type_check_mode().is_true() { self.type_check_graph(graph.clone()).await?; } - self.module_graph_builder.build_fast_check_graph( - &mut graph, - BuildFastCheckGraphOptions { - workspace_fast_check: true, - }, - )?; + if build_fast_check_graph { + self.module_graph_builder.build_fast_check_graph( + &mut graph, + BuildFastCheckGraphOptions { + workspace_fast_check: true, + }, + )?; + } Ok(graph) } @@ -330,12 +310,7 @@ impl ModuleGraphCreator { }) .await?; - graph_valid_with_cli_options( - &graph, - self.fs.as_ref(), - &graph.roots, - &self.options, - )?; + self.graph_valid(&graph)?; if let Some(lockfile) = &self.lockfile { graph_lock_or_exit(&graph, &mut lockfile.lock()); } @@ -349,6 +324,10 @@ impl ModuleGraphCreator { } } + pub fn graph_valid(&self, graph: &ModuleGraph) -> Result<(), AnyError> { + self.module_graph_builder.graph_valid(graph) + } + async fn type_check_graph( &self, graph: ModuleGraph, @@ -658,6 +637,30 @@ impl ModuleGraphBuilder { permissions, ) } + + /// Check if `roots` and their deps are available. Returns `Ok(())` if + /// so. Returns `Err(_)` if there is a known module graph or resolution + /// error statically reachable from `roots` and not a dynamic import. + pub fn graph_valid(&self, graph: &ModuleGraph) -> Result<(), AnyError> { + self.graph_roots_valid(graph, &graph.roots) + } + + pub fn graph_roots_valid( + &self, + graph: &ModuleGraph, + roots: &[ModuleSpecifier], + ) -> Result<(), AnyError> { + graph_valid( + graph, + self.fs.as_ref(), + roots, + GraphValidOptions { + is_vendoring: false, + follow_type_only: self.options.type_check_mode().is_true(), + check_js: self.options.check_js(), + }, + ) + } } pub fn error_for_any_npm_specifier( @@ -697,7 +700,8 @@ pub fn enhanced_module_error_message( error: &ModuleError, ) -> String { let additional_message = match error { - ModuleError::Missing(specifier, _) => { + ModuleError::LoadingErr(specifier, _, _) // ex. "Is a directory" error + | ModuleError::Missing(specifier, _) => { SloppyImportsResolver::resolve_with_fs( fs, specifier, diff --git a/cli/module_loader.rs b/cli/module_loader.rs index 17ec535f9..0ff7ef4ed 100644 --- a/cli/module_loader.rs +++ b/cli/module_loader.rs @@ -7,7 +7,6 @@ use crate::args::TsTypeLib; use crate::cache::ParsedSourceCache; use crate::emit::Emitter; use crate::graph_util::graph_lock_or_exit; -use crate::graph_util::graph_valid_with_cli_options; use crate::graph_util::CreateGraphOptions; use crate::graph_util::ModuleGraphBuilder; use crate::graph_util::ModuleGraphContainer; @@ -51,7 +50,6 @@ use deno_graph::JsonModule; use deno_graph::Module; use deno_graph::Resolution; use deno_lockfile::Lockfile; -use deno_runtime::deno_fs; use deno_runtime::deno_node::NodeResolutionMode; use deno_runtime::permissions::PermissionsContainer; use deno_semver::npm::NpmPackageReqReference; @@ -65,7 +63,6 @@ use std::sync::Arc; pub struct ModuleLoadPreparer { options: Arc<CliOptions>, - fs: Arc<dyn deno_fs::FileSystem>, graph_container: Arc<ModuleGraphContainer>, lockfile: Option<Arc<Mutex<Lockfile>>>, module_graph_builder: Arc<ModuleGraphBuilder>, @@ -77,7 +74,6 @@ impl ModuleLoadPreparer { #[allow(clippy::too_many_arguments)] pub fn new( options: Arc<CliOptions>, - fs: Arc<dyn deno_fs::FileSystem>, graph_container: Arc<ModuleGraphContainer>, lockfile: Option<Arc<Mutex<Lockfile>>>, module_graph_builder: Arc<ModuleGraphBuilder>, @@ -86,7 +82,6 @@ impl ModuleLoadPreparer { ) -> Self { Self { options, - fs, graph_container, lockfile, module_graph_builder, @@ -134,12 +129,7 @@ impl ModuleLoadPreparer { ) .await?; - graph_valid_with_cli_options( - graph, - self.fs.as_ref(), - &roots, - &self.options, - )?; + self.module_graph_builder.graph_roots_valid(graph, &roots)?; // If there is a lockfile... if let Some(lockfile) = &self.lockfile { diff --git a/cli/tools/bench/mod.rs b/cli/tools/bench/mod.rs index 2f0d59f49..43b7103cd 100644 --- a/cli/tools/bench/mod.rs +++ b/cli/tools/bench/mod.rs @@ -7,7 +7,6 @@ use crate::colors; use crate::display::write_json_to_stdout; use crate::factory::CliFactory; use crate::factory::CliFactoryBuilder; -use crate::graph_util::graph_valid_with_cli_options; use crate::graph_util::has_graph_root_local_dependent_changed; use crate::module_loader::ModuleLoadPreparer; use crate::ops; @@ -526,14 +525,10 @@ pub async fn run_benchmarks_with_watch( Permissions::from_options(&cli_options.permissions_options())?; let graph = module_graph_creator - .create_graph(graph_kind, bench_modules.clone()) + .create_graph(graph_kind, bench_modules) .await?; - graph_valid_with_cli_options( - &graph, - factory.fs().as_ref(), - &bench_modules, - cli_options, - )?; + module_graph_creator.graph_valid(&graph)?; + let bench_modules = &graph.roots; let bench_modules_to_reload = if let Some(changed_paths) = changed_paths { @@ -542,7 +537,7 @@ pub async fn run_benchmarks_with_watch( for bench_module_specifier in bench_modules { if has_graph_root_local_dependent_changed( &graph, - &bench_module_specifier, + bench_module_specifier, &changed_paths, ) { result.push(bench_module_specifier.clone()); @@ -556,7 +551,7 @@ pub async fn run_benchmarks_with_watch( let worker_factory = Arc::new(factory.create_cli_main_worker_factory().await?); - // todo(THIS PR): why are we collecting specifiers twice in a row? + // todo(dsherret): why are we collecting specifiers twice in a row? // Seems like a perf bug. let specifiers = collect_specifiers(bench_options.files, is_supported_bench_path)? diff --git a/cli/tools/lint/mod.rs b/cli/tools/lint/mod.rs index 1240b391f..ee7350fb4 100644 --- a/cli/tools/lint/mod.rs +++ b/cli/tools/lint/mod.rs @@ -184,7 +184,9 @@ async fn lint_files( .filter_map(|p| ModuleSpecifier::from_file_path(p).ok()) .collect::<HashSet<_>>(); futures.push(deno_core::unsync::spawn(async move { - let graph = module_graph_creator.create_publish_graph(&members).await?; + let graph = module_graph_creator + .create_and_validate_publish_graph(&members, true) + .await?; // todo(dsherret): this isn't exactly correct as linting isn't properly // setup to handle workspaces. Iterating over the workspace members // should be done at a higher level because it also needs to take into diff --git a/cli/tools/registry/mod.rs b/cli/tools/registry/mod.rs index 9c670ebc1..7b940592f 100644 --- a/cli/tools/registry/mod.rs +++ b/cli/tools/registry/mod.rs @@ -812,8 +812,10 @@ async fn build_and_check_graph_for_publish( diagnostics_collector: &PublishDiagnosticsCollector, packages: &[WorkspaceMemberConfig], ) -> Result<Arc<deno_graph::ModuleGraph>, deno_core::anyhow::Error> { - let graph = module_graph_creator.create_publish_graph(packages).await?; - graph.valid()?; + let build_fast_check_graph = !allow_slow_types; + let graph = module_graph_creator + .create_and_validate_publish_graph(packages, build_fast_check_graph) + .await?; // todo(dsherret): move to lint rule collect_invalid_external_imports(&graph, diagnostics_collector); diff --git a/cli/tools/test/mod.rs b/cli/tools/test/mod.rs index eb0cdd4ce..13cf9f774 100644 --- a/cli/tools/test/mod.rs +++ b/cli/tools/test/mod.rs @@ -10,7 +10,6 @@ use crate::factory::CliFactory; use crate::factory::CliFactoryBuilder; use crate::file_fetcher::File; use crate::file_fetcher::FileFetcher; -use crate::graph_util::graph_valid_with_cli_options; use crate::graph_util::has_graph_root_local_dependent_changed; use crate::module_loader::ModuleLoadPreparer; use crate::ops; @@ -1612,14 +1611,10 @@ pub async fn run_tests_with_watch( let permissions = Permissions::from_options(&cli_options.permissions_options())?; let graph = module_graph_creator - .create_graph(graph_kind, test_modules.clone()) + .create_graph(graph_kind, test_modules) .await?; - graph_valid_with_cli_options( - &graph, - factory.fs().as_ref(), - &test_modules, - &cli_options, - )?; + module_graph_creator.graph_valid(&graph)?; + let test_modules = &graph.roots; let test_modules_to_reload = if let Some(changed_paths) = changed_paths { @@ -1628,7 +1623,7 @@ pub async fn run_tests_with_watch( for test_module_specifier in test_modules { if has_graph_root_local_dependent_changed( &graph, - &test_module_specifier, + test_module_specifier, &changed_paths, ) { result.push(test_module_specifier.clone()); diff --git a/tests/integration/publish_tests.rs b/tests/integration/publish_tests.rs index ce93ccda3..cb1072bc0 100644 --- a/tests/integration/publish_tests.rs +++ b/tests/integration/publish_tests.rs @@ -275,6 +275,15 @@ itest!(sloppy_imports { http_server: true, }); +itest!(sloppy_imports_not_enabled { + args: "publish --token 'sadfasdf' --dry-run", + output: "publish/sloppy_imports_not_enabled.out", + cwd: Some("publish/sloppy_imports"), + envs: env_vars_for_jsr_tests(), + http_server: true, + exit_code: 1, +}); + itest!(sloppy_imports_no_warnings { args: "publish --token 'sadfasdf' --dry-run --unstable-sloppy-imports", output: "publish/sloppy_imports_no_warnings.out", diff --git a/tests/testdata/publish/sloppy_imports_not_enabled.out b/tests/testdata/publish/sloppy_imports_not_enabled.out new file mode 100644 index 000000000..c2f74ad2c --- /dev/null +++ b/tests/testdata/publish/sloppy_imports_not_enabled.out @@ -0,0 +1,2 @@ +error: [WILDCARD] Maybe specify path to 'index.ts' file in directory instead or run with --unstable-sloppy-imports + at file:///[WILDCARD]/sloppy_imports/mod.ts:1:20 |