From 7ed90a20d04982ae15a52ae2378cbffd4b6839df Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 5 Jun 2024 11:04:16 -0400 Subject: [PATCH] fix: better handling of npm resolution occurring on workers (#24094) Closes https://github.com/denoland/deno/issues/24063 --- .dprint.json | 8 +- Cargo.lock | 40 ++-- Cargo.toml | 2 +- cli/Cargo.toml | 16 +- cli/args/mod.rs | 2 +- cli/cache/emit.rs | 40 ++-- cli/cache/parsed_source.rs | 2 +- cli/emit.rs | 65 +++++-- cli/graph_util.rs | 23 +-- cli/lsp/analysis.rs | 13 +- cli/lsp/code_lens.rs | 7 +- cli/lsp/completions.rs | 2 +- cli/lsp/diagnostics.rs | 2 +- cli/lsp/documents.rs | 120 ++++++------ cli/lsp/language_server.rs | 10 +- cli/lsp/resolver.rs | 11 +- cli/lsp/testing/collectors.rs | 4 +- cli/module_loader.rs | 37 +--- cli/node.rs | 2 +- cli/npm/managed/cache/mod.rs | 1 + cli/npm/managed/cache/registry_info.rs | 107 +++++------ cli/npm/managed/cache/tarball.rs | 92 +++++----- cli/npm/managed/cache/value_creator.rs | 101 ++++++++++ cli/npm/managed/installer.rs | 3 +- cli/npm/managed/mod.rs | 57 +++--- cli/npm/managed/registry.rs | 58 ++---- cli/npm/managed/resolution.rs | 46 ++--- cli/resolver.rs | 113 +++++++++--- cli/standalone/mod.rs | 2 +- cli/tools/bench/mod.rs | 4 +- cli/tools/coverage/mod.rs | 13 +- cli/tools/doc.rs | 2 +- cli/tools/fmt.rs | 8 +- cli/tools/lint/mod.rs | 8 +- cli/tools/registry/diagnostics.rs | 22 +-- cli/tools/registry/graph.rs | 4 +- cli/tools/registry/tar.rs | 2 +- cli/tools/registry/unfurl.rs | 50 ++--- cli/tools/repl/session.rs | 11 +- cli/tools/test/mod.rs | 8 +- cli/tools/vendor/analyze.rs | 3 +- cli/tools/vendor/build.rs | 2 +- cli/tools/vendor/import_map.rs | 2 +- cli/util/text_encoding.rs | 173 +++++++++++++++--- runtime/shared.rs | 11 +- tests/Cargo.toml | 2 +- tests/integration/watcher_tests.rs | 12 +- tests/specs/README.md | 4 +- .../lint/no_slow_types/no_slow_types.out | 3 + .../no_slow_types_entrypoint.out | 3 + .../no_slow_types_entrypoint.out | 3 + .../lint/no_slow_types_workspace/output.out | 4 + tests/specs/mod.rs | 36 +++- tests/specs/npm/workers/__test__.jsonc | 5 + tests/specs/npm/workers/main.out | 5 + tests/specs/npm/workers/main.ts | 9 + tests/specs/npm/workers/worker1.ts | 9 + tests/specs/npm/workers/worker2.ts | 6 + tests/specs/npm/workers/worker3.ts | 6 + .../publish.out | 2 + .../publish/has_slow_types/has_slow_types.out | 1 + .../publish/invalid_import/invalid_import.out | 2 + .../invalid_import_esm_sh_suggestion.out | 2 + .../publish/missing_constraint/publish.out | 3 + .../publish/prefer_fast_check_graph/main.out | 1 + .../unanalyzable_dynamic_import.out | 1 + tests/specs/schema.json | 6 + .../doc/referenced_private_types_lint.out | 1 + tools/util.js | 6 + 69 files changed, 902 insertions(+), 539 deletions(-) create mode 100644 cli/npm/managed/cache/value_creator.rs create mode 100644 tests/specs/npm/workers/__test__.jsonc create mode 100644 tests/specs/npm/workers/main.out create mode 100644 tests/specs/npm/workers/main.ts create mode 100644 tests/specs/npm/workers/worker1.ts create mode 100644 tests/specs/npm/workers/worker2.ts create mode 100644 tests/specs/npm/workers/worker3.ts diff --git a/.dprint.json b/.dprint.json index d34155a014..f9cb60c74d 100644 --- a/.dprint.json +++ b/.dprint.json @@ -57,10 +57,10 @@ "ext/websocket/autobahn/reports" ], "plugins": [ - "https://plugins.dprint.dev/typescript-0.90.5.wasm", - "https://plugins.dprint.dev/json-0.19.2.wasm", - "https://plugins.dprint.dev/markdown-0.17.0.wasm", - "https://plugins.dprint.dev/toml-0.6.1.wasm", + "https://plugins.dprint.dev/typescript-0.91.1.wasm", + "https://plugins.dprint.dev/json-0.19.3.wasm", + "https://plugins.dprint.dev/markdown-0.17.1.wasm", + "https://plugins.dprint.dev/toml-0.6.2.wasm", "https://plugins.dprint.dev/exec-0.4.4.json@c207bf9b9a4ee1f0ecb75c594f774924baf62e8e53a2ce9d873816a408cecbf7" ] } diff --git a/Cargo.lock b/Cargo.lock index 18a9ed6cd8..b424ee82d7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1175,9 +1175,9 @@ dependencies = [ [[package]] name = "deno_ast" -version = "0.38.2" +version = "0.39.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "584547d27786a734536fde7088f8429d355569c39410427be44695c300618408" +checksum = "32edef567e3090862e865c75628f4d35ace80ca90e0fc5263a7d10fa307ae899" dependencies = [ "anyhow", "base64 0.21.7", @@ -1385,9 +1385,9 @@ dependencies = [ [[package]] name = "deno_doc" -version = "0.137.0" +version = "0.139.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "57f13d6254b2e05b014e8464647025fa28ef2f385c9c102744a27bd788eb3ebe" +checksum = "c9cd9891748fbd9847c9aeed31635c4c1b5d9a949f6fdd80613b082bdd863518" dependencies = [ "ammonia", "anyhow", @@ -1410,9 +1410,9 @@ dependencies = [ [[package]] name = "deno_emit" -version = "0.41.0" +version = "0.42.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ebe4b6c67f21a73901e962e92d51065f3c1bb42d2195bca8c2fef9f1808c4c2d" +checksum = "25bc64f886c76647400ed8f807ba7dba82e0b52e57e5426a83094cfe22ee19c9" dependencies = [ "anyhow", "base64 0.21.7", @@ -1478,9 +1478,9 @@ dependencies = [ [[package]] name = "deno_graph" -version = "0.77.2" +version = "0.78.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "192d6f61d5418c928d29b2666b916df65a3b5677ce454fc6a4b4969983a02abe" +checksum = "c4ccd2755a805983f96aeccd211c1f7585b6bfec77471f502c47227abe375682" dependencies = [ "anyhow", "async-trait", @@ -1589,9 +1589,9 @@ dependencies = [ [[package]] name = "deno_lint" -version = "0.59.1" +version = "0.60.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0568595fd7f8ad76ddf66d70c1c5e3e680ad95c3d5abb44556e94d824643d6e2" +checksum = "bf6a9540b371b123e3df4ab5fd59af0defc0d834e08ebfb3deacc41837963368" dependencies = [ "anyhow", "deno_ast", @@ -2269,9 +2269,9 @@ dependencies = [ [[package]] name = "dprint-plugin-json" -version = "0.19.2" +version = "0.19.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e292d0d74f3f51b1ff3e446c8809bcdd0b6079b49cf6c0d452c85927a2575246" +checksum = "a19f4a9f2f548b2098b8ec597d7bb40af133b6e9a3187c1d3c4caa101b8c93c3" dependencies = [ "anyhow", "dprint-core", @@ -2296,9 +2296,9 @@ dependencies = [ [[package]] name = "dprint-plugin-markdown" -version = "0.17.0" +version = "0.17.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2b2642e4a5f3a2262bb9baef8739f90d99b73ca21bc65f46c320a7817fd65438" +checksum = "da8df52eef864c2577ad3fb28c596935e2c0161eb09f6d5e239b10fecda2ec1c" dependencies = [ "anyhow", "dprint-core", @@ -2311,9 +2311,9 @@ dependencies = [ [[package]] name = "dprint-plugin-typescript" -version = "0.90.5" +version = "0.91.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d7c3c339020ebbbbbe5fc049350935ee2ea2ba5a3fc01f753588639a30404cda" +checksum = "4170a1aea5c8d899e9fa96be972931b1f0beaf6f6ba2f3f40a48a13071b376ea" dependencies = [ "anyhow", "deno_ast", @@ -2539,9 +2539,9 @@ dependencies = [ [[package]] name = "eszip" -version = "0.70.1" +version = "0.71.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e5f9947a8dd5ba292461c84a5bf142497e2840c4165994c5c3b3ae4954d38fef" +checksum = "3c3763e2d3e56ed5f770f9ab133aca20b1e7fa840f2408f79575ad96f942af2e" dependencies = [ "anyhow", "base64 0.21.7", @@ -2652,9 +2652,9 @@ checksum = "c007b1ae3abe1cb6f85a16305acd418b7ca6343b953633fee2b76d8f108b830f" [[package]] name = "file_test_runner" -version = "0.7.0" +version = "0.7.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b8797fcdc5c6b8c06839900c30f5c59b3541ef2bec218579470ce7b1afc17ee9" +checksum = "05b23dcc1b671771c6f59fdace6da685735c925f859733e8fd07fba6cae6462a" dependencies = [ "anyhow", "crossbeam-channel", diff --git a/Cargo.toml b/Cargo.toml index 464dc97987..845fefb951 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,7 +43,7 @@ license = "MIT" repository = "https://github.com/denoland/deno" [workspace.dependencies] -deno_ast = { version = "=0.38.2", features = ["transpiling"] } +deno_ast = { version = "=0.39.0", features = ["transpiling"] } deno_core = { version = "0.284.0" } deno_bench_util = { version = "0.148.0", path = "./bench_util" } diff --git a/cli/Cargo.toml b/cli/Cargo.toml index d0b65e12a6..d72ee794b8 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -67,17 +67,17 @@ deno_ast = { workspace = true, features = ["bundler", "cjs", "codegen", "proposa deno_cache_dir = { workspace = true } deno_config = "=0.16.4" deno_core = { workspace = true, features = ["include_js_files_for_snapshotting"] } -deno_doc = { version = "=0.137.0", features = ["html", "syntect"] } -deno_emit = "=0.41.0" -deno_graph = { version = "=0.77.2", features = ["tokio_executor"] } -deno_lint = { version = "=0.59.1", features = ["docs"] } +deno_doc = { version = "=0.139.0", features = ["html", "syntect"] } +deno_emit = "=0.42.0" +deno_graph = { version = "=0.78.0", features = ["tokio_executor"] } +deno_lint = { version = "=0.60.0", features = ["docs"] } deno_lockfile.workspace = true deno_npm = "=0.21.0" deno_runtime = { workspace = true, features = ["include_js_files_for_snapshotting"] } deno_semver = "=0.5.4" deno_task_shell = "=0.16.1" deno_terminal.workspace = true -eszip = "=0.70.1" +eszip = "=0.71.0" napi_sym.workspace = true async-trait.workspace = true @@ -96,10 +96,10 @@ dashmap = "5.5.3" data-encoding.workspace = true dissimilar = "=1.0.4" dotenvy = "0.15.7" -dprint-plugin-json = "=0.19.2" +dprint-plugin-json = "=0.19.3" dprint-plugin-jupyter = "=0.1.3" -dprint-plugin-markdown = "=0.17.0" -dprint-plugin-typescript = "=0.90.5" +dprint-plugin-markdown = "=0.17.1" +dprint-plugin-typescript = "=0.91.1" env_logger = "=0.10.0" fancy-regex = "=0.10.0" faster-hex.workspace = true diff --git a/cli/args/mod.rs b/cli/args/mod.rs index 766ddf52dd..6b2d361298 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -197,7 +197,7 @@ pub fn ts_config_to_transpile_and_emit_options( }, deno_ast::EmitOptions { inline_sources: options.inline_sources, - keep_comments: true, + remove_comments: false, source_map, source_map_file: None, }, diff --git a/cli/cache/emit.rs b/cli/cache/emit.rs index ac7c09a9ad..1d903fbba6 100644 --- a/cli/cache/emit.rs +++ b/cli/cache/emit.rs @@ -45,7 +45,7 @@ impl EmitCache { &self, specifier: &ModuleSpecifier, expected_source_hash: u64, - ) -> Option { + ) -> Option> { let meta_filename = self.get_meta_filename(specifier)?; let emit_filename = self.get_emit_filename(specifier)?; @@ -63,8 +63,7 @@ impl EmitCache { } // everything looks good, return it - let emit_text = String::from_utf8(emit_bytes).ok()?; - Some(emit_text) + Some(emit_bytes) } /// Gets the filepath which stores the emit. @@ -85,7 +84,7 @@ impl EmitCache { &self, specifier: &ModuleSpecifier, source_hash: u64, - code: &str, + code: &[u8], ) { if let Err(err) = self.set_emit_code_result(specifier, source_hash, code) { // should never error here, but if it ever does don't fail @@ -101,7 +100,7 @@ impl EmitCache { &self, specifier: &ModuleSpecifier, source_hash: u64, - code: &str, + code: &[u8], ) -> Result<(), AnyError> { let meta_filename = self .get_meta_filename(specifier) @@ -113,14 +112,14 @@ impl EmitCache { // save the metadata let metadata = EmitMetadata { source_hash, - emit_hash: compute_emit_hash(code.as_bytes(), self.cli_version), + emit_hash: compute_emit_hash(code, self.cli_version), }; self .disk_cache .set(&meta_filename, &serde_json::to_vec(&metadata)?)?; // save the emit source - self.disk_cache.set(&emit_filename, code.as_bytes())?; + self.disk_cache.set(&emit_filename, code)?; Ok(()) } @@ -163,6 +162,8 @@ mod test { disk_cache: disk_cache.clone(), cli_version: "1.0.0", }; + let to_string = + |bytes: Vec| -> String { String::from_utf8(bytes).unwrap() }; let specifier1 = ModuleSpecifier::from_file_path(temp_dir.path().join("file1.ts")) @@ -173,16 +174,19 @@ mod test { assert_eq!(cache.get_emit_code(&specifier1, 1), None); let emit_code1 = "text1".to_string(); let emit_code2 = "text2".to_string(); - cache.set_emit_code(&specifier1, 10, &emit_code1); - cache.set_emit_code(&specifier2, 2, &emit_code2); + cache.set_emit_code(&specifier1, 10, emit_code1.as_bytes()); + cache.set_emit_code(&specifier2, 2, emit_code2.as_bytes()); // providing the incorrect source hash assert_eq!(cache.get_emit_code(&specifier1, 5), None); // providing the correct source hash assert_eq!( - cache.get_emit_code(&specifier1, 10), + cache.get_emit_code(&specifier1, 10).map(to_string), Some(emit_code1.clone()), ); - assert_eq!(cache.get_emit_code(&specifier2, 2), Some(emit_code2)); + assert_eq!( + cache.get_emit_code(&specifier2, 2).map(to_string), + Some(emit_code2) + ); // try changing the cli version (should not load previous ones) let cache = EmitCache { @@ -190,19 +194,25 @@ mod test { cli_version: "2.0.0", }; assert_eq!(cache.get_emit_code(&specifier1, 10), None); - cache.set_emit_code(&specifier1, 5, &emit_code1); + cache.set_emit_code(&specifier1, 5, emit_code1.as_bytes()); // recreating the cache should still load the data because the CLI version is the same let cache = EmitCache { disk_cache, cli_version: "2.0.0", }; - assert_eq!(cache.get_emit_code(&specifier1, 5), Some(emit_code1)); + assert_eq!( + cache.get_emit_code(&specifier1, 5).map(to_string), + Some(emit_code1) + ); // adding when already exists should not cause issue let emit_code3 = "asdf".to_string(); - cache.set_emit_code(&specifier1, 20, &emit_code3); + cache.set_emit_code(&specifier1, 20, emit_code3.as_bytes()); assert_eq!(cache.get_emit_code(&specifier1, 5), None); - assert_eq!(cache.get_emit_code(&specifier1, 20), Some(emit_code3)); + assert_eq!( + cache.get_emit_code(&specifier1, 20).map(to_string), + Some(emit_code3) + ); } } diff --git a/cli/cache/parsed_source.rs b/cli/cache/parsed_source.rs index 688f2b9fb6..e956361f46 100644 --- a/cli/cache/parsed_source.rs +++ b/cli/cache/parsed_source.rs @@ -75,7 +75,7 @@ impl ParsedSourceCache { ) -> Result { if let Some(parsed_source) = self.remove_parsed_source(specifier) { if parsed_source.media_type() == media_type - && parsed_source.text_info().text_str() == source.as_ref() + && parsed_source.text().as_ref() == source.as_ref() { // note: message used tests log::debug!("Removed parsed source: {}", specifier); diff --git a/cli/emit.rs b/cli/emit.rs index fc815b6ee5..98ee59fab9 100644 --- a/cli/emit.rs +++ b/cli/emit.rs @@ -10,7 +10,7 @@ use deno_core::error::AnyError; use deno_core::futures::stream::FuturesUnordered; use deno_core::futures::FutureExt; use deno_core::futures::StreamExt; -use deno_core::ModuleCodeString; +use deno_core::ModuleCodeBytes; use deno_core::ModuleSpecifier; use deno_graph::MediaType; use deno_graph::Module; @@ -90,7 +90,7 @@ impl Emitter { &self, specifier: &ModuleSpecifier, source: &str, - ) -> Option { + ) -> Option> { let source_hash = self.get_source_hash(source); self.emit_cache.get_emit_code(specifier, source_hash) } @@ -100,7 +100,7 @@ impl Emitter { specifier: &ModuleSpecifier, media_type: MediaType, source: &Arc, - ) -> Result { + ) -> Result { // Note: keep this in sync with the sync version below let helper = EmitParsedSourceHelper(self); match helper.pre_emit_parsed_source(specifier, source) { @@ -139,7 +139,7 @@ impl Emitter { specifier: &ModuleSpecifier, media_type: MediaType, source: &Arc, - ) -> Result { + ) -> Result { // Note: keep this in sync with the async version above let helper = EmitParsedSourceHelper(self); match helper.pre_emit_parsed_source(specifier, source) { @@ -172,16 +172,43 @@ impl Emitter { ModuleSpecifier::to_file_path(specifier).unwrap(), ) .await?; - let source_arc: Arc = source_code.into(); - let parsed_source = self - .parsed_source_cache - .remove_or_parse_module(specifier, source_arc, media_type)?; - let mut options = self.transpile_and_emit_options.1.clone(); - options.source_map = SourceMapOption::None; - let transpiled_source = parsed_source - .transpile(&self.transpile_and_emit_options.0, &options)? - .into_source(); - Ok(transpiled_source.text) + match media_type { + MediaType::TypeScript + | MediaType::Mts + | MediaType::Cts + | MediaType::Jsx + | MediaType::Tsx => { + let source_arc: Arc = source_code.into(); + let parsed_source = self + .parsed_source_cache + .remove_or_parse_module(specifier, source_arc, media_type)?; + // HMR doesn't work with embedded source maps for some reason, so set + // the option to not use them (though you should test this out because + // this statement is probably wrong) + let mut options = self.transpile_and_emit_options.1.clone(); + options.source_map = SourceMapOption::None; + let transpiled_source = parsed_source + .transpile(&self.transpile_and_emit_options.0, &options)? + .into_source() + .into_string()?; + Ok(transpiled_source.text) + } + MediaType::JavaScript + | MediaType::Mjs + | MediaType::Cjs + | MediaType::Dts + | MediaType::Dmts + | MediaType::Dcts + | MediaType::Json + | MediaType::Wasm + | MediaType::TsBuildInfo + | MediaType::SourceMap + | MediaType::Unknown => { + // clear this specifier from the parsed source cache as it's now out of date + self.parsed_source_cache.free(specifier); + Ok(source_code) + } + } } /// A hashing function that takes the source code and uses the global emit @@ -196,7 +223,7 @@ impl Emitter { } enum PreEmitResult { - Cached(ModuleCodeString), + Cached(ModuleCodeBytes), NotCached { source_hash: u64 }, } @@ -214,7 +241,7 @@ impl<'a> EmitParsedSourceHelper<'a> { if let Some(emit_code) = self.0.emit_cache.get_emit_code(specifier, source_hash) { - PreEmitResult::Cached(emit_code.into()) + PreEmitResult::Cached(emit_code.into_boxed_slice().into()) } else { PreEmitResult::NotCached { source_hash } } @@ -240,7 +267,7 @@ impl<'a> EmitParsedSourceHelper<'a> { specifier: &ModuleSpecifier, transpile_result: TranspileResult, source_hash: u64, - ) -> ModuleCodeString { + ) -> ModuleCodeBytes { let transpiled_source = match transpile_result { TranspileResult::Owned(source) => source, TranspileResult::Cloned(source) => { @@ -252,8 +279,8 @@ impl<'a> EmitParsedSourceHelper<'a> { self.0.emit_cache.set_emit_code( specifier, source_hash, - &transpiled_source.text, + &transpiled_source.source, ); - transpiled_source.text.into() + transpiled_source.source.into_boxed_slice().into() } } diff --git a/cli/graph_util.rs b/cli/graph_util.rs index b7e3240b53..5e587b0d41 100644 --- a/cli/graph_util.rs +++ b/cli/graph_util.rs @@ -78,7 +78,7 @@ pub fn graph_valid( let mut errors = graph .walk( - roots, + roots.iter(), deno_graph::WalkOptions { check_js: options.check_js, follow_type_only: options.follow_type_only, @@ -479,7 +479,7 @@ impl ModuleGraphBuilder { }; let cli_resolver = &self.resolver; let graph_resolver = cli_resolver.as_graph_resolver(); - let graph_npm_resolver = cli_resolver.as_graph_npm_resolver(); + let graph_npm_resolver = cli_resolver.create_graph_npm_resolver(); let maybe_file_watcher_reporter = self .maybe_file_watcher_reporter .as_ref() @@ -503,7 +503,7 @@ impl ModuleGraphBuilder { executor: Default::default(), file_system: &DenoGraphFsAdapter(self.fs.as_ref()), jsr_url_provider: &CliJsrUrlProvider, - npm_resolver: Some(graph_npm_resolver), + npm_resolver: Some(&graph_npm_resolver), module_analyzer: &analyzer, reporter: maybe_file_watcher_reporter, resolver: Some(graph_resolver), @@ -595,7 +595,6 @@ impl ModuleGraphBuilder { if has_redirects_changed || has_jsr_package_deps_changed || has_jsr_package_mappings_changed - || has_npm_packages_changed { if let Some(lockfile) = &self.lockfile { let mut lockfile = lockfile.lock(); @@ -624,10 +623,6 @@ impl ModuleGraphBuilder { .add_package_deps(&name.to_string(), deps.map(|s| s.to_string())); } } - // npm packages - if has_npm_packages_changed { - self.npm_resolver.as_managed().unwrap().lock(&mut lockfile); - } } } @@ -652,7 +647,7 @@ impl ModuleGraphBuilder { let parser = self.parsed_source_cache.as_capturing_parser(); let cli_resolver = &self.resolver; let graph_resolver = cli_resolver.as_graph_resolver(); - let graph_npm_resolver = cli_resolver.as_graph_npm_resolver(); + let graph_npm_resolver = cli_resolver.create_graph_npm_resolver(); let workspace_members = if options.workspace_fast_check { Some(self.options.resolve_deno_graph_workspace_members()?) } else { @@ -666,7 +661,7 @@ impl ModuleGraphBuilder { fast_check_dts: false, module_parser: Some(&parser), resolver: Some(graph_resolver), - npm_resolver: Some(graph_npm_resolver), + npm_resolver: Some(&graph_npm_resolver), workspace_fast_check: if let Some(members) = &workspace_members { deno_graph::WorkspaceFastCheckOption::Enabled(members) } else { @@ -701,7 +696,10 @@ impl ModuleGraphBuilder { /// 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) + self.graph_roots_valid( + graph, + &graph.roots.iter().cloned().collect::>(), + ) } pub fn graph_roots_valid( @@ -891,9 +889,8 @@ pub fn has_graph_root_local_dependent_changed( root: &ModuleSpecifier, canonicalized_changed_paths: &HashSet, ) -> bool { - let roots = vec![root.clone()]; let mut dependent_specifiers = graph.walk( - &roots, + std::iter::once(root), deno_graph::WalkOptions { follow_dynamic: true, follow_type_only: true, diff --git a/cli/lsp/analysis.rs b/cli/lsp/analysis.rs index f60f2e0444..62feeb602e 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, - maybe_parsed_source: Option, + 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, - maybe_parsed_source: Option, + 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 d7e9e70527..c7e0c59bbd 100644 --- a/cli/lsp/code_lens.rs +++ b/cli/lsp/code_lens.rs @@ -67,7 +67,7 @@ impl DenoTestCollector { fn add_code_lenses>(&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, 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 4e0e4b46bd..f02ba64094 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 9e54c8106b..6504c38fed 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 7cf839a699..c7323d0c85 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -155,7 +155,7 @@ impl AssetOrDocument { pub fn text(&self) -> Arc { 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> { + ) -> Option<&Result> { 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, resolver: Arc, specifier: ModuleSpecifier, - text_info: SourceTextInfo, + text: Arc, + text_info_cell: once_cell::sync::OnceCell, } impl Document { @@ -291,7 +292,7 @@ impl Document { #[allow(clippy::too_many_arguments)] fn new( specifier: ModuleSpecifier, - content: Arc, + text: Arc, maybe_lsp_version: Option, maybe_language_id: Option, maybe_headers: Option>, @@ -300,7 +301,6 @@ impl Document { cache: &Arc, file_referrer: Option, ) -> Arc { - 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, ) -> Result, 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 = 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 { - self.text_info.text() + pub fn content(&self) -> &Arc { + &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 { @@ -647,8 +659,8 @@ impl Document { pub fn maybe_parsed_source( &self, - ) -> Option> { - self.open_data.as_ref()?.maybe_parsed_source.clone() + ) -> Option<&Result> { + self.open_data.as_ref()?.maybe_parsed_source.as_ref() } pub async fn maybe_test_module(&self) -> Option> { @@ -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, maybe_headers: Option<&HashMap>, media_type: MediaType, file_referrer: Option<&ModuleSpecifier>, resolver: &LspResolver, ) -> (Option, Option) { - 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, 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 e362a9e7e5..466c5b430c 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 599db4876f..2b465f695b 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 508e50f9b3..7b00228dfd 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() diff --git a/cli/module_loader.rs b/cli/module_loader.rs index e5f45ea3c3..1d5fdd85f6 100644 --- a/cli/module_loader.rs +++ b/cli/module_loader.rs @@ -371,8 +371,7 @@ impl // it to work with --inspect or --inspect-brk code_source.code } else { - // reduce memory and throw away the source map - // because we don't need it + // v8 is slower when source maps are present, so we strip them code_without_source_map(code_source.code) }; let module_type = match code_source.media_type { @@ -413,7 +412,7 @@ impl Ok(ModuleSource::new_with_redirect( module_type, - ModuleSourceCode::String(code), + code, specifier, &code_source.found_url, code_cache, @@ -424,26 +423,6 @@ impl &self, referrer: &str, ) -> Result { - // todo(https://github.com/denoland/deno_core/pull/741): use function from deno_core - fn specifier_has_uri_scheme(specifier: &str) -> bool { - let mut chars = specifier.chars(); - let mut len = 0usize; - // The first character must be a letter. - match chars.next() { - Some(c) if c.is_ascii_alphabetic() => len += 1, - _ => return false, - } - // Second and following characters must be either a letter, number, - // plus sign, minus sign, or dot. - loop { - match chars.next() { - Some(c) if c.is_ascii_alphanumeric() || "+-.".contains(c) => len += 1, - Some(':') if len >= 2 => return true, - _ => return false, - } - } - } - let referrer = if referrer.is_empty() && self.shared.is_repl { // FIXME(bartlomieju): this is a hacky way to provide compatibility with REPL // and `Deno.core.evalContext` API. Ideally we should always have a referrer filled @@ -452,7 +431,7 @@ impl referrer }; - if specifier_has_uri_scheme(referrer) { + if deno_core::specifier_has_uri_scheme(referrer) { deno_core::resolve_url(referrer).map_err(|e| e.into()) } else if referrer == "." { // main module, use the initial cwd @@ -612,7 +591,7 @@ impl self.parsed_source_cache.free(specifier); Ok(ModuleCodeStringSource { - code: transpile_result, + code: ModuleSourceCode::Bytes(transpile_result), found_url: specifier.clone(), media_type, }) @@ -647,7 +626,7 @@ impl self.parsed_source_cache.free(specifier); Ok(ModuleCodeStringSource { - code: transpile_result, + code: ModuleSourceCode::Bytes(transpile_result), found_url: specifier.clone(), media_type, }) @@ -673,7 +652,7 @@ impl specifier, .. })) => Ok(CodeOrDeferredEmit::Code(ModuleCodeStringSource { - code: source.clone().into(), + code: ModuleSourceCode::String(source.clone().into()), found_url: specifier.clone(), media_type: *media_type, })), @@ -712,7 +691,7 @@ impl self.parsed_source_cache.free(specifier); Ok(CodeOrDeferredEmit::Code(ModuleCodeStringSource { - code, + code: ModuleSourceCode::String(code), found_url: specifier.clone(), media_type: *media_type, })) @@ -892,7 +871,7 @@ impl SourceMapGetter _ => return None, } let source = self.0.load_prepared_module_sync(&specifier, None).ok()?; - source_map_from_code(&source.code) + source_map_from_code(source.code.as_bytes()) } fn get_source_line( diff --git a/cli/node.rs b/cli/node.rs index 4c285e3f46..c696fcac93 100644 --- a/cli/node.rs +++ b/cli/node.rs @@ -85,7 +85,7 @@ impl CliCjsCodeAnalyzer { move || -> Result<_, deno_ast::ParseDiagnostic> { let parsed_source = deno_ast::parse_program(deno_ast::ParseParams { specifier, - text_info: deno_ast::SourceTextInfo::new(source), + text: source, media_type, capture_tokens: true, scope_analysis: false, diff --git a/cli/npm/managed/cache/mod.rs b/cli/npm/managed/cache/mod.rs index f409744b98..531fd09881 100644 --- a/cli/npm/managed/cache/mod.rs +++ b/cli/npm/managed/cache/mod.rs @@ -28,6 +28,7 @@ use crate::util::fs::hard_link_dir_recursive; mod registry_info; mod tarball; mod tarball_extract; +mod value_creator; pub use registry_info::RegistryInfoDownloader; pub use tarball::TarballCache; diff --git a/cli/npm/managed/cache/registry_info.rs b/cli/npm/managed/cache/registry_info.rs index 24f0a12e70..131b931926 100644 --- a/cli/npm/managed/cache/registry_info.rs +++ b/cli/npm/managed/cache/registry_info.rs @@ -8,8 +8,7 @@ use deno_core::anyhow::bail; use deno_core::anyhow::Context; use deno_core::error::custom_error; use deno_core::error::AnyError; -use deno_core::futures::future::BoxFuture; -use deno_core::futures::future::Shared; +use deno_core::futures::future::LocalBoxFuture; use deno_core::futures::FutureExt; use deno_core::parking_lot::Mutex; use deno_core::serde_json; @@ -23,6 +22,7 @@ use crate::http_util::HttpClientProvider; use crate::npm::common::maybe_auth_header_for_npm_registry; use crate::util::progress_bar::ProgressBar; +use super::value_creator::MultiRuntimeAsyncValueCreator; use super::NpmCache; // todo(dsherret): create seams and unit test this @@ -30,7 +30,7 @@ use super::NpmCache; #[derive(Debug, Clone)] enum MemoryCacheItem { /// The cache item hasn't loaded yet. - PendingFuture(Shared), + Pending(Arc>), /// The item has loaded in the past and was stored in the file system cache. /// There is no reason to request this package from the npm registry again /// for the duration of execution. @@ -48,7 +48,7 @@ enum FutureResult { } type PendingRegistryLoadFuture = - BoxFuture<'static, Result>>; + LocalBoxFuture<'static, Result>; /// Downloads packuments from the npm registry. /// @@ -79,7 +79,7 @@ impl RegistryInfoDownloader { } pub async fn load_package_info( - &self, + self: &Arc, name: &str, ) -> Result>, AnyError> { let registry_url = self.npmrc.get_registry_url(name); @@ -98,7 +98,7 @@ impl RegistryInfoDownloader { } async fn load_package_info_inner( - &self, + self: &Arc, name: &str, registry_url: &Url, registry_config: &RegistryConfig, @@ -112,18 +112,20 @@ impl RegistryInfoDownloader { )); } - let (created, cache_item) = { + let cache_item = { let mut mem_cache = self.memory_cache.lock(); if let Some(cache_item) = mem_cache.get(name) { - (false, cache_item.clone()) + cache_item.clone() } else { let future = self.create_load_future(name, registry_url, registry_config); - let cache_item = MemoryCacheItem::PendingFuture(future); + let value_creator = MultiRuntimeAsyncValueCreator::new(future); + let cache_item = MemoryCacheItem::Pending(Arc::new(value_creator)); mem_cache.insert(name.to_string(), cache_item.clone()); - (true, cache_item) + cache_item } }; + match cache_item { MemoryCacheItem::FsCached => { // this struct previously loaded from the registry, so we can load it from the file system cache @@ -135,40 +137,35 @@ impl RegistryInfoDownloader { MemoryCacheItem::MemoryCached(maybe_info) => { maybe_info.clone().map_err(|e| anyhow!("{}", e)) } - MemoryCacheItem::PendingFuture(future) => { - if created { - match future.await { - Ok(FutureResult::SavedFsCache(info)) => { - // return back the future and mark this package as having - // been saved in the cache for next time it's requested - *self.memory_cache.lock().get_mut(name).unwrap() = - MemoryCacheItem::FsCached; - Ok(Some(info)) - } - Ok(FutureResult::ErroredFsCache(info)) => { - // since saving to the fs cache failed, keep the package information in memory - *self.memory_cache.lock().get_mut(name).unwrap() = - MemoryCacheItem::MemoryCached(Ok(Some(info.clone()))); - Ok(Some(info)) - } - Ok(FutureResult::PackageNotExists) => { - *self.memory_cache.lock().get_mut(name).unwrap() = - MemoryCacheItem::MemoryCached(Ok(None)); - Ok(None) - } - Err(err) => { - let return_err = anyhow!("{}", err); - *self.memory_cache.lock().get_mut(name).unwrap() = - MemoryCacheItem::MemoryCached(Err(err)); - Err(return_err) - } + MemoryCacheItem::Pending(value_creator) => { + let downloader = self.clone(); + let future = value_creator.get(move || { + downloader.create_load_future(name, registry_url, registry_config) + }); + match future.await { + Ok(FutureResult::SavedFsCache(info)) => { + // return back the future and mark this package as having + // been saved in the cache for next time it's requested + *self.memory_cache.lock().get_mut(name).unwrap() = + MemoryCacheItem::FsCached; + Ok(Some(info)) } - } else { - match future.await { - Ok(FutureResult::SavedFsCache(info)) => Ok(Some(info)), - Ok(FutureResult::ErroredFsCache(info)) => Ok(Some(info)), - Ok(FutureResult::PackageNotExists) => Ok(None), - Err(err) => Err(anyhow!("{}", err)), + Ok(FutureResult::ErroredFsCache(info)) => { + // since saving to the fs cache failed, keep the package information in memory + *self.memory_cache.lock().get_mut(name).unwrap() = + MemoryCacheItem::MemoryCached(Ok(Some(info.clone()))); + Ok(Some(info)) + } + Ok(FutureResult::PackageNotExists) => { + *self.memory_cache.lock().get_mut(name).unwrap() = + MemoryCacheItem::MemoryCached(Ok(None)); + Ok(None) + } + Err(err) => { + let return_err = anyhow!("{}", err); + *self.memory_cache.lock().get_mut(name).unwrap() = + MemoryCacheItem::MemoryCached(Err(err)); + Err(return_err) } } } @@ -203,23 +200,19 @@ impl RegistryInfoDownloader { } fn create_load_future( - &self, + self: &Arc, name: &str, registry_url: &Url, registry_config: &RegistryConfig, - ) -> Shared { + ) -> PendingRegistryLoadFuture { + let downloader = self.clone(); let package_url = self.get_package_url(name, registry_url); let maybe_auth_header = maybe_auth_header_for_npm_registry(registry_config); let guard = self.progress_bar.update(package_url.as_str()); - let cache = self.cache.clone(); - let http_client_provider = self.http_client_provider.clone(); let name = name.to_string(); - // force this future to be polled on the current runtime because it's not - // safe to share `HttpClient`s across runtimes and because a restart of - // npm resolution might cause this package not to be resolved again - // causing the future to never be polled - deno_core::unsync::spawn(async move { - let maybe_bytes = http_client_provider + async move { + let maybe_bytes = downloader + .http_client_provider .get_or_create()? .download_with_progress(package_url, maybe_auth_header, &guard) .await?; @@ -228,7 +221,7 @@ impl RegistryInfoDownloader { let future_result = deno_core::unsync::spawn_blocking( move || -> Result { let package_info = serde_json::from_slice(&bytes)?; - match cache.save_package_info(&name, &package_info) { + match downloader.cache.save_package_info(&name, &package_info) { Ok(()) => { Ok(FutureResult::SavedFsCache(Arc::new(package_info))) } @@ -248,10 +241,8 @@ impl RegistryInfoDownloader { } None => Ok(FutureResult::PackageNotExists), } - }) - .map(|result| result.unwrap().map_err(Arc::new)) - .boxed() - .shared() + } + .boxed_local() } fn get_package_url(&self, name: &str, registry_url: &Url) -> Url { diff --git a/cli/npm/managed/cache/tarball.rs b/cli/npm/managed/cache/tarball.rs index a116ad1cf2..e65578ff33 100644 --- a/cli/npm/managed/cache/tarball.rs +++ b/cli/npm/managed/cache/tarball.rs @@ -8,8 +8,7 @@ use deno_core::anyhow::bail; use deno_core::anyhow::Context; use deno_core::error::custom_error; use deno_core::error::AnyError; -use deno_core::futures::future::BoxFuture; -use deno_core::futures::future::Shared; +use deno_core::futures::future::LocalBoxFuture; use deno_core::futures::FutureExt; use deno_core::parking_lot::Mutex; use deno_npm::npm_rc::ResolvedNpmRc; @@ -24,6 +23,7 @@ use crate::util::progress_bar::ProgressBar; use super::tarball_extract::verify_and_extract_tarball; use super::tarball_extract::TarballExtractionMode; +use super::value_creator::MultiRuntimeAsyncValueCreator; use super::NpmCache; // todo(dsherret): create seams and unit test this @@ -31,7 +31,7 @@ use super::NpmCache; #[derive(Debug, Clone)] enum MemoryCacheItem { /// The cache item hasn't finished yet. - PendingFuture(Shared>>>), + Pending(Arc>), /// The result errored. Errored(Arc), /// This package has already been cached. @@ -71,7 +71,7 @@ impl TarballCache { } pub async fn ensure_package( - &self, + self: &Arc, package: &PackageNv, dist: &NpmPackageVersionDistInfo, ) -> Result<(), AnyError> { @@ -82,69 +82,67 @@ impl TarballCache { } async fn ensure_package_inner( - &self, + self: &Arc, package_nv: &PackageNv, dist: &NpmPackageVersionDistInfo, ) -> Result<(), AnyError> { - let (created, cache_item) = { + let cache_item = { let mut mem_cache = self.memory_cache.lock(); if let Some(cache_item) = mem_cache.get(package_nv) { - (false, cache_item.clone()) + cache_item.clone() } else { let future = self.create_setup_future(package_nv.clone(), dist.clone()); - let cache_item = MemoryCacheItem::PendingFuture(future); + let value_creator = MultiRuntimeAsyncValueCreator::new(future); + let cache_item = MemoryCacheItem::Pending(Arc::new(value_creator)); mem_cache.insert(package_nv.clone(), cache_item.clone()); - (true, cache_item) + cache_item } }; match cache_item { MemoryCacheItem::Cached => Ok(()), MemoryCacheItem::Errored(err) => Err(anyhow!("{}", err)), - MemoryCacheItem::PendingFuture(future) => { - if created { - match future.await { - Ok(_) => { - *self.memory_cache.lock().get_mut(package_nv).unwrap() = - MemoryCacheItem::Cached; - Ok(()) - } - Err(err) => { - let result_err = anyhow!("{}", err); - *self.memory_cache.lock().get_mut(package_nv).unwrap() = - MemoryCacheItem::Errored(err); - Err(result_err) - } + MemoryCacheItem::Pending(creator) => { + let tarball_cache = self.clone(); + let result = creator + .get(move || { + tarball_cache.create_setup_future(package_nv.clone(), dist.clone()) + }) + .await; + match result { + Ok(_) => { + *self.memory_cache.lock().get_mut(package_nv).unwrap() = + MemoryCacheItem::Cached; + Ok(()) + } + Err(err) => { + let result_err = anyhow!("{}", err); + *self.memory_cache.lock().get_mut(package_nv).unwrap() = + MemoryCacheItem::Errored(err); + Err(result_err) } - } else { - future.await.map_err(|err| anyhow!("{}", err)) } } } } fn create_setup_future( - &self, + self: &Arc, package_nv: PackageNv, dist: NpmPackageVersionDistInfo, - ) -> Shared>>> { - let registry_url = self.npmrc.get_registry_url(&package_nv.name); - let registry_config = - self.npmrc.get_registry_config(&package_nv.name).clone(); - - let cache = self.cache.clone(); - let fs = self.fs.clone(); - let progress_bar = self.progress_bar.clone(); - let package_folder = - cache.package_folder_for_nv_and_url(&package_nv, registry_url); - let http_client_provider = self.http_client_provider.clone(); - - deno_core::unsync::spawn(async move { - let should_use_cache = cache.should_use_cache_for_package(&package_nv); - let package_folder_exists = fs.exists_sync(&package_folder); + ) -> LocalBoxFuture<'static, Result<(), AnyError>> { + let tarball_cache = self.clone(); + async move { + let registry_url = tarball_cache.npmrc.get_registry_url(&package_nv.name); + let registry_config = + tarball_cache.npmrc.get_registry_config(&package_nv.name).clone(); + let package_folder = + tarball_cache.cache.package_folder_for_nv_and_url(&package_nv, registry_url); + let should_use_cache = tarball_cache.cache.should_use_cache_for_package(&package_nv); + let package_folder_exists = tarball_cache.fs.exists_sync(&package_folder); if should_use_cache && package_folder_exists { return Ok(()); - } else if cache.cache_setting() == &CacheSetting::Only { + } else if tarball_cache.cache.cache_setting() == &CacheSetting::Only { return Err(custom_error( "NotCached", format!( @@ -162,8 +160,9 @@ impl TarballCache { let maybe_auth_header = maybe_auth_header_for_npm_registry(®istry_config); - let guard = progress_bar.update(&dist.tarball); - let maybe_bytes = http_client_provider.get_or_create()? + let guard = tarball_cache.progress_bar.update(&dist.tarball); + let maybe_bytes = tarball_cache.http_client_provider + .get_or_create()? .download_with_progress(&dist.tarball, maybe_auth_header, &guard) .await?; match maybe_bytes { @@ -198,9 +197,6 @@ impl TarballCache { bail!("Could not find npm package tarball at: {}", dist.tarball); } } - }) - .map(|result| result.unwrap().map_err(Arc::new)) - .boxed() - .shared() + }.boxed_local() } } diff --git a/cli/npm/managed/cache/value_creator.rs b/cli/npm/managed/cache/value_creator.rs new file mode 100644 index 0000000000..38801b3aa3 --- /dev/null +++ b/cli/npm/managed/cache/value_creator.rs @@ -0,0 +1,101 @@ +// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. + +use std::sync::Arc; + +use deno_core::error::AnyError; +use deno_core::futures::future::BoxFuture; +use deno_core::futures::future::LocalBoxFuture; +use deno_core::futures::future::Shared; +use deno_core::futures::FutureExt; +use deno_core::parking_lot::Mutex; +use tokio::task::JoinError; + +// todo(dsherret): unit test this + +type FutureResult = Result>; +type JoinResult = Result, Arc>; + +#[derive(Debug)] +struct State { + retry_index: usize, + future: Shared>>, +} + +/// Attempts to create a shared value asynchronously on one tokio runtime while +/// many runtimes are requesting the value. +/// +/// This is only useful when the value needs to get created once across +/// many runtimes. +/// +/// This handles the case where one tokio runtime goes down while another +/// one is still running. +#[derive(Debug)] +pub struct MultiRuntimeAsyncValueCreator { + state: Mutex>, +} + +impl MultiRuntimeAsyncValueCreator { + pub fn new( + future: LocalBoxFuture<'static, Result>, + ) -> Self { + Self { + state: Mutex::new(State { + retry_index: 0, + future: Self::create_shared_future(future), + }), + } + } + + pub async fn get( + &self, + recreate_future: impl Fn() -> LocalBoxFuture<'static, Result>, + ) -> Result> { + let (mut future, mut retry_index) = { + let state = self.state.lock(); + (state.future.clone(), state.retry_index) + }; + + loop { + let result = future.await; + + match result { + Ok(result) => return result, + Err(join_error) => { + if join_error.is_cancelled() { + let mut state = self.state.lock(); + + if state.retry_index == retry_index { + // we were the first one to retry, so create a new future + // that we'll run from the current runtime + state.retry_index += 1; + state.future = Self::create_shared_future(recreate_future()); + } + + retry_index = state.retry_index; + future = state.future.clone(); + + // just in case we're stuck in a loop + if retry_index > 1000 { + panic!("Something went wrong.") // should never happen + } + } else { + panic!("{}", join_error); + } + } + } + } + } + + fn create_shared_future( + future: LocalBoxFuture<'static, Result>, + ) -> Shared>> { + deno_core::unsync::spawn(future) + .map(|result| match result { + Ok(Ok(value)) => Ok(Ok(value)), + Ok(Err(err)) => Ok(Err(Arc::new(err))), + Err(err) => Err(Arc::new(err)), + }) + .boxed() + .shared() + } +} diff --git a/cli/npm/managed/installer.rs b/cli/npm/managed/installer.rs index f762be70ec..694e01206f 100644 --- a/cli/npm/managed/installer.rs +++ b/cli/npm/managed/installer.rs @@ -105,7 +105,8 @@ impl PackageJsonDepsInstaller { let (req, info) = result?; let result = inner .npm_resolution - .resolve_pkg_req_as_pending_with_info(req, &info); + .resolve_pkg_req_as_pending_with_info(req, &info) + .await; if let Err(err) = result { if inner.npm_registry_api.mark_force_reload() { log::debug!("Failed to resolve package. Retrying. Error: {err:#}"); diff --git a/cli/npm/managed/mod.rs b/cli/npm/managed/mod.rs index 7c20ceedcb..f139a6f4b0 100644 --- a/cli/npm/managed/mod.rs +++ b/cli/npm/managed/mod.rs @@ -11,8 +11,9 @@ use deno_core::anyhow::Context; use deno_core::error::AnyError; use deno_core::parking_lot::Mutex; use deno_core::serde_json; -use deno_graph::NpmPackageReqResolution; +use deno_graph::NpmPackageReqsResolution; use deno_npm::npm_rc::ResolvedNpmRc; +use deno_npm::registry::NpmPackageInfo; use deno_npm::registry::NpmRegistryApi; use deno_npm::resolution::NpmResolutionSnapshot; use deno_npm::resolution::PackageReqNotFoundError; @@ -203,12 +204,12 @@ fn create_api( ) -> Arc { Arc::new(CliNpmRegistryApi::new( npm_cache.clone(), - RegistryInfoDownloader::new( + Arc::new(RegistryInfoDownloader::new( npm_cache, options.http_client_provider.clone(), options.npmrc.clone(), options.text_only_progress_bar.clone(), - ), + )), )) } @@ -387,11 +388,6 @@ impl ManagedCliNpmResolver { self.resolution.add_package_reqs(packages).await?; self.fs_resolver.cache_packages().await?; - // If there's a lock file, update it with all discovered npm packages - if let Some(lockfile) = &self.maybe_lockfile { - self.lock(&mut lockfile.lock()); - } - Ok(()) } @@ -418,10 +414,6 @@ impl ManagedCliNpmResolver { .serialized_valid_snapshot_for_system(system_info) } - pub fn lock(&self, lockfile: &mut Lockfile) { - self.resolution.lock(lockfile) - } - pub async fn inject_synthetic_types_node_package( &self, ) -> Result<(), AnyError> { @@ -442,25 +434,33 @@ impl ManagedCliNpmResolver { self.fs_resolver.cache_packages().await } - /// Resolves a package requirement for deno graph. This should only be - /// called by deno_graph's NpmResolver or for resolving packages in - /// a package.json - pub fn resolve_npm_for_deno_graph( + /// Resolves package requirements for deno graph. + pub async fn resolve_npm_for_deno_graph( &self, - pkg_req: &PackageReq, - ) -> NpmPackageReqResolution { - let result = self.resolution.resolve_pkg_req_as_pending(pkg_req); - match result { - Ok(nv) => NpmPackageReqResolution::Ok(nv), - Err(err) => { - if self.npm_api.mark_force_reload() { - log::debug!("Restarting npm specifier resolution to check for new registry information. Error: {:#}", err); - NpmPackageReqResolution::ReloadRegistryInfo(err.into()) - } else { - NpmPackageReqResolution::Err(err.into()) + reqs_with_pkg_infos: &[(&PackageReq, Arc)], + ) -> NpmPackageReqsResolution { + let results = self + .resolution + .resolve_pkg_reqs_as_pending_with_info(reqs_with_pkg_infos) + .await; + + let mut resolutions = Vec::with_capacity(results.len()); + for result in results { + match result { + Ok(nv) => { + resolutions.push(Ok(nv)); + } + Err(err) => { + if self.npm_api.mark_force_reload() { + log::debug!("Restarting npm specifier resolution to check for new registry information. Error: {:#}", err); + return NpmPackageReqsResolution::ReloadRegistryInfo; + } else { + resolutions.push(Err(Arc::new(err.into()))); + } } } } + NpmPackageReqsResolution::Resolutions(resolutions) } pub fn resolve_pkg_folder_from_deno_module( @@ -490,13 +490,12 @@ impl ManagedCliNpmResolver { pub async fn cache_package_info( &self, package_name: &str, - ) -> Result<(), AnyError> { + ) -> Result, AnyError> { // this will internally cache the package information self .npm_api .package_info(package_name) .await - .map(|_| ()) .map_err(|err| err.into()) } diff --git a/cli/npm/managed/registry.rs b/cli/npm/managed/registry.rs index 32161f2358..14c3bd38f3 100644 --- a/cli/npm/managed/registry.rs +++ b/cli/npm/managed/registry.rs @@ -21,14 +21,13 @@ use crate::util::sync::AtomicFlag; use super::cache::NpmCache; use super::cache::RegistryInfoDownloader; -// todo(dsherret): make this per worker #[derive(Debug)] pub struct CliNpmRegistryApi(Option>); impl CliNpmRegistryApi { pub fn new( cache: Arc, - registry_info_downloader: RegistryInfoDownloader, + registry_info_downloader: Arc, ) -> Self { Self(Some(Arc::new(CliNpmRegistryApiInner { cache, @@ -44,13 +43,6 @@ impl CliNpmRegistryApi { self.inner().clear_memory_cache(); } - pub fn get_cached_package_info( - &self, - name: &str, - ) -> Option> { - self.inner().get_cached_package_info(name) - } - fn inner(&self) -> &Arc { // this panicking indicates a bug in the code where this // wasn't initialized @@ -76,20 +68,7 @@ impl NpmRegistryApi for CliNpmRegistryApi { } fn mark_force_reload(&self) -> bool { - // never force reload the registry information if reloading - // is disabled or if we're already reloading - if matches!( - self.inner().cache.cache_setting(), - CacheSetting::Only | CacheSetting::ReloadAll - ) { - return false; - } - if self.inner().force_reload_flag.raise() { - self.clear_memory_cache(); // clear the cache to force reloading - true - } else { - false - } + self.inner().mark_force_reload() } } @@ -108,7 +87,7 @@ struct CliNpmRegistryApiInner { force_reload_flag: AtomicFlag, mem_cache: Mutex>, previously_reloaded_packages: Mutex>, - registry_info_downloader: RegistryInfoDownloader, + registry_info_downloader: Arc, } impl CliNpmRegistryApiInner { @@ -128,7 +107,7 @@ impl CliNpmRegistryApiInner { let api = self.clone(); let name = name.to_string(); async move { - if (api.cache.cache_setting().should_use_for_npm_package(&name) && !api.force_reload()) + if (api.cache.cache_setting().should_use_for_npm_package(&name) && !api.force_reload_flag.is_raised()) // if this has been previously reloaded, then try loading from the // file system cache || !api.previously_reloaded_packages.lock().insert(name.to_string()) @@ -175,8 +154,21 @@ impl CliNpmRegistryApiInner { } } - fn force_reload(&self) -> bool { - self.force_reload_flag.is_raised() + fn mark_force_reload(&self) -> bool { + // never force reload the registry information if reloading + // is disabled or if we're already reloading + if matches!( + self.cache.cache_setting(), + CacheSetting::Only | CacheSetting::ReloadAll + ) { + return false; + } + if self.force_reload_flag.raise() { + self.clear_memory_cache(); + true + } else { + false + } } async fn load_file_cached_package_info( @@ -205,16 +197,4 @@ impl CliNpmRegistryApiInner { fn clear_memory_cache(&self) { self.mem_cache.lock().clear(); } - - pub fn get_cached_package_info( - &self, - name: &str, - ) -> Option> { - let mem_cache = self.mem_cache.lock(); - if let Some(CacheItem::Resolved(maybe_info)) = mem_cache.get(name) { - maybe_info.clone() - } else { - None - } - } } diff --git a/cli/npm/managed/resolution.rs b/cli/npm/managed/resolution.rs index 9cea5d305e..3562d5aff4 100644 --- a/cli/npm/managed/resolution.rs +++ b/cli/npm/managed/resolution.rs @@ -223,34 +223,42 @@ impl NpmResolution { /// Resolves a package requirement for deno graph. This should only be /// called by deno_graph's NpmResolver or for resolving packages in /// a package.json - pub fn resolve_pkg_req_as_pending( + pub async fn resolve_pkg_req_as_pending_with_info( &self, pkg_req: &PackageReq, + pkg_info: &NpmPackageInfo, ) -> Result { - // we should always have this because it should have been cached before here - let package_info = self.api.get_cached_package_info(&pkg_req.name).unwrap(); - self.resolve_pkg_req_as_pending_with_info(pkg_req, &package_info) - } - - /// Resolves a package requirement for deno graph. This should only be - /// called by deno_graph's NpmResolver or for resolving packages in - /// a package.json - pub fn resolve_pkg_req_as_pending_with_info( - &self, - pkg_req: &PackageReq, - package_info: &NpmPackageInfo, - ) -> Result { - debug_assert_eq!(pkg_req.name, package_info.name); + debug_assert_eq!(pkg_req.name, pkg_info.name); + let _permit = self.update_queue.acquire().await; let mut snapshot = self.snapshot.write(); let pending_resolver = get_npm_pending_resolver(&self.api); let nv = pending_resolver.resolve_package_req_as_pending( &mut snapshot, pkg_req, - package_info, + pkg_info, )?; Ok(nv) } + pub async fn resolve_pkg_reqs_as_pending_with_info( + &self, + reqs_with_pkg_infos: &[(&PackageReq, Arc)], + ) -> Vec> { + let _permit = self.update_queue.acquire().await; + let mut snapshot = self.snapshot.write(); + let pending_resolver = get_npm_pending_resolver(&self.api); + let mut results = Vec::with_capacity(reqs_with_pkg_infos.len()); + for (pkg_req, pkg_info) in reqs_with_pkg_infos { + debug_assert_eq!(pkg_req.name, pkg_info.name); + results.push(pending_resolver.resolve_package_req_as_pending( + &mut snapshot, + pkg_req, + pkg_info, + )); + } + results + } + pub fn package_reqs(&self) -> HashMap { self.snapshot.read().package_reqs().clone() } @@ -291,11 +299,6 @@ impl NpmResolution { .read() .as_valid_serialized_for_system(system_info) } - - pub fn lock(&self, lockfile: &mut Lockfile) { - let snapshot = self.snapshot.read(); - populate_lockfile_from_snapshot(lockfile, &snapshot) - } } async fn add_package_reqs_to_snapshot( @@ -370,6 +373,7 @@ fn populate_lockfile_from_snapshot( lockfile: &mut Lockfile, snapshot: &NpmResolutionSnapshot, ) { + assert!(!snapshot.has_pending()); for (package_req, nv) in snapshot.package_reqs() { lockfile.insert_package_specifier( format!("npm:{}", package_req), diff --git a/cli/resolver.rs b/cli/resolver.rs index 8fbacd8f17..a28dfce91a 100644 --- a/cli/resolver.rs +++ b/cli/resolver.rs @@ -1,5 +1,6 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. +use async_trait::async_trait; use dashmap::DashMap; use dashmap::DashSet; use deno_ast::MediaType; @@ -9,15 +10,15 @@ use deno_core::error::AnyError; use deno_core::futures::future; use deno_core::futures::future::LocalBoxFuture; use deno_core::futures::FutureExt; -use deno_core::ModuleCodeString; +use deno_core::ModuleSourceCode; use deno_core::ModuleSpecifier; -use deno_graph::source::NpmPackageReqResolution; -use deno_graph::source::NpmResolver; use deno_graph::source::ResolutionMode; use deno_graph::source::ResolveError; use deno_graph::source::Resolver; use deno_graph::source::UnknownBuiltInNodeModuleError; use deno_graph::source::DEFAULT_JSX_IMPORT_SOURCE_MODULE; +use deno_graph::NpmPackageReqsResolution; +use deno_npm::registry::NpmPackageInfo; use deno_runtime::deno_fs; use deno_runtime::deno_fs::FileSystem; use deno_runtime::deno_node::is_builtin_node_module; @@ -34,6 +35,8 @@ use deno_semver::npm::NpmPackageReqReference; use deno_semver::package::PackageReq; use import_map::ImportMap; use std::borrow::Cow; +use std::cell::RefCell; +use std::collections::HashMap; use std::path::Path; use std::path::PathBuf; use std::rc::Rc; @@ -60,7 +63,7 @@ pub fn format_range_with_colors(range: &deno_graph::Range) -> String { } pub struct ModuleCodeStringSource { - pub code: ModuleCodeString, + pub code: ModuleSourceCode, pub found_url: ModuleSpecifier, pub media_type: MediaType, } @@ -293,7 +296,7 @@ impl NpmModuleLoader { let file_path = specifier.to_file_path().unwrap(); let code = self .fs - .read_text_file_async(file_path.clone(), None) + .read_file_async(file_path.clone(), None) .await .map_err(AnyError::from) .with_context(|| { @@ -329,16 +332,20 @@ impl NpmModuleLoader { let code = if self.cjs_resolutions.contains(specifier) { // translate cjs to esm if it's cjs and inject node globals - self - .node_code_translator - .translate_cjs_to_esm(specifier, Some(code), permissions) - .await? + let code = String::from_utf8(code)?; + ModuleSourceCode::String( + self + .node_code_translator + .translate_cjs_to_esm(specifier, Some(code), permissions) + .await? + .into(), + ) } else { // esm and json code is untouched - code + ModuleSourceCode::Bytes(code.into_boxed_slice().into()) }; Ok(ModuleCodeStringSource { - code: code.into(), + code, found_url: specifier.clone(), media_type: MediaType::from_specifier(specifier), }) @@ -498,8 +505,12 @@ impl CliGraphResolver { self } - pub fn as_graph_npm_resolver(&self) -> &dyn NpmResolver { - self + pub fn create_graph_npm_resolver(&self) -> WorkerCliNpmGraphResolver { + WorkerCliNpmGraphResolver { + npm_resolver: self.npm_resolver.as_ref(), + memory_cache: Default::default(), + bare_node_builtins_enabled: self.bare_node_builtins_enabled, + } } pub fn found_package_json_dep(&self) -> bool { @@ -782,7 +793,17 @@ fn resolve_package_json_dep( Ok(None) } -impl NpmResolver for CliGraphResolver { +#[derive(Debug)] +pub struct WorkerCliNpmGraphResolver<'a> { + npm_resolver: Option<&'a Arc>, + // use a cache per worker so that another worker doesn't clear the + // cache of another worker + memory_cache: Rc>>>, + bare_node_builtins_enabled: bool, +} + +#[async_trait(?Send)] +impl<'a> deno_graph::source::NpmResolver for WorkerCliNpmGraphResolver<'a> { fn resolve_builtin_node_module( &self, specifier: &ModuleSpecifier, @@ -818,17 +839,20 @@ impl NpmResolver for CliGraphResolver { &self, package_name: &str, ) -> LocalBoxFuture<'static, Result<(), AnyError>> { - match &self.npm_resolver { + match self.npm_resolver { Some(npm_resolver) if npm_resolver.as_managed().is_some() => { - let package_name = package_name.to_string(); let npm_resolver = npm_resolver.clone(); + let package_name = package_name.to_string(); + let memory_cache = self.memory_cache.clone(); async move { if let Some(managed) = npm_resolver.as_managed() { - managed.cache_package_info(&package_name).await?; + let package_info = + managed.cache_package_info(&package_name).await?; + memory_cache.borrow_mut().insert(package_name, package_info); } Ok(()) } - .boxed() + .boxed_local() } _ => { // return it succeeded and error at the import site below @@ -837,19 +861,50 @@ impl NpmResolver for CliGraphResolver { } } - fn resolve_npm(&self, package_req: &PackageReq) -> NpmPackageReqResolution { + async fn resolve_pkg_reqs( + &self, + package_reqs: &[&PackageReq], + ) -> NpmPackageReqsResolution { match &self.npm_resolver { - Some(npm_resolver) => match npm_resolver.as_inner() { - InnerCliNpmResolverRef::Managed(npm_resolver) => { - npm_resolver.resolve_npm_for_deno_graph(package_req) + Some(npm_resolver) => { + let npm_resolver = match npm_resolver.as_inner() { + InnerCliNpmResolverRef::Managed(npm_resolver) => npm_resolver, + // if we are using byonm, then this should never be called because + // we don't use deno_graph's npm resolution in this case + InnerCliNpmResolverRef::Byonm(_) => unreachable!(), + }; + + let reqs_with_package_infos = { + let memory_cache = self.memory_cache.borrow(); + package_reqs + .iter() + .map(|package_req| { + let package_info = memory_cache + .get(&package_req.name) + .unwrap() // ok because deno_graph would have called load_and_cache_npm_package_info + .clone(); + (*package_req, package_info) + }) + .collect::>() + }; + let result = npm_resolver + .resolve_npm_for_deno_graph(&reqs_with_package_infos) + .await; + if matches!(result, NpmPackageReqsResolution::ReloadRegistryInfo) { + self.memory_cache.borrow_mut().clear(); } - // if we are using byonm, then this should never be called because - // we don't use deno_graph's npm resolution in this case - InnerCliNpmResolverRef::Byonm(_) => unreachable!(), - }, - None => NpmPackageReqResolution::Err(anyhow!( - "npm specifiers were requested; but --no-npm is specified" - )), + result + } + None => NpmPackageReqsResolution::Resolutions( + package_reqs + .iter() + .map(|_| { + Err(Arc::new(anyhow!( + "npm specifiers were requested; but --no-npm is specified" + ))) + }) + .collect(), + ), } } diff --git a/cli/standalone/mod.rs b/cli/standalone/mod.rs index 92c061a99b..5e8c5c0b64 100644 --- a/cli/standalone/mod.rs +++ b/cli/standalone/mod.rs @@ -218,7 +218,7 @@ impl ModuleLoader for EmbeddedModuleLoader { MediaType::Json => ModuleType::Json, _ => ModuleType::JavaScript, }, - ModuleSourceCode::String(code_source.code), + code_source.code, &original_specifier, &code_source.found_url, None, diff --git a/cli/tools/bench/mod.rs b/cli/tools/bench/mod.rs index dd94205cb8..a6c8d3e16b 100644 --- a/cli/tools/bench/mod.rs +++ b/cli/tools/bench/mod.rs @@ -506,14 +506,14 @@ pub async fn run_benchmarks_with_watch( let bench_modules_to_reload = if let Some(changed_paths) = changed_paths { let changed_paths = changed_paths.into_iter().collect::>(); - let mut result = Vec::new(); + let mut result = IndexSet::with_capacity(bench_modules.len()); for bench_module_specifier in bench_modules { if has_graph_root_local_dependent_changed( &graph, bench_module_specifier, &changed_paths, ) { - result.push(bench_module_specifier.clone()); + result.insert(bench_module_specifier.clone()); } } result diff --git a/cli/tools/coverage/mod.rs b/cli/tools/coverage/mod.rs index 030bf425e5..6175bd9646 100644 --- a/cli/tools/coverage/mod.rs +++ b/cli/tools/coverage/mod.rs @@ -25,7 +25,6 @@ use deno_core::serde_json; use deno_core::sourcemap::SourceMap; use deno_core::url::Url; use deno_core::LocalInspectorSession; -use deno_core::ModuleCodeString; use regex::Regex; use std::fs; use std::fs::File; @@ -565,7 +564,7 @@ pub async fn cover_files( | MediaType::Cjs | MediaType::Mjs | MediaType::Json => None, - MediaType::Dts | MediaType::Dmts | MediaType::Dcts => Some(String::new()), + MediaType::Dts | MediaType::Dmts | MediaType::Dcts => Some(Vec::new()), MediaType::TypeScript | MediaType::Jsx | MediaType::Mts @@ -586,11 +585,13 @@ pub async fn cover_files( unreachable!() } }; - let runtime_code: ModuleCodeString = transpiled_code - .map(|c| c.into()) - .unwrap_or_else(|| original_source.clone().into()); + let runtime_code: String = match transpiled_code { + Some(code) => String::from_utf8(code) + .with_context(|| format!("Failed decoding {}", file.specifier))?, + None => original_source.to_string(), + }; - let source_map = source_map_from_code(&runtime_code); + let source_map = source_map_from_code(runtime_code.as_bytes()); let coverage_report = generate_coverage_report( &script_coverage, runtime_code.as_str().to_owned(), diff --git a/cli/tools/doc.rs b/cli/tools/doc.rs index bebe4567f9..44bf19d506 100644 --- a/cli/tools/doc.rs +++ b/cli/tools/doc.rs @@ -350,7 +350,7 @@ fn check_diagnostics(diagnostics: &[DocDiagnostic]) -> Result<(), AnyError> { for (_, diagnostics_by_col) in diagnostics_by_lc { for (_, diagnostics) in diagnostics_by_col { for diagnostic in diagnostics { - log::error!("{}", diagnostic.display()); + log::error!("{}\n", diagnostic.display()); } } } diff --git a/cli/tools/fmt.rs b/cli/tools/fmt.rs index ef3d48cb51..b37a8e06b4 100644 --- a/cli/tools/fmt.rs +++ b/cli/tools/fmt.rs @@ -211,7 +211,7 @@ fn format_markdown( codeblock_config.line_width = line_width; dprint_plugin_typescript::format_text( &fake_filename, - text, + text.to_string(), &codeblock_config, ) } @@ -255,7 +255,11 @@ pub fn format_file( ), _ => { let config = get_resolved_typescript_config(fmt_options); - dprint_plugin_typescript::format_text(file_path, file_text, &config) + dprint_plugin_typescript::format_text( + file_path, + file_text.to_string(), + &config, + ) } } } diff --git a/cli/tools/lint/mod.rs b/cli/tools/lint/mod.rs index 7253c99b24..151d0f11d1 100644 --- a/cli/tools/lint/mod.rs +++ b/cli/tools/lint/mod.rs @@ -244,7 +244,7 @@ async fn lint_files( incremental_cache.update_file( &file_path, // ensure the returned text is used here as it may have been modified via --fix - file_source.text_info().text_str(), + file_source.text(), ) } } @@ -396,7 +396,7 @@ fn lint_file_and_fix( media_type, linter, config.clone(), - source.text_info(), + source.text_info_lazy(), &diagnostics, )?; match change { @@ -423,7 +423,7 @@ fn lint_file_and_fix( if fix_iterations > 0 { // everything looks good and the file still parses, so write it out - fs::write(file_path, source.text_info().text_str()) + fs::write(file_path, source.text().as_ref()) .context("Failed writing fix to file.")?; } @@ -668,7 +668,7 @@ impl LintReporter for PrettyLintReporter { } } - log::error!("{}", d.display()); + log::error!("{}\n", d.display()); } fn visit_error(&mut self, file_path: &str, err: &AnyError) { diff --git a/cli/tools/registry/diagnostics.rs b/cli/tools/registry/diagnostics.rs index 1c3a3bd589..3f3e1ee96a 100644 --- a/cli/tools/registry/diagnostics.rs +++ b/cli/tools/registry/diagnostics.rs @@ -40,11 +40,7 @@ impl PublishDiagnosticsCollector { diagnostics.sort_by_cached_key(|d| d.sorting_key()); for diagnostic in diagnostics { - // todo(https://github.com/denoland/deno_ast/issues/245): use log crate here - #[allow(clippy::print_stderr)] - { - eprint!("{}", diagnostic.display()); - } + log::error!("{}", diagnostic.display()); if matches!(diagnostic.level(), DiagnosticLevel::Error) { errors += 1; } @@ -287,7 +283,7 @@ impl Diagnostic for PublishDiagnostic { Some(DiagnosticSnippet { source: Cow::Borrowed(text_info), - highlight: DiagnosticSnippetHighlight { + highlights: vec![DiagnosticSnippetHighlight { style: DiagnosticSnippetHighlightStyle::Error, range: DiagnosticSourceRange { start: DiagnosticSourcePos::LineAndCol { @@ -300,7 +296,7 @@ impl Diagnostic for PublishDiagnostic { }, }, description: Some("the specifier".into()), - }, + }], }) } @@ -314,14 +310,14 @@ impl Diagnostic for PublishDiagnostic { .. } => Some(DiagnosticSnippet { source: Cow::Borrowed(text_info), - highlight: DiagnosticSnippetHighlight { + highlights: vec![DiagnosticSnippetHighlight { style: DiagnosticSnippetHighlightStyle::Warning, range: DiagnosticSourceRange { start: DiagnosticSourcePos::SourcePos(range.start), end: DiagnosticSourcePos::SourcePos(range.end), }, description: Some("the unanalyzable dynamic import".into()), - }, + }], }), }, InvalidPath { .. } => None, @@ -343,14 +339,14 @@ impl Diagnostic for PublishDiagnostic { range, text_info, .. } => Some(DiagnosticSnippet { source: Cow::Borrowed(text_info), - highlight: DiagnosticSnippetHighlight { + highlights: vec![DiagnosticSnippetHighlight { style: DiagnosticSnippetHighlightStyle::Error, range: DiagnosticSourceRange { start: DiagnosticSourcePos::SourcePos(range.start), end: DiagnosticSourcePos::SourcePos(range.end), }, description: Some("the triple slash directive".into()), - }, + }], }), } } @@ -398,14 +394,14 @@ impl Diagnostic for PublishDiagnostic { let end = replacement.line_end(0); Some(DiagnosticSnippet { source: Cow::Owned(replacement), - highlight: DiagnosticSnippetHighlight { + highlights: vec![DiagnosticSnippetHighlight { style: DiagnosticSnippetHighlightStyle::Hint, range: DiagnosticSourceRange { start: DiagnosticSourcePos::SourcePos(start), end: DiagnosticSourcePos::SourcePos(end), }, description: Some("try this specifier".into()), - }, + }], }) } None => None, diff --git a/cli/tools/registry/graph.rs b/cli/tools/registry/graph.rs index b363efdb45..73b72c1b69 100644 --- a/cli/tools/registry/graph.rs +++ b/cli/tools/registry/graph.rs @@ -130,7 +130,7 @@ impl GraphDiagnosticsCollector { prefer_fast_check_graph: false, follow_type_only: true, }; - let mut iter = graph.walk(&graph.roots, options); + let mut iter = graph.walk(graph.roots.iter(), options); while let Some((specifier, entry)) = iter.next() { if skip_specifiers.contains(specifier) { iter.skip_previous_dependencies(); @@ -196,7 +196,7 @@ fn check_for_banned_triple_slash_directives( PublishDiagnostic::BannedTripleSlashDirectives { specifier: parsed_source.specifier().clone(), range: comment.range(), - text_info: parsed_source.text_info().clone(), + text_info: parsed_source.text_info_lazy().clone(), }, ); } diff --git a/cli/tools/registry/tar.rs b/cli/tools/registry/tar.rs index 8124a0c9ea..f98d4b09c9 100644 --- a/cli/tools/registry/tar.rs +++ b/cli/tools/registry/tar.rs @@ -126,7 +126,7 @@ fn resolve_content_maybe_unfurling( let text = String::from_utf8(data)?; deno_ast::parse_module(deno_ast::ParseParams { specifier: specifier.clone(), - text_info: deno_ast::SourceTextInfo::from_string(text), + text: text.into(), media_type, capture_tokens: false, maybe_syntax: None, diff --git a/cli/tools/registry/unfurl.rs b/cli/tools/registry/unfurl.rs index f45b6ffc37..bc3272835f 100644 --- a/cli/tools/registry/unfurl.rs +++ b/cli/tools/registry/unfurl.rs @@ -121,16 +121,15 @@ impl<'a> SpecifierUnfurler<'a> { fn try_unfurl_dynamic_dep( &self, module_url: &lsp_types::Url, - parsed_source: &ParsedSource, + text_info: &SourceTextInfo, dep: &deno_graph::DynamicDependencyDescriptor, text_changes: &mut Vec, ) -> bool { match &dep.argument { deno_graph::DynamicArgument::String(specifier) => { - let range = to_range(parsed_source, &dep.argument_range); - let maybe_relative_index = parsed_source.text_info().text_str() - [range.start..range.end] - .find(specifier); + let range = to_range(text_info, &dep.argument_range); + let maybe_relative_index = + text_info.text_str()[range.start..range.end].find(specifier); let Some(relative_index) = maybe_relative_index else { return true; // always say it's analyzable for a string }; @@ -159,9 +158,9 @@ impl<'a> SpecifierUnfurler<'a> { let Some(unfurled) = unfurled else { return true; // nothing to unfurl }; - let range = to_range(parsed_source, &dep.argument_range); + let range = to_range(text_info, &dep.argument_range); let maybe_relative_index = - parsed_source.text_info().text_str()[range.start..].find(specifier); + text_info.text_str()[range.start..].find(specifier); let Some(relative_index) = maybe_relative_index else { return false; }; @@ -192,6 +191,7 @@ impl<'a> SpecifierUnfurler<'a> { diagnostic_reporter: &mut dyn FnMut(SpecifierUnfurlerDiagnostic), ) -> String { let mut text_changes = Vec::new(); + let text_info = parsed_source.text_info_lazy(); let module_info = ParserModuleAnalyzer::module_info(parsed_source); let analyze_specifier = |specifier: &str, @@ -199,7 +199,7 @@ impl<'a> SpecifierUnfurler<'a> { text_changes: &mut Vec| { if let Some(unfurled) = self.unfurl_specifier(url, specifier) { text_changes.push(deno_ast::TextChange { - range: to_range(parsed_source, range), + range: to_range(text_info, range), new_text: unfurled, }); } @@ -214,27 +214,19 @@ impl<'a> SpecifierUnfurler<'a> { ); } DependencyDescriptor::Dynamic(dep) => { - let success = self.try_unfurl_dynamic_dep( - url, - parsed_source, - dep, - &mut text_changes, - ); + let success = + self.try_unfurl_dynamic_dep(url, text_info, dep, &mut text_changes); if !success { - let start_pos = parsed_source - .text_info() - .line_start(dep.argument_range.start.line) + let start_pos = text_info.line_start(dep.argument_range.start.line) + dep.argument_range.start.character; - let end_pos = parsed_source - .text_info() - .line_start(dep.argument_range.end.line) + let end_pos = text_info.line_start(dep.argument_range.end.line) + dep.argument_range.end.character; diagnostic_reporter( SpecifierUnfurlerDiagnostic::UnanalyzableDynamicImport { specifier: url.to_owned(), range: SourceRange::new(start_pos, end_pos), - text_info: parsed_source.text_info().clone(), + text_info: text_info.clone(), }, ); } @@ -267,10 +259,8 @@ impl<'a> SpecifierUnfurler<'a> { ); } - let rewritten_text = deno_ast::apply_text_changes( - parsed_source.text_info().text_str(), - text_changes, - ); + let rewritten_text = + deno_ast::apply_text_changes(text_info.text_str(), text_changes); rewritten_text } } @@ -295,13 +285,13 @@ fn relative_url( } fn to_range( - parsed_source: &ParsedSource, + text_info: &SourceTextInfo, range: &deno_graph::PositionRange, ) -> std::ops::Range { let mut range = range - .as_source_range(parsed_source.text_info()) - .as_byte_range(parsed_source.text_info().range().start); - let text = &parsed_source.text_info().text_str()[range.clone()]; + .as_source_range(text_info) + .as_byte_range(text_info.range().start); + let text = &text_info.text_str()[range.clone()]; if text.starts_with('"') || text.starts_with('\'') { range.start += 1; } @@ -338,7 +328,7 @@ mod tests { capture_tokens: false, maybe_syntax: None, scope_analysis: false, - text_info: deno_ast::SourceTextInfo::new(source_code.into()), + text: source_code.into(), }) .unwrap() } diff --git a/cli/tools/repl/session.rs b/cli/tools/repl/session.rs index 1bc8a96d0e..b122713c03 100644 --- a/cli/tools/repl/session.rs +++ b/cli/tools/repl/session.rs @@ -639,10 +639,11 @@ impl ReplSession { source_map: deno_ast::SourceMapOption::None, source_map_file: None, inline_sources: false, - keep_comments: false, + remove_comments: false, }, )? .into_source() + .into_string()? .text; let value = self @@ -817,7 +818,7 @@ fn parse_source_as( let parsed = deno_ast::parse_module(deno_ast::ParseParams { specifier, - text_info: deno_ast::SourceTextInfo::from_string(source), + text: source.into(), media_type, capture_tokens: true, maybe_syntax: None, @@ -884,7 +885,7 @@ fn analyze_jsx_pragmas( range: comment_source_to_position_range( c.start(), &m, - parsed_source.text_info(), + parsed_source.text_info_lazy(), true, ), }); @@ -898,7 +899,7 @@ fn analyze_jsx_pragmas( range: comment_source_to_position_range( c.start(), &m, - parsed_source.text_info(), + parsed_source.text_info_lazy(), false, ), }); @@ -912,7 +913,7 @@ fn analyze_jsx_pragmas( range: comment_source_to_position_range( c.start(), &m, - parsed_source.text_info(), + parsed_source.text_info_lazy(), false, ), }); diff --git a/cli/tools/test/mod.rs b/cli/tools/test/mod.rs index fa69ad950b..06ff39abe1 100644 --- a/cli/tools/test/mod.rs +++ b/cli/tools/test/mod.rs @@ -1249,7 +1249,7 @@ fn extract_files_from_source_comments( ) -> Result, AnyError> { let parsed_source = deno_ast::parse_module(deno_ast::ParseParams { specifier: specifier.clone(), - text_info: deno_ast::SourceTextInfo::new(source), + text: source, media_type, capture_tokens: false, maybe_syntax: None, @@ -1273,7 +1273,7 @@ fn extract_files_from_source_comments( specifier, &comment.text, media_type, - parsed_source.text_info().line_index(comment.start()), + parsed_source.text_info_lazy().line_index(comment.start()), blocks_regex, lines_regex, ) @@ -1877,7 +1877,7 @@ pub async fn run_tests_with_watch( let test_modules_to_reload = if let Some(changed_paths) = changed_paths { - let mut result = Vec::new(); + let mut result = IndexSet::with_capacity(test_modules.len()); let changed_paths = changed_paths.into_iter().collect::>(); for test_module_specifier in test_modules { if has_graph_root_local_dependent_changed( @@ -1885,7 +1885,7 @@ pub async fn run_tests_with_watch( test_module_specifier, &changed_paths, ) { - result.push(test_module_specifier.clone()); + result.insert(test_module_specifier.clone()); } } result diff --git a/cli/tools/vendor/analyze.rs b/cli/tools/vendor/analyze.rs index 2b00f6bf47..3e5964be66 100644 --- a/cli/tools/vendor/analyze.rs +++ b/cli/tools/vendor/analyze.rs @@ -64,7 +64,6 @@ mod test { use deno_ast::ModuleSpecifier; use deno_ast::ParseParams; use deno_ast::ParsedSource; - use deno_ast::SourceTextInfo; use super::has_default_export; @@ -107,7 +106,7 @@ mod test { maybe_syntax: None, media_type: MediaType::TypeScript, scope_analysis: false, - text_info: SourceTextInfo::from_string(text.to_string()), + text: text.into(), }) .unwrap() } diff --git a/cli/tools/vendor/build.rs b/cli/tools/vendor/build.rs index fd7401f777..5aef631928 100644 --- a/cli/tools/vendor/build.rs +++ b/cli/tools/vendor/build.rs @@ -118,7 +118,7 @@ pub async fn build< graph_util::graph_valid( &graph, &real_fs, - &graph.roots, + &graph.roots.iter().cloned().collect::>(), graph_util::GraphValidOptions { is_vendoring: true, check_js: true, diff --git a/cli/tools/vendor/import_map.rs b/cli/tools/vendor/import_map.rs index 7f627f35e5..68f2530d71 100644 --- a/cli/tools/vendor/import_map.rs +++ b/cli/tools/vendor/import_map.rs @@ -250,7 +250,7 @@ fn visit_modules( let parsed_source = parsed_source_cache.get_parsed_source_from_js_module(module)?; - let text_info = parsed_source.text_info().clone(); + let text_info = parsed_source.text_info_lazy().clone(); for dep in module.dependencies.values() { visit_resolution( diff --git a/cli/util/text_encoding.rs b/cli/util/text_encoding.rs index 25d827eb64..d2e0832c95 100644 --- a/cli/util/text_encoding.rs +++ b/cli/util/text_encoding.rs @@ -1,44 +1,132 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. +use std::ops::Range; + use base64::prelude::BASE64_STANDARD; use base64::Engine; -use deno_core::ModuleCodeString; +use deno_core::ModuleSourceCode; static SOURCE_MAP_PREFIX: &[u8] = b"//# sourceMappingURL=data:application/json;base64,"; -pub fn source_map_from_code(code: &ModuleCodeString) -> Option> { - let bytes = code.as_bytes(); - let last_line = bytes.rsplit(|u| *u == b'\n').next()?; - if last_line.starts_with(SOURCE_MAP_PREFIX) { - let input = last_line.split_at(SOURCE_MAP_PREFIX.len()).1; - let decoded_map = BASE64_STANDARD - .decode(input) - .expect("Unable to decode source map from emitted file."); - Some(decoded_map) +pub fn source_map_from_code(code: &[u8]) -> Option> { + let range = find_source_map_range(code)?; + let source_map_range = &code[range]; + let input = source_map_range.split_at(SOURCE_MAP_PREFIX.len()).1; + let decoded_map = BASE64_STANDARD.decode(input).ok()?; + Some(decoded_map) +} + +/// Truncate the source code before the source map. +pub fn code_without_source_map(code: ModuleSourceCode) -> ModuleSourceCode { + use deno_core::ModuleCodeBytes; + + match code { + ModuleSourceCode::String(mut code) => { + if let Some(range) = find_source_map_range(code.as_bytes()) { + code.truncate(range.start); + } + ModuleSourceCode::String(code) + } + ModuleSourceCode::Bytes(code) => { + if let Some(range) = find_source_map_range(code.as_bytes()) { + let source_map_index = range.start; + ModuleSourceCode::Bytes(match code { + ModuleCodeBytes::Static(bytes) => { + ModuleCodeBytes::Static(&bytes[..source_map_index]) + } + ModuleCodeBytes::Boxed(bytes) => { + // todo(dsherret): should be possible without cloning + ModuleCodeBytes::Boxed( + bytes[..source_map_index].to_vec().into_boxed_slice(), + ) + } + ModuleCodeBytes::Arc(bytes) => ModuleCodeBytes::Boxed( + bytes[..source_map_index].to_vec().into_boxed_slice(), + ), + }) + } else { + ModuleSourceCode::Bytes(code) + } + } + } +} + +fn find_source_map_range(code: &[u8]) -> Option> { + fn last_non_blank_line_range(code: &[u8]) -> Option> { + let mut hit_non_whitespace = false; + let mut range_end = code.len(); + for i in (0..code.len()).rev() { + match code[i] { + b' ' | b'\t' => { + if !hit_non_whitespace { + range_end = i; + } + } + b'\n' | b'\r' => { + if hit_non_whitespace { + return Some(i + 1..range_end); + } + range_end = i; + } + _ => { + hit_non_whitespace = true; + } + } + } + None + } + + let range = last_non_blank_line_range(code)?; + if code[range.start..range.end].starts_with(SOURCE_MAP_PREFIX) { + Some(range) } else { None } } -/// Truncate the source code before the source map. -pub fn code_without_source_map(mut code: ModuleCodeString) -> ModuleCodeString { - let bytes = code.as_bytes(); - for i in (0..bytes.len()).rev() { - if bytes[i] == b'\n' { - if bytes[i + 1..].starts_with(SOURCE_MAP_PREFIX) { - code.truncate(i + 1); - } - return code; - } - } - code -} - #[cfg(test)] mod tests { + use std::sync::Arc; + + use deno_core::ModuleCodeBytes; + use deno_core::ModuleCodeString; + use super::*; + #[test] + fn test_source_map_from_code() { + let to_string = + |bytes: Vec| -> String { String::from_utf8(bytes).unwrap() }; + assert_eq!( + source_map_from_code( + b"test\n//# sourceMappingURL=data:application/json;base64,dGVzdGluZ3Rlc3Rpbmc=", + ).map(to_string), + Some("testingtesting".to_string()) + ); + assert_eq!( + source_map_from_code( + b"test\n//# sourceMappingURL=data:application/json;base64,dGVzdGluZ3Rlc3Rpbmc=\n \n", + ).map(to_string), + Some("testingtesting".to_string()) + ); + assert_eq!( + source_map_from_code( + b"test\n//# sourceMappingURL=data:application/json;base64,dGVzdGluZ3Rlc3Rpbmc=\n test\n", + ), + None + ); + assert_eq!( + source_map_from_code( + b"\"use strict\"; + +throw new Error(\"Hello world!\"); +//# sourceMappingURL=data:application/json;base64,{", + ), + None + ); + } + #[test] fn test_source_without_source_map() { run_test("", ""); @@ -62,14 +150,39 @@ mod tests { "\n//# sourceMappingURL=data:application/json;base64,test", "\n", ); + run_test( + "test\n//# sourceMappingURL=data:application/json;base64,test\n\n", + "test\n", + ); + run_test( + "test\n//# sourceMappingURL=data:application/json;base64,test\n \n ", + "test\n", + ); fn run_test(input: &'static str, output: &'static str) { - assert_eq!( - code_without_source_map(ModuleCodeString::from_static(input)) - .as_str() - .to_owned(), - output - ); + let forms = [ + ModuleSourceCode::String(ModuleCodeString::from_static(input)), + ModuleSourceCode::String({ + let text: Arc = input.into(); + text.into() + }), + ModuleSourceCode::String({ + let text: String = input.into(); + text.into() + }), + ModuleSourceCode::Bytes(ModuleCodeBytes::Static(input.as_bytes())), + ModuleSourceCode::Bytes(ModuleCodeBytes::Boxed( + input.as_bytes().to_vec().into_boxed_slice(), + )), + ModuleSourceCode::Bytes(ModuleCodeBytes::Arc( + input.as_bytes().to_vec().into(), + )), + ]; + for form in forms { + let result = code_without_source_map(form); + let bytes = result.as_bytes(); + assert_eq!(bytes, output.as_bytes()); + } } } } diff --git a/runtime/shared.rs b/runtime/shared.rs index 61f9ca2250..1b2136c638 100644 --- a/runtime/shared.rs +++ b/runtime/shared.rs @@ -4,7 +4,6 @@ use deno_ast::MediaType; use deno_ast::ParseParams; use deno_ast::SourceMapOption; -use deno_ast::SourceTextInfo; use deno_core::error::AnyError; use deno_core::extension; use deno_core::Extension; @@ -88,7 +87,7 @@ pub fn maybe_transpile_source( let parsed = deno_ast::parse_module(ParseParams { specifier: deno_core::url::Url::parse(&name).unwrap(), - text_info: SourceTextInfo::from_string(source.as_str().to_owned()), + text: source.into(), media_type, capture_tokens: false, scope_analysis: false, @@ -111,9 +110,9 @@ pub fn maybe_transpile_source( )? .into_source(); - let maybe_source_map: Option = transpiled_source - .source_map - .map(|sm| sm.into_bytes().into()); + let maybe_source_map: Option = + transpiled_source.source_map.map(|sm| sm.into()); + let source_text = String::from_utf8(transpiled_source.source)?; - Ok((transpiled_source.text.into(), maybe_source_map)) + Ok((source_text.into(), maybe_source_map)) } diff --git a/tests/Cargo.toml b/tests/Cargo.toml index 4b8db95647..46a152e1b3 100644 --- a/tests/Cargo.toml +++ b/tests/Cargo.toml @@ -43,7 +43,7 @@ deno_lockfile.workspace = true deno_terminal.workspace = true deno_tls.workspace = true fastwebsockets = { workspace = true, features = ["upgrade", "unstable-split"] } -file_test_runner = "0.7.0" +file_test_runner = "0.7.2" flaky_test = "=0.1.0" http.workspace = true http-body-util.workspace = true diff --git a/tests/integration/watcher_tests.rs b/tests/integration/watcher_tests.rs index 42e4924a84..252411627d 100644 --- a/tests/integration/watcher_tests.rs +++ b/tests/integration/watcher_tests.rs @@ -1725,7 +1725,7 @@ globalThis.state = { i: 0 }; function bar() { globalThis.state.i = 0; - console.log("got request1", globalThis.state.i); + console.error("got request1", globalThis.state.i); } function handler(_req) { @@ -1738,8 +1738,14 @@ console.log("Listening...") "#, ); - wait_contains("Failed to reload module", &mut stderr_lines).await; - wait_contains("File change detected", &mut stderr_lines).await; + wait_contains("Replaced changed module", &mut stderr_lines).await; + util::deno_cmd() + .current_dir(t.path()) + .arg("eval") + .arg("await fetch('http://localhost:11111');") + .spawn() + .unwrap(); + wait_contains("got request1", &mut stderr_lines).await; check_alive_then_kill(child); } diff --git a/tests/specs/README.md b/tests/specs/README.md index 61fa60f538..bd3430f058 100644 --- a/tests/specs/README.md +++ b/tests/specs/README.md @@ -76,9 +76,7 @@ Or if you want to run several tests at the same time: ### Top level properties -- `base` - The base config to use for the test. Options: - - `jsr` - Uses env vars for jsr. - - `npm` - Uses env vars for npm. +- `repeat` (number) - Number of times to repeat a test. - `tempDir` (boolean) - Copy all the non-test files to a temporary directory and execute the command in that temporary directory. - By default, tests are executed with a current working directory of the test, diff --git a/tests/specs/lint/no_slow_types/no_slow_types.out b/tests/specs/lint/no_slow_types/no_slow_types.out index 5828906e76..1093d032c6 100644 --- a/tests/specs/lint/no_slow_types/no_slow_types.out +++ b/tests/specs/lint/no_slow_types/no_slow_types.out @@ -3,6 +3,7 @@ error[no-slow-types]: missing explicit return type in the public API | 1 | export function add(a: number, b: number) { | ^^^ this function is missing an explicit return type + | = hint: add an explicit return type to the function info: all functions in the public API must have an explicit return type @@ -14,6 +15,7 @@ error[no-slow-types]: missing explicit return type in the public API | 1 | export function addB(a: number, b: number) { | ^^^^ this function is missing an explicit return type + | = hint: add an explicit return type to the function info: all functions in the public API must have an explicit return type @@ -25,6 +27,7 @@ error[no-slow-types]: missing explicit return type in the public API | 2 | export function addD(a: number, b: number) { | ^^^^ this function is missing an explicit return type + | = hint: add an explicit return type to the function info: all functions in the public API must have an explicit return type diff --git a/tests/specs/lint/no_slow_types/no_slow_types_entrypoint.out b/tests/specs/lint/no_slow_types/no_slow_types_entrypoint.out index b8c1013bf0..895dcb6cd9 100644 --- a/tests/specs/lint/no_slow_types/no_slow_types_entrypoint.out +++ b/tests/specs/lint/no_slow_types/no_slow_types_entrypoint.out @@ -3,6 +3,7 @@ error[no-slow-types]: missing explicit return type in the public API | 1 | export function add(a: number, b: number) { | ^^^ this function is missing an explicit return type + | = hint: add an explicit return type to the function info: all functions in the public API must have an explicit return type @@ -14,6 +15,7 @@ error[no-slow-types]: missing explicit return type in the public API | 1 | export function addB(a: number, b: number) { | ^^^^ this function is missing an explicit return type + | = hint: add an explicit return type to the function info: all functions in the public API must have an explicit return type @@ -25,6 +27,7 @@ error[no-slow-types]: missing explicit return type in the public API | 2 | export function addD(a: number, b: number) { | ^^^^ this function is missing an explicit return type + | = hint: add an explicit return type to the function info: all functions in the public API must have an explicit return type diff --git a/tests/specs/lint/no_slow_types_entrypoint/no_slow_types_entrypoint.out b/tests/specs/lint/no_slow_types_entrypoint/no_slow_types_entrypoint.out index b8c1013bf0..895dcb6cd9 100644 --- a/tests/specs/lint/no_slow_types_entrypoint/no_slow_types_entrypoint.out +++ b/tests/specs/lint/no_slow_types_entrypoint/no_slow_types_entrypoint.out @@ -3,6 +3,7 @@ error[no-slow-types]: missing explicit return type in the public API | 1 | export function add(a: number, b: number) { | ^^^ this function is missing an explicit return type + | = hint: add an explicit return type to the function info: all functions in the public API must have an explicit return type @@ -14,6 +15,7 @@ error[no-slow-types]: missing explicit return type in the public API | 1 | export function addB(a: number, b: number) { | ^^^^ this function is missing an explicit return type + | = hint: add an explicit return type to the function info: all functions in the public API must have an explicit return type @@ -25,6 +27,7 @@ error[no-slow-types]: missing explicit return type in the public API | 2 | export function addD(a: number, b: number) { | ^^^^ this function is missing an explicit return type + | = hint: add an explicit return type to the function info: all functions in the public API must have an explicit return type diff --git a/tests/specs/lint/no_slow_types_workspace/output.out b/tests/specs/lint/no_slow_types_workspace/output.out index 05f54099b3..1f19fbab8d 100644 --- a/tests/specs/lint/no_slow_types_workspace/output.out +++ b/tests/specs/lint/no_slow_types_workspace/output.out @@ -3,6 +3,7 @@ error[no-slow-types]: missing explicit return type in the public API | 1 | export function addB(a: number, b: number) { | ^^^^ this function is missing an explicit return type + | = hint: add an explicit return type to the function info: all functions in the public API must have an explicit return type @@ -14,6 +15,7 @@ error[no-slow-types]: missing explicit return type in the public API | 2 | export function addD(a: number, b: number) { | ^^^^ this function is missing an explicit return type + | = hint: add an explicit return type to the function info: all functions in the public API must have an explicit return type @@ -25,6 +27,7 @@ error[no-slow-types]: missing explicit return type in the public API | 1 | export function add(a: number, b: number) { | ^^^ this function is missing an explicit return type + | = hint: add an explicit return type to the function info: all functions in the public API must have an explicit return type @@ -36,6 +39,7 @@ error[no-slow-types]: missing explicit return type in the public API | 2 | export function addC(a: number, b: number) { | ^^^^ this function is missing an explicit return type + | = hint: add an explicit return type to the function info: all functions in the public API must have an explicit return type diff --git a/tests/specs/mod.rs b/tests/specs/mod.rs index a153322db6..3a52ab29ea 100644 --- a/tests/specs/mod.rs +++ b/tests/specs/mod.rs @@ -17,6 +17,7 @@ use file_test_runner::collection::CollectTestsError; use file_test_runner::collection::CollectedCategoryOrTest; use file_test_runner::collection::CollectedTest; use file_test_runner::collection::CollectedTestCategory; +use file_test_runner::SubTestResult; use file_test_runner::TestResult; use serde::Deserialize; use test_util::tests_path; @@ -109,6 +110,8 @@ struct MultiStepMetaData { #[serde(default)] pub envs: HashMap, #[serde(default)] + pub repeat: Option, + #[serde(default)] pub steps: Vec, } @@ -119,6 +122,8 @@ struct SingleTestMetaData { pub base: Option, #[serde(default)] pub temp_dir: bool, + #[serde(default)] + pub repeat: Option, #[serde(flatten)] pub step: StepMetaData, } @@ -128,6 +133,7 @@ impl SingleTestMetaData { MultiStepMetaData { base: self.base, temp_dir: self.temp_dir, + repeat: self.repeat, envs: Default::default(), steps: vec![self.step], } @@ -213,8 +219,26 @@ fn run_test(test: &CollectedTest) -> TestResult { let cwd = PathRef::new(&test.path).parent(); let metadata_value = test.data.clone(); let diagnostic_logger = Rc::new(RefCell::new(Vec::::new())); - let result = TestResult::from_maybe_panic(AssertUnwindSafe(|| { - run_test_inner(metadata_value, &cwd, diagnostic_logger.clone()) + let result = TestResult::from_maybe_panic_or_result(AssertUnwindSafe(|| { + let metadata = deserialize_value(metadata_value); + if let Some(repeat) = metadata.repeat { + TestResult::SubTests( + (0..repeat) + .map(|i| { + let diagnostic_logger = diagnostic_logger.clone(); + SubTestResult { + name: format!("run {}", i + 1), + result: TestResult::from_maybe_panic(AssertUnwindSafe(|| { + run_test_inner(&metadata, &cwd, diagnostic_logger); + })), + } + }) + .collect(), + ) + } else { + run_test_inner(&metadata, &cwd, diagnostic_logger.clone()); + TestResult::Passed + } })); match result { TestResult::Failed { @@ -232,15 +256,13 @@ fn run_test(test: &CollectedTest) -> TestResult { } fn run_test_inner( - metadata_value: serde_json::Value, + metadata: &MultiStepMetaData, cwd: &PathRef, diagnostic_logger: Rc>>, ) { - let metadata = deserialize_value(metadata_value); - - let context = test_context_from_metadata(&metadata, cwd, diagnostic_logger); + let context = test_context_from_metadata(metadata, cwd, diagnostic_logger); for step in metadata.steps.iter().filter(|s| should_run_step(s)) { - let run_func = || run_step(step, &metadata, cwd, &context); + let run_func = || run_step(step, metadata, cwd, &context); if step.flaky { run_flaky(run_func); } else { diff --git a/tests/specs/npm/workers/__test__.jsonc b/tests/specs/npm/workers/__test__.jsonc new file mode 100644 index 0000000000..f2066c17f1 --- /dev/null +++ b/tests/specs/npm/workers/__test__.jsonc @@ -0,0 +1,5 @@ +{ + "tempDir": true, + "args": "run -A --quiet --lock=deno.lock main.ts", + "output": "main.out" +} diff --git a/tests/specs/npm/workers/main.out b/tests/specs/npm/workers/main.out new file mode 100644 index 0000000000..55ff51cd3d --- /dev/null +++ b/tests/specs/npm/workers/main.out @@ -0,0 +1,5 @@ +[UNORDERED_START] +1 +2 +3 +[UNORDERED_END] diff --git a/tests/specs/npm/workers/main.ts b/tests/specs/npm/workers/main.ts new file mode 100644 index 0000000000..f51cf4d902 --- /dev/null +++ b/tests/specs/npm/workers/main.ts @@ -0,0 +1,9 @@ +new Worker(new URL("./worker1.ts", import.meta.url), { + type: "module", +}); +new Worker(new URL("./worker2.ts", import.meta.url), { + type: "module", +}); +new Worker(new URL("./worker3.ts", import.meta.url), { + type: "module", +}); diff --git a/tests/specs/npm/workers/worker1.ts b/tests/specs/npm/workers/worker1.ts new file mode 100644 index 0000000000..1cdc91d562 --- /dev/null +++ b/tests/specs/npm/workers/worker1.ts @@ -0,0 +1,9 @@ +import "npm:chalk@4"; +import "npm:react@18.2"; +import "npm:preact@10.19"; +import "npm:ajv"; +import "npm:has"; +import "npm:picocolors"; + +console.log(1); +self.close(); diff --git a/tests/specs/npm/workers/worker2.ts b/tests/specs/npm/workers/worker2.ts new file mode 100644 index 0000000000..2495ebaccc --- /dev/null +++ b/tests/specs/npm/workers/worker2.ts @@ -0,0 +1,6 @@ +import "npm:@denotest/esm-basic"; +import "npm:@denotest/add"; +import "npm:@denotest/subtract"; + +console.log(2); +self.close(); diff --git a/tests/specs/npm/workers/worker3.ts b/tests/specs/npm/workers/worker3.ts new file mode 100644 index 0000000000..ae18d6aa9e --- /dev/null +++ b/tests/specs/npm/workers/worker3.ts @@ -0,0 +1,6 @@ +import "npm:@denotest/subtract"; +import "npm:@denotest/add"; +import "npm:@denotest/esm-basic"; + +console.log(3); +self.close(); diff --git a/tests/specs/publish/banned_triple_slash_directives/publish.out b/tests/specs/publish/banned_triple_slash_directives/publish.out index a67736bc21..f1827538ae 100644 --- a/tests/specs/publish/banned_triple_slash_directives/publish.out +++ b/tests/specs/publish/banned_triple_slash_directives/publish.out @@ -6,6 +6,7 @@ error[banned-triple-slash-directives]: triple slash directives that modify globa | 1 | /// | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the triple slash directive + | = hint: remove the triple slash directive info: instead instruct the user of your package to specify these directives @@ -17,6 +18,7 @@ error[banned-triple-slash-directives]: triple slash directives that modify globa | 2 | /// | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the triple slash directive + | = hint: remove the triple slash directive info: instead instruct the user of your package to specify these directives diff --git a/tests/specs/publish/has_slow_types/has_slow_types.out b/tests/specs/publish/has_slow_types/has_slow_types.out index 43ac86658d..b771349029 100644 --- a/tests/specs/publish/has_slow_types/has_slow_types.out +++ b/tests/specs/publish/has_slow_types/has_slow_types.out @@ -5,6 +5,7 @@ error[missing-explicit-return-type]: missing explicit return type in the public | 2 | export function getRandom() { | ^^^^^^^^^ this function is missing an explicit return type + | = hint: add an explicit return type to the function info: all functions in the public API must have an explicit return type diff --git a/tests/specs/publish/invalid_import/invalid_import.out b/tests/specs/publish/invalid_import/invalid_import.out index 1f95290dcb..929fc72cd3 100644 --- a/tests/specs/publish/invalid_import/invalid_import.out +++ b/tests/specs/publish/invalid_import/invalid_import.out @@ -10,6 +10,7 @@ error[invalid-external-import]: invalid import to a non-JSR 'http' specifier | 1 | import "http://localhost:4545/welcome.ts"; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the specifier + | = hint: replace this import with one from jsr or npm, or vendor the dependency into your package info: the import was resolved to 'http://localhost:4545/welcome.ts' @@ -22,6 +23,7 @@ error[invalid-external-import]: invalid import to a non-JSR 'http' specifier | 2 | import "$echo"; | ^^^^^^^ the specifier + | = hint: replace this import with one from jsr or npm, or vendor the dependency into your package info: the import was resolved to 'http://localhost:4545/echo.ts' diff --git a/tests/specs/publish/invalid_import_esm_sh_suggestion/invalid_import_esm_sh_suggestion.out b/tests/specs/publish/invalid_import_esm_sh_suggestion/invalid_import_esm_sh_suggestion.out index a014f3de6b..a7235cbf16 100644 --- a/tests/specs/publish/invalid_import_esm_sh_suggestion/invalid_import_esm_sh_suggestion.out +++ b/tests/specs/publish/invalid_import_esm_sh_suggestion/invalid_import_esm_sh_suggestion.out @@ -7,10 +7,12 @@ error[invalid-external-import]: invalid import to a non-JSR 'http' specifier | 1 | import "http://esm.sh/react-dom@18.2.0/server"; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the specifier + | = hint: replace this import with one from jsr or npm, or vendor the dependency into your package | 1 | "npm:react-dom@18.2.0" | ---------------------- try this specifier + | info: the import was resolved to 'http://esm.sh/react-dom@18.2.0/server' info: this specifier is not allowed to be imported on jsr diff --git a/tests/specs/publish/missing_constraint/publish.out b/tests/specs/publish/missing_constraint/publish.out index 846612979e..601035b059 100644 --- a/tests/specs/publish/missing_constraint/publish.out +++ b/tests/specs/publish/missing_constraint/publish.out @@ -6,6 +6,7 @@ error[missing-constraint]: specifier 'jsr:@denotest/add' is missing a version co | 1 | import { add } from "add"; | ^^^^^ the specifier + | = hint: specify a version constraint for the specifier in the import map info: the specifier resolved to version 1.0.0 today, but will resolve to a different @@ -17,6 +18,7 @@ error[missing-constraint]: specifier 'npm:@denotest/esm-basic' is missing a vers | 2 | import * as basic from "basic"; | ^^^^^^^ the specifier + | = hint: specify a version constraint for the specifier in the import map info: the specifier resolved to version 1.0.0 today, but will resolve to a different @@ -28,6 +30,7 @@ error[missing-constraint]: specifier 'jsr:@denotest/deps' is missing a version c | 3 | import * as deps from "jsr:@denotest/deps"; | ^^^^^^^^^^^^^^^^^^^^ the specifier + | = hint: specify a version constraint for the specifier info: the specifier resolved to version 1.0.0 today, but will resolve to a different diff --git a/tests/specs/publish/prefer_fast_check_graph/main.out b/tests/specs/publish/prefer_fast_check_graph/main.out index a68fac83a2..6c68dde8ac 100644 --- a/tests/specs/publish/prefer_fast_check_graph/main.out +++ b/tests/specs/publish/prefer_fast_check_graph/main.out @@ -7,6 +7,7 @@ error[invalid-external-import]: invalid import to a non-JSR 'https' specifier | 1 | export * from "https://deno.land/std/assert/assert.ts"; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the specifier + | = hint: replace this import with one from jsr or npm, or vendor the dependency into your package info: the import was resolved to 'https://deno.land/std/assert/assert.ts' diff --git a/tests/specs/publish/unanalyzable_dynamic_import/unanalyzable_dynamic_import.out b/tests/specs/publish/unanalyzable_dynamic_import/unanalyzable_dynamic_import.out index 7f3ca55554..af92d46b92 100644 --- a/tests/specs/publish/unanalyzable_dynamic_import/unanalyzable_dynamic_import.out +++ b/tests/specs/publish/unanalyzable_dynamic_import/unanalyzable_dynamic_import.out @@ -6,6 +6,7 @@ warning[unanalyzable-dynamic-import]: unable to analyze dynamic import | 2 | await import("asd " + asd); | ^^^^^^^^^^^^ the unanalyzable dynamic import + | info: after publishing this package, imports from the local import map / package.json do not work info: dynamic imports that can not be analyzed at publish time will not be rewritten automatically diff --git a/tests/specs/schema.json b/tests/specs/schema.json index 0edfb45077..d41babd19e 100644 --- a/tests/specs/schema.json +++ b/tests/specs/schema.json @@ -69,6 +69,9 @@ "type": "string" } }, + "repeat": { + "type": "number" + }, "steps": { "type": "array", "items": { @@ -112,6 +115,9 @@ "type": "string" } }, + "repeat": { + "type": "number" + }, "tests": { "type": "object", "additionalProperties": { diff --git a/tests/testdata/doc/referenced_private_types_lint.out b/tests/testdata/doc/referenced_private_types_lint.out index 328435cd79..1de3247bf8 100644 --- a/tests/testdata/doc/referenced_private_types_lint.out +++ b/tests/testdata/doc/referenced_private_types_lint.out @@ -14,6 +14,7 @@ error[private-type-ref]: public type 'MyClass.prototype.prop' references private | 1 | interface MyInterface { | - this is the referenced type + | info: to ensure documentation is complete all types that are exposed in the public API must be public diff --git a/tools/util.js b/tools/util.js index 75ee1b5272..1497a28873 100644 --- a/tools/util.js +++ b/tools/util.js @@ -227,6 +227,12 @@ export async function downloadPrebuilt(toolName) { ); } spinner.text = `Successfully downloaded: ${toolName}`; + try { + // necessary on Windows it seems + await Deno.remove(toolPath); + } catch { + // ignore + } await Deno.rename(tempFile, toolPath); } catch (e) { spinner.fail();