From 783533d2e354ad73356d7517b26293e48c10fc17 Mon Sep 17 00:00:00 2001 From: Kenta Moriuchi Date: Tue, 30 Apr 2024 05:43:05 +0900 Subject: [PATCH 1/5] FUTURE: remove import assertions support for JavaScript (#23541) Ref #17944, https://github.com/swc-project/swc/issues/8893 TypeScript removes the `assert` keywords in the transpile, so this PR only works for JavaScript files --- cli/args/mod.rs | 2 +- cli/lsp/config.rs | 3 ++- cli/main.rs | 11 ++++++++++- tests/integration/run_tests.rs | 10 ---------- .../future/import_assertions/__test__.jsonc | 16 ++++++++++++++++ tests/specs/future/import_assertions/error.out | 4 ++++ tests/specs/future/import_assertions/main.js | 2 ++ tests/specs/future/import_assertions/main.json | 3 +++ tests/specs/future/import_assertions/success.out | 1 + tests/testdata/npm/import_json/main.js | 2 +- tests/testdata/run/deno_futures_env.ts | 3 --- tests/testdata/subdir/mod7.js | 2 +- tests/testdata/subdir/mod8.js | 2 +- tools/napi/generate_symbols_lists.js | 2 +- 14 files changed, 43 insertions(+), 20 deletions(-) create mode 100644 tests/specs/future/import_assertions/__test__.jsonc create mode 100644 tests/specs/future/import_assertions/error.out create mode 100644 tests/specs/future/import_assertions/main.js create mode 100644 tests/specs/future/import_assertions/main.json create mode 100644 tests/specs/future/import_assertions/success.out delete mode 100644 tests/testdata/run/deno_futures_env.ts diff --git a/cli/args/mod.rs b/cli/args/mod.rs index 1d8b06f49d201c..b77a8afdbf1889 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -110,7 +110,7 @@ pub static DENO_DISABLE_PEDANTIC_NODE_WARNINGS: Lazy = Lazy::new(|| { .is_some() }); -static DENO_FUTURE: Lazy = +pub static DENO_FUTURE: Lazy = Lazy::new(|| std::env::var("DENO_FUTURE").ok().is_some()); pub fn jsr_url() -> &'static Url { diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index 5ddc41cb22d624..e5703a21a49767 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -4,6 +4,7 @@ use super::logging::lsp_log; use crate::args::ConfigFile; use crate::args::FmtOptions; use crate::args::LintOptions; +use crate::args::DENO_FUTURE; use crate::cache::FastInsecureHasher; use crate::file_fetcher::FileFetcher; use crate::lsp::logging::lsp_warn; @@ -1324,7 +1325,7 @@ impl ConfigData { .as_ref() .map(|c| c.has_unstable("byonm")) .unwrap_or(false) - || (std::env::var("DENO_FUTURE").is_ok() + || (*DENO_FUTURE && package_json.is_some() && config_file .as_ref() diff --git a/cli/main.rs b/cli/main.rs index 142ae017ce9348..3b103e7807b56e 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -29,6 +29,7 @@ mod worker; use crate::args::flags_from_vec; use crate::args::DenoSubcommand; use crate::args::Flags; +use crate::args::DENO_FUTURE; use crate::util::display; use crate::util::v8::get_v8_flags_from_env; use crate::util::v8::init_v8_flags; @@ -389,7 +390,15 @@ fn resolve_flags_and_init( // Using same default as VSCode: // https://github.com/microsoft/vscode/blob/48d4ba271686e8072fc6674137415bc80d936bc7/extensions/typescript-language-features/src/configuration/configuration.ts#L213-L214 DenoSubcommand::Lsp => vec!["--max-old-space-size=3072".to_string()], - _ => vec![], + _ => { + if *DENO_FUTURE { + // deno_ast removes TypeScript `assert` keywords, so this flag only affects JavaScript + // TODO(petamoriken): Need to check TypeScript `assert` keywords in deno_ast + vec!["--no-harmony-import-assertions".to_string()] + } else { + vec![] + } + } }; init_v8_flags(&default_v8_flags, &flags.v8_flags, get_v8_flags_from_env()); diff --git a/tests/integration/run_tests.rs b/tests/integration/run_tests.rs index e28251b23d4d01..e92fd36276e2b0 100644 --- a/tests/integration/run_tests.rs +++ b/tests/integration/run_tests.rs @@ -1694,16 +1694,6 @@ fn type_directives_js_main() { assert_not_contains!(output.combined_output(), "type_reference.d.ts"); } -#[test] -fn test_deno_futures_env() { - let context = TestContextBuilder::new().add_future_env_vars().build(); - let output = context - .new_command() - .args("run --quiet --reload run/deno_futures_env.ts") - .run(); - output.assert_exit_code(0); -} - itest!(type_directives_redirect { args: "run --reload --check run/type_directives_redirect.ts", output: "run/type_directives_redirect.ts.out", diff --git a/tests/specs/future/import_assertions/__test__.jsonc b/tests/specs/future/import_assertions/__test__.jsonc new file mode 100644 index 00000000000000..a1e759c75ebd3a --- /dev/null +++ b/tests/specs/future/import_assertions/__test__.jsonc @@ -0,0 +1,16 @@ +{ + "steps": [ + { + "args": "run main.js", + "output": "error.out", + "exitCode": 1, + "envs": { + "DENO_FUTURE": "1" + } + }, + { + "args": "run main.js", + "output": "success.out" + } + ] +} diff --git a/tests/specs/future/import_assertions/error.out b/tests/specs/future/import_assertions/error.out new file mode 100644 index 00000000000000..cef663f35fb015 --- /dev/null +++ b/tests/specs/future/import_assertions/error.out @@ -0,0 +1,4 @@ +error: Uncaught SyntaxError: Unexpected identifier 'assert' +import foo from "./main.json" assert { type: "json" }; + ^ + at (file:///[WILDCARD]/main.js:[WILDCARD]) diff --git a/tests/specs/future/import_assertions/main.js b/tests/specs/future/import_assertions/main.js new file mode 100644 index 00000000000000..9b4c4d0360977a --- /dev/null +++ b/tests/specs/future/import_assertions/main.js @@ -0,0 +1,2 @@ +import foo from "./main.json" assert { type: "json" }; +console.log(foo); diff --git a/tests/specs/future/import_assertions/main.json b/tests/specs/future/import_assertions/main.json new file mode 100644 index 00000000000000..abdd5202bdfbe6 --- /dev/null +++ b/tests/specs/future/import_assertions/main.json @@ -0,0 +1,3 @@ +{ + "foo": "foo" +} diff --git a/tests/specs/future/import_assertions/success.out b/tests/specs/future/import_assertions/success.out new file mode 100644 index 00000000000000..70ec274d9afb99 --- /dev/null +++ b/tests/specs/future/import_assertions/success.out @@ -0,0 +1 @@ +{ foo: "foo" } diff --git a/tests/testdata/npm/import_json/main.js b/tests/testdata/npm/import_json/main.js index b752bdef8b6aa1..ac6cee9a830df1 100644 --- a/tests/testdata/npm/import_json/main.js +++ b/tests/testdata/npm/import_json/main.js @@ -1,4 +1,4 @@ -import json from "npm:@denotest/binary-package@1/package.json" assert { +import json from "npm:@denotest/binary-package@1/package.json" with { type: "json", }; console.log(json); diff --git a/tests/testdata/run/deno_futures_env.ts b/tests/testdata/run/deno_futures_env.ts deleted file mode 100644 index 21f76e367560e4..00000000000000 --- a/tests/testdata/run/deno_futures_env.ts +++ /dev/null @@ -1,3 +0,0 @@ -if (typeof window !== "undefined") { - throw new Error("Window global available"); -} diff --git a/tests/testdata/subdir/mod7.js b/tests/testdata/subdir/mod7.js index 2bd4b5eb785b66..e71ced92e906f7 100644 --- a/tests/testdata/subdir/mod7.js +++ b/tests/testdata/subdir/mod7.js @@ -1,3 +1,3 @@ -import json1 from "./json_1.json" assert { type: "json" }; +import json1 from "./json_1.json" with { type: "json" }; console.log(json1); diff --git a/tests/testdata/subdir/mod8.js b/tests/testdata/subdir/mod8.js index 5bf7a49a880b97..ed41d992c06dcc 100644 --- a/tests/testdata/subdir/mod8.js +++ b/tests/testdata/subdir/mod8.js @@ -1,3 +1,3 @@ -import json3 from "./json_3.json" assert { type: "json" }; +import json3 from "./json_3.json" with { type: "json" }; console.log(json3); diff --git a/tools/napi/generate_symbols_lists.js b/tools/napi/generate_symbols_lists.js index 96636ffd7d2069..11cf1c434ae92e 100755 --- a/tools/napi/generate_symbols_lists.js +++ b/tools/napi/generate_symbols_lists.js @@ -1,7 +1,7 @@ #!/usr/bin/env -S deno run --allow-read --allow-write // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. -import exports from "../../cli/napi/sym/symbol_exports.json" assert { +import exports from "../../cli/napi/sym/symbol_exports.json" with { type: "json", }; From 5cae3439912ad60eb2866f3d4372a5fe4d0de957 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Tue, 30 Apr 2024 02:41:19 +0100 Subject: [PATCH 2/5] refactor(lsp): move fields from Documents to LspResolver (#23585) --- cli/lsp/analysis.rs | 5 +- cli/lsp/diagnostics.rs | 4 +- cli/lsp/documents.rs | 178 ++++++------------------------------- cli/lsp/language_server.rs | 22 ++--- cli/lsp/resolver.rs | 166 ++++++++++++++++++++++++++++++++-- cli/lsp/tsc.rs | 8 +- 6 files changed, 203 insertions(+), 180 deletions(-) diff --git a/cli/lsp/analysis.rs b/cli/lsp/analysis.rs index 63d39ad6e379d7..23b6bb09990ba4 100644 --- a/cli/lsp/analysis.rs +++ b/cli/lsp/analysis.rs @@ -259,8 +259,7 @@ impl<'a> TsResponseImportMapper<'a> { let version = Version::parse_standard(segments.next()?).ok()?; let nv = PackageNv { name, version }; let path = segments.collect::>().join("/"); - let jsr_resolver = self.documents.get_jsr_resolver(); - let export = jsr_resolver.lookup_export_for_path(&nv, &path)?; + let export = self.resolver.jsr_lookup_export_for_path(&nv, &path)?; let sub_path = (export != ".").then_some(export); let mut req = None; req = req.or_else(|| { @@ -282,7 +281,7 @@ impl<'a> TsResponseImportMapper<'a> { } None }); - req = req.or_else(|| jsr_resolver.lookup_req_for_nv(&nv)); + req = req.or_else(|| self.resolver.jsr_lookup_req_for_nv(&nv)); let spec_str = if let Some(req) = req { let req_ref = PackageReqReference { req, sub_path }; JsrPackageReqReference::new(req_ref).to_string() diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index 87bb72d1e53d7a..1825a97a42c42b 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -1591,7 +1591,7 @@ mod tests { location.to_path_buf(), RealDenoCacheEnv, )); - let mut documents = Documents::new(cache); + let mut documents = Documents::new(cache.clone()); for (specifier, source, version, language_id) in fixtures { let specifier = resolve_url(specifier).expect("failed to create specifier"); @@ -1614,7 +1614,7 @@ mod tests { config.tree.inject_config_file(config_file).await; } let resolver = LspResolver::default() - .with_new_config(&config, None, None) + .with_new_config(&config, cache, None, None) .await; StateSnapshot { project_version: 0, diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 71cc63f836065f..60b2385cb584d1 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -12,7 +12,6 @@ use super::tsc::AssetDocument; use crate::cache::HttpCache; use crate::graph_util::CliJsrUrlProvider; -use crate::jsr::JsrCacheResolver; use crate::lsp::logging::lsp_warn; use crate::resolver::SloppyImportsFsEntry; use crate::resolver::SloppyImportsResolution; @@ -32,7 +31,6 @@ use deno_core::futures::FutureExt; use deno_core::parking_lot::Mutex; use deno_core::ModuleSpecifier; use deno_graph::source::ResolutionMode; -use deno_graph::GraphImport; use deno_graph::Resolution; use deno_lockfile::Lockfile; use deno_runtime::deno_node; @@ -716,64 +714,6 @@ pub fn to_lsp_range(range: &deno_graph::Range) -> lsp::Range { } } -#[derive(Debug)] -struct RedirectResolver { - cache: Arc, - redirects: Mutex>, -} - -impl RedirectResolver { - pub fn new(cache: Arc) -> Self { - Self { - cache, - redirects: Mutex::new(HashMap::new()), - } - } - - pub fn resolve( - &self, - specifier: &ModuleSpecifier, - ) -> Option { - let scheme = specifier.scheme(); - if !DOCUMENT_SCHEMES.contains(&scheme) { - return None; - } - - if scheme == "http" || scheme == "https" { - let mut redirects = self.redirects.lock(); - if let Some(specifier) = redirects.get(specifier) { - Some(specifier.clone()) - } else { - let redirect = self.resolve_remote(specifier, 10)?; - redirects.insert(specifier.clone(), redirect.clone()); - Some(redirect) - } - } else { - Some(specifier.clone()) - } - } - - fn resolve_remote( - &self, - specifier: &ModuleSpecifier, - redirect_limit: usize, - ) -> Option { - if redirect_limit > 0 { - let cache_key = self.cache.cache_item_key(specifier).ok()?; - let headers = self.cache.read_headers(&cache_key).ok().flatten()?; - if let Some(location) = headers.get("location") { - let redirect = - deno_core::resolve_import(location, specifier.as_str()).ok()?; - self.resolve_remote(&redirect, redirect_limit - 1) - } else { - Some(specifier.clone()) - } - } else { - None - } - } -} - #[derive(Debug, Default)] struct FileSystemDocuments { docs: DashMap>, @@ -918,21 +858,15 @@ pub struct Documents { open_docs: HashMap>, /// Documents stored on the file system. file_system_docs: Arc, - /// Any imports to the context supplied by configuration files. This is like - /// the imports into the a module graph in CLI. - imports: Arc>, /// A resolver that takes into account currently loaded import map and JSX /// settings. resolver: Arc, - jsr_resolver: Arc, lockfile: Option>>, /// The npm package requirements found in npm specifiers. npm_specifier_reqs: Arc>, /// Gets if any document had a node: specifier such that a @types/node package /// should be injected. has_injected_types_node_package: bool, - /// Resolves a specifier to its final redirected to specifier. - redirect_resolver: Arc, /// If --unstable-sloppy-imports is enabled. unstable_sloppy_imports: bool, } @@ -945,29 +879,14 @@ impl Documents { dirty: true, open_docs: HashMap::default(), file_system_docs: Default::default(), - imports: Default::default(), resolver: Default::default(), - jsr_resolver: Arc::new(JsrCacheResolver::new(cache.clone(), None)), lockfile: None, npm_specifier_reqs: Default::default(), has_injected_types_node_package: false, - redirect_resolver: Arc::new(RedirectResolver::new(cache)), unstable_sloppy_imports: false, } } - pub fn initialize(&mut self, config: &Config) { - self.config = Arc::new(config.clone()); - } - - pub fn module_graph_imports(&self) -> impl Iterator { - self - .imports - .values() - .flat_map(|i| i.dependencies.values()) - .flat_map(|value| value.get_type().or_else(|| value.get_code())) - } - /// "Open" a document from the perspective of the editor, meaning that /// requests for information from the document will come from the in-memory /// representation received from the language server client, versus reading @@ -1102,11 +1021,14 @@ impl Documents { let specifier = if let Ok(jsr_req_ref) = JsrPackageReqReference::from_specifier(specifier) { - Cow::Owned(self.jsr_resolver.jsr_to_registry_url(&jsr_req_ref)?) + Cow::Owned(self.resolver.jsr_to_registry_url(&jsr_req_ref)?) } else { Cow::Borrowed(specifier) }; - self.redirect_resolver.resolve(&specifier) + if !DOCUMENT_SCHEMES.contains(&specifier.scheme()) { + return None; + } + self.resolver.resolve_redirects(&specifier) } } @@ -1279,7 +1201,8 @@ impl Documents { results.push(None); } } else if let Some(specifier) = self - .resolve_imports_dependency(specifier) + .resolver + .resolve_graph_import(specifier) .and_then(|r| r.maybe_specifier()) { results.push(self.resolve_dependency(specifier, referrer)); @@ -1308,62 +1231,19 @@ impl Documents { results } - /// Update the location of the on disk cache for the document store. - pub fn set_cache(&mut self, cache: Arc) { - // TODO update resolved dependencies? - self.cache = cache.clone(); - self.redirect_resolver = Arc::new(RedirectResolver::new(cache)); - self.dirty = true; - } - - pub fn get_jsr_resolver(&self) -> &Arc { - &self.jsr_resolver - } - - pub fn refresh_lockfile(&mut self, lockfile: Option>>) { - self.jsr_resolver = - Arc::new(JsrCacheResolver::new(self.cache.clone(), lockfile.clone())); - self.lockfile = lockfile; - } - pub fn update_config( &mut self, config: &Config, resolver: &Arc, + cache: Arc, workspace_files: &BTreeSet, ) { self.config = Arc::new(config.clone()); + self.cache = cache; let config_data = config.tree.root_data(); let config_file = config_data.and_then(|d| d.config_file.as_deref()); self.resolver = resolver.clone(); - self.jsr_resolver = Arc::new(JsrCacheResolver::new( - self.cache.clone(), - config.tree.root_lockfile().cloned(), - )); self.lockfile = config.tree.root_lockfile().cloned(); - self.redirect_resolver = - Arc::new(RedirectResolver::new(self.cache.clone())); - let graph_resolver = self.resolver.as_graph_resolver(); - let npm_resolver = self.resolver.as_graph_npm_resolver(); - self.imports = Arc::new( - if let Some(Ok(imports)) = config_file.map(|cf| cf.to_maybe_imports()) { - imports - .into_iter() - .map(|(referrer, imports)| { - let graph_import = GraphImport::new( - &referrer, - imports, - &CliJsrUrlProvider, - Some(graph_resolver), - Some(npm_resolver), - ); - (referrer, graph_import) - }) - .collect() - } else { - IndexMap::new() - }, - ); self.unstable_sloppy_imports = config_file .map(|c| c.has_unstable("sloppy-imports")) .unwrap_or(false); @@ -1516,19 +1396,6 @@ impl Documents { Some((doc.specifier().clone(), media_type)) } } - - /// Iterate through any "imported" modules, checking to see if a dependency - /// is available. This is used to provide "global" imports like the JSX import - /// source. - fn resolve_imports_dependency(&self, specifier: &str) -> Option<&Resolution> { - for graph_imports in self.imports.values() { - let maybe_dep = graph_imports.dependencies.get(specifier); - if maybe_dep.is_some() { - return maybe_dep.map(|d| &d.maybe_type); - } - } - None - } } fn node_resolve_npm_req_ref( @@ -1702,20 +1569,20 @@ mod tests { use test_util::PathRef; use test_util::TempDir; - fn setup(temp_dir: &TempDir) -> (Documents, PathRef) { + fn setup(temp_dir: &TempDir) -> (Documents, PathRef, Arc) { let location = temp_dir.path().join("deps"); let cache = Arc::new(GlobalHttpCache::new( location.to_path_buf(), RealDenoCacheEnv, )); - let documents = Documents::new(cache); - (documents, location) + let documents = Documents::new(cache.clone()); + (documents, location, cache) } #[test] fn test_documents_open_close() { let temp_dir = TempDir::new(); - let (mut documents, _) = setup(&temp_dir); + let (mut documents, _, _) = setup(&temp_dir); let specifier = ModuleSpecifier::parse("file:///a.ts").unwrap(); let content = r#"import * as b from "./b.ts"; console.log(b); @@ -1741,7 +1608,7 @@ console.log(b); #[test] fn test_documents_change() { let temp_dir = TempDir::new(); - let (mut documents, _) = setup(&temp_dir); + let (mut documents, _, _) = setup(&temp_dir); let specifier = ModuleSpecifier::parse("file:///a.ts").unwrap(); let content = r#"import * as b from "./b.ts"; console.log(b); @@ -1785,7 +1652,7 @@ console.log(b, "hello deno"); // it should never happen that a user of this API causes this to happen, // but we'll guard against it anyway let temp_dir = TempDir::new(); - let (mut documents, documents_path) = setup(&temp_dir); + let (mut documents, documents_path, _) = setup(&temp_dir); let file_path = documents_path.join("file.ts"); let file_specifier = ModuleSpecifier::from_file_path(&file_path).unwrap(); documents_path.create_dir_all(); @@ -1813,7 +1680,7 @@ console.log(b, "hello deno"); // it should never happen that a user of this API causes this to happen, // but we'll guard against it anyway let temp_dir = TempDir::new(); - let (mut documents, documents_path) = setup(&temp_dir); + let (mut documents, documents_path, cache) = setup(&temp_dir); fs::create_dir_all(&documents_path).unwrap(); let file1_path = documents_path.join("file1.ts"); @@ -1862,9 +1729,14 @@ console.log(b, "hello deno"); .await; let resolver = LspResolver::default() - .with_new_config(&config, None, None) + .with_new_config(&config, cache.clone(), None, None) .await; - documents.update_config(&config, &resolver, &workspace_files); + documents.update_config( + &config, + &resolver, + cache.clone(), + &workspace_files, + ); // open the document let document = documents.open( @@ -1906,9 +1778,9 @@ console.log(b, "hello deno"); .await; let resolver = LspResolver::default() - .with_new_config(&config, None, None) + .with_new_config(&config, cache.clone(), None, None) .await; - documents.update_config(&config, &resolver, &workspace_files); + documents.update_config(&config, &resolver, cache, &workspace_files); // check the document's dependencies let document = documents.get(&file1_specifier).unwrap(); diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 07d3d8cb78f82d..3a378d0ea404e7 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -312,9 +312,8 @@ impl LanguageServer { .show_message(MessageType::WARNING, err); } let mut inner = self.0.write().await; - let lockfile = inner.config.tree.root_lockfile().cloned(); - inner.documents.refresh_lockfile(lockfile); - inner.refresh_npm_specifiers().await; + inner.refresh_resolver().await; + inner.refresh_documents_config().await; inner.post_cache(result.mark).await; } Ok(Some(json!(true))) @@ -687,7 +686,6 @@ impl Inner { .map(|c| c as Arc) .unwrap_or(global_cache); self.deps_http_cache = cache.clone(); - self.documents.set_cache(cache.clone()); self.cache_metadata.set_cache(cache); self.url_map.set_cache(maybe_local_cache); self.maybe_global_cache_path = new_cache_path; @@ -810,8 +808,6 @@ impl Inner { self.config.update_capabilities(¶ms.capabilities); } - self.documents.initialize(&self.config); - if let Err(e) = self .ts_server .start(self.config.internal_inspect().to_address()) @@ -1016,10 +1012,14 @@ impl Inner { } } } + } + + async fn refresh_resolver(&mut self) { self.resolver = self .resolver .with_new_config( &self.config, + self.deps_http_cache.clone(), self.maybe_global_cache_path.as_deref(), Some(&self.http_client), ) @@ -1030,6 +1030,7 @@ impl Inner { self.documents.update_config( &self.config, &self.resolver, + self.deps_http_cache.clone(), &self.workspace_files, ); @@ -1169,6 +1170,7 @@ impl Inner { lsp_warn!("Error updating registries: {:#}", err); self.client.show_message(MessageType::WARNING, err); } + self.refresh_resolver().await; self.refresh_documents_config().await; self.diagnostics_server.invalidate_all(); self.send_diagnostics_update(); @@ -1217,6 +1219,7 @@ impl Inner { self.workspace_files_hash = 0; self.refresh_workspace_files(); self.refresh_config_tree().await; + self.refresh_resolver().await; deno_config_changes.extend(changes.iter().filter_map(|(s, e)| { self.config.tree.watched_file_type(s).and_then(|t| { let configuration_type = match t.1 { @@ -1518,10 +1521,7 @@ impl Inner { if let Ok(jsr_req_ref) = JsrPackageReqReference::from_specifier(specifier) { - if let Some(url) = self - .documents - .get_jsr_resolver() - .jsr_to_registry_url(&jsr_req_ref) + if let Some(url) = self.resolver.jsr_to_registry_url(&jsr_req_ref) { result = format!("{result} (<{url}>)"); } @@ -2991,6 +2991,7 @@ impl tower_lsp::LanguageServer for LanguageServer { { let mut ls = self.0.write().await; init_log_file(ls.config.log_file()); + ls.refresh_resolver().await; ls.refresh_documents_config().await; ls.diagnostics_server.invalidate_all(); ls.send_diagnostics_update(); @@ -3125,6 +3126,7 @@ impl tower_lsp::LanguageServer for LanguageServer { let mut ls = self.0.write().await; ls.refresh_workspace_files(); ls.refresh_config_tree().await; + ls.refresh_resolver().await; ls.refresh_documents_config().await; ls.diagnostics_server.invalidate_all(); ls.send_diagnostics_update(); diff --git a/cli/lsp/resolver.rs b/cli/lsp/resolver.rs index 076d48bb4d977e..1aa8830722e431 100644 --- a/cli/lsp/resolver.rs +++ b/cli/lsp/resolver.rs @@ -4,7 +4,9 @@ use crate::args::package_json; use crate::args::CacheSetting; use crate::cache::DenoDir; use crate::cache::FastInsecureHasher; +use crate::graph_util::CliJsrUrlProvider; use crate::http_util::HttpClient; +use crate::jsr::JsrCacheResolver; use crate::lsp::config::Config; use crate::lsp::config::ConfigData; use crate::lsp::logging::lsp_warn; @@ -21,10 +23,14 @@ use crate::resolver::CliGraphResolverOptions; use crate::resolver::CliNodeResolver; use crate::util::progress_bar::ProgressBar; use crate::util::progress_bar::ProgressBarStyle; +use deno_cache_dir::HttpCache; use deno_core::error::AnyError; +use deno_core::parking_lot::Mutex; use deno_graph::source::NpmResolver; use deno_graph::source::Resolver; +use deno_graph::GraphImport; use deno_graph::ModuleSpecifier; +use deno_graph::Resolution; use deno_npm::NpmSystemInfo; use deno_runtime::deno_fs; use deno_runtime::deno_node::NodeResolution; @@ -33,9 +39,13 @@ use deno_runtime::deno_node::NodeResolver; use deno_runtime::deno_node::PackageJson; use deno_runtime::fs_util::specifier_to_file_path; use deno_runtime::permissions::PermissionsContainer; +use deno_semver::jsr::JsrPackageReqReference; use deno_semver::npm::NpmPackageReqReference; +use deno_semver::package::PackageNv; use deno_semver::package::PackageReq; +use indexmap::IndexMap; use package_json::PackageJsonDepsProvider; +use std::collections::HashMap; use std::path::Path; use std::rc::Rc; use std::sync::Arc; @@ -43,19 +53,25 @@ use std::sync::Arc; #[derive(Debug, Clone)] pub struct LspResolver { graph_resolver: Arc, + jsr_resolver: Option>, npm_resolver: Option>, node_resolver: Option>, npm_config_hash: LspNpmConfigHash, + redirect_resolver: Option>, + graph_imports: Arc>, config: Arc, } impl Default for LspResolver { fn default() -> Self { Self { - graph_resolver: create_graph_resolver(&Default::default(), None, None), + graph_resolver: create_graph_resolver(None, None, None), + jsr_resolver: None, npm_resolver: None, node_resolver: None, npm_config_hash: LspNpmConfigHash(0), + redirect_resolver: None, + graph_imports: Default::default(), config: Default::default(), } } @@ -65,15 +81,16 @@ impl LspResolver { pub async fn with_new_config( &self, config: &Config, + cache: Arc, global_cache_path: Option<&Path>, http_client: Option<&Arc>, ) -> Arc { let npm_config_hash = LspNpmConfigHash::new(config, global_cache_path); + let config_data = config.tree.root_data(); let mut npm_resolver = None; let mut node_resolver = None; if npm_config_hash != self.npm_config_hash { - if let (Some(http_client), Some(config_data)) = - (http_client, config.tree.root_data()) + if let (Some(http_client), Some(config_data)) = (http_client, config_data) { npm_resolver = create_npm_resolver(config_data, global_cache_path, http_client) @@ -85,15 +102,44 @@ impl LspResolver { node_resolver = self.node_resolver.clone(); } let graph_resolver = create_graph_resolver( - config, + config_data, npm_resolver.as_ref(), node_resolver.as_ref(), ); + let jsr_resolver = Some(Arc::new(JsrCacheResolver::new( + cache.clone(), + config_data.and_then(|d| d.lockfile.clone()), + ))); + let redirect_resolver = Some(Arc::new(RedirectResolver::new(cache))); + let graph_imports = config_data + .and_then(|d| d.config_file.as_ref()) + .and_then(|cf| cf.to_maybe_imports().ok()) + .map(|imports| { + Arc::new( + imports + .into_iter() + .map(|(referrer, imports)| { + let graph_import = GraphImport::new( + &referrer, + imports, + &CliJsrUrlProvider, + Some(graph_resolver.as_ref()), + Some(graph_resolver.as_ref()), + ); + (referrer, graph_import) + }) + .collect(), + ) + }) + .unwrap_or_default(); Arc::new(Self { graph_resolver, + jsr_resolver, npm_resolver, node_resolver, npm_config_hash, + redirect_resolver, + graph_imports, config: Arc::new(config.clone()), }) } @@ -103,15 +149,18 @@ impl LspResolver { self.npm_resolver.as_ref().map(|r| r.clone_snapshotted()); let node_resolver = create_node_resolver(npm_resolver.as_ref()); let graph_resolver = create_graph_resolver( - &self.config, + self.config.tree.root_data(), npm_resolver.as_ref(), node_resolver.as_ref(), ); Arc::new(Self { graph_resolver, + jsr_resolver: self.jsr_resolver.clone(), npm_resolver, node_resolver, npm_config_hash: self.npm_config_hash.clone(), + redirect_resolver: self.redirect_resolver.clone(), + graph_imports: self.graph_imports.clone(), config: self.config.clone(), }) } @@ -136,10 +185,49 @@ impl LspResolver { self.graph_resolver.as_ref() } + pub fn jsr_to_registry_url( + &self, + req_ref: &JsrPackageReqReference, + ) -> Option { + self.jsr_resolver.as_ref()?.jsr_to_registry_url(req_ref) + } + + pub fn jsr_lookup_export_for_path( + &self, + nv: &PackageNv, + path: &str, + ) -> Option { + self.jsr_resolver.as_ref()?.lookup_export_for_path(nv, path) + } + + pub fn jsr_lookup_req_for_nv(&self, nv: &PackageNv) -> Option { + self.jsr_resolver.as_ref()?.lookup_req_for_nv(nv) + } + pub fn maybe_managed_npm_resolver(&self) -> Option<&ManagedCliNpmResolver> { self.npm_resolver.as_ref().and_then(|r| r.as_managed()) } + pub fn graph_import_specifiers( + &self, + ) -> impl Iterator { + self + .graph_imports + .values() + .flat_map(|i| i.dependencies.values()) + .flat_map(|value| value.get_type().or_else(|| value.get_code())) + } + + pub fn resolve_graph_import(&self, specifier: &str) -> Option<&Resolution> { + for graph_imports in self.graph_imports.values() { + let maybe_dep = graph_imports.dependencies.get(specifier); + if maybe_dep.is_some() { + return maybe_dep.map(|d| &d.maybe_type); + } + } + None + } + pub fn in_npm_package(&self, specifier: &ModuleSpecifier) -> bool { if let Some(npm_resolver) = &self.npm_resolver { return npm_resolver.in_npm_package(specifier); @@ -203,6 +291,16 @@ impl LspResolver { node_resolver .get_closest_package_json(referrer, &PermissionsContainer::allow_all()) } + + pub fn resolve_redirects( + &self, + specifier: &ModuleSpecifier, + ) -> Option { + let Some(redirect_resolver) = self.redirect_resolver.as_ref() else { + return Some(specifier.clone()); + }; + redirect_resolver.resolve(specifier) + } } async fn create_npm_resolver( @@ -275,11 +373,10 @@ fn create_node_resolver( } fn create_graph_resolver( - config: &Config, + config_data: Option<&ConfigData>, npm_resolver: Option<&Arc>, node_resolver: Option<&Arc>, ) -> Arc { - let config_data = config.tree.root_data(); let config_file = config_data.and_then(|d| d.config_file.as_deref()); Arc::new(CliGraphResolver::new(CliGraphResolverOptions { node_resolver: node_resolver.cloned(), @@ -296,7 +393,7 @@ fn create_graph_resolver( maybe_import_map: config_data.and_then(|d| d.import_map.clone()), maybe_vendor_dir: config_data.and_then(|d| d.vendor_dir.as_ref()), bare_node_builtins_enabled: config_file - .map(|config| config.has_unstable("bare-node-builtins")) + .map(|cf| cf.has_unstable("bare-node-builtins")) .unwrap_or(false), // Don't set this for the LSP because instead we'll use the OpenDocumentsLoader // because it's much easier and we get diagnostics/quick fixes about a redirected @@ -326,3 +423,56 @@ impl LspNpmConfigHash { Self(hasher.finish()) } } + +#[derive(Debug)] +struct RedirectResolver { + cache: Arc, + redirects: Mutex>, +} + +impl RedirectResolver { + pub fn new(cache: Arc) -> Self { + Self { + cache, + redirects: Mutex::new(HashMap::new()), + } + } + + pub fn resolve( + &self, + specifier: &ModuleSpecifier, + ) -> Option { + if matches!(specifier.scheme(), "http" | "https") { + let mut redirects = self.redirects.lock(); + if let Some(specifier) = redirects.get(specifier) { + Some(specifier.clone()) + } else { + let redirect = self.resolve_remote(specifier, 10)?; + redirects.insert(specifier.clone(), redirect.clone()); + Some(redirect) + } + } else { + Some(specifier.clone()) + } + } + + fn resolve_remote( + &self, + specifier: &ModuleSpecifier, + redirect_limit: usize, + ) -> Option { + if redirect_limit > 0 { + let cache_key = self.cache.cache_item_key(specifier).ok()?; + let headers = self.cache.read_headers(&cache_key).ok().flatten()?; + if let Some(location) = headers.get("location") { + let redirect = + deno_core::resolve_import(location, specifier.as_str()).ok()?; + self.resolve_remote(&redirect, redirect_limit - 1) + } else { + Some(specifier.clone()) + } + } else { + None + } + } +} diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 8a20ffb0f7b900..06f618e1fb20c7 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -4124,9 +4124,9 @@ fn op_script_names(state: &mut OpState) -> Vec { } // inject these next because they're global - for import in documents.module_graph_imports() { - if seen.insert(import.as_str()) { - result.push(import.to_string()); + for specifier in state.state_snapshot.resolver.graph_import_specifiers() { + if seen.insert(specifier.as_str()) { + result.push(specifier.to_string()); } } @@ -5095,7 +5095,7 @@ mod tests { ) .await; let resolver = LspResolver::default() - .with_new_config(&config, None, None) + .with_new_config(&config, cache.clone(), None, None) .await; StateSnapshot { project_version: 0, From 850331164138233ea86c9a5032d99ad29c79efe1 Mon Sep 17 00:00:00 2001 From: Satya Rohith Date: Tue, 30 Apr 2024 07:49:29 +0530 Subject: [PATCH 3/5] fix(ext/node): read(0) before destroying http2stream (#23505) This patch enables gRPC hello world client example to work. Towards #23246 #3326 --- ext/node/polyfills/http2.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/ext/node/polyfills/http2.ts b/ext/node/polyfills/http2.ts index 02e66e3da3e55d..bc8347469737df 100644 --- a/ext/node/polyfills/http2.ts +++ b/ext/node/polyfills/http2.ts @@ -1019,6 +1019,7 @@ export class ClientHttp2Stream extends Duplex { core.tryClose(this[kDenoResponse].bodyRid); this.push(null); debugHttp2(">>> read null chunk"); + this.read(0); this[kMaybeDestroy](); return; } From ce4a7773ccb606c51c824881e37043bc937c94f2 Mon Sep 17 00:00:00 2001 From: findmyhappy <167661649+findmyhappy@users.noreply.github.com> Date: Tue, 30 Apr 2024 15:59:56 +0800 Subject: [PATCH 4/5] docs: fix some typos in comments (#23520) --- cli/lsp/tsc.rs | 2 +- ext/net/tcp.rs | 2 +- ext/webgpu/01_webgpu.js | 2 +- tools/copyright_checker.js | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 06f618e1fb20c7..28127c6d756e0b 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -3176,7 +3176,7 @@ fn get_parameters_from_parts(parts: &[SymbolDisplayPart]) -> Vec { break; } } else if part.text == "..." && paren_count == 1 { - // Found rest parmeter. Do not fill in any further arguments. + // Found rest parameter. Do not fill in any further arguments. break; } else if part.text == "{" { brace_count += 1; diff --git a/ext/net/tcp.rs b/ext/net/tcp.rs index 58362024333b92..63baa8e4befdc6 100644 --- a/ext/net/tcp.rs +++ b/ext/net/tcp.rs @@ -7,7 +7,7 @@ use socket2::Domain; use socket2::Protocol; use socket2::Type; -/// Our per-process `Connections`. We can use this to find an existant listener for +/// Our per-process `Connections`. We can use this to find an existent listener for /// a given local address and clone its socket for us to listen on in our thread. static CONNS: std::sync::OnceLock> = std::sync::OnceLock::new(); diff --git a/ext/webgpu/01_webgpu.js b/ext/webgpu/01_webgpu.js index b8b13ebf80f49c..d3e9bf4e38a097 100644 --- a/ext/webgpu/01_webgpu.js +++ b/ext/webgpu/01_webgpu.js @@ -193,7 +193,7 @@ function assertDeviceMatch( const resourceDevice = assertDevice(resource, prefix, resourceContext); if (resourceDevice.rid !== self.rid) { throw new DOMException( - `${prefix}: ${resourceContext} belongs to a diffent device than ${selfContext}.`, + `${prefix}: ${resourceContext} belongs to a different device than ${selfContext}.`, "OperationError", ); } diff --git a/tools/copyright_checker.js b/tools/copyright_checker.js index 86289b9d4589ce..73c5e6a712517d 100644 --- a/tools/copyright_checker.js +++ b/tools/copyright_checker.js @@ -71,7 +71,7 @@ export async function checkCopyright() { !fileText.startsWith(C_STYLE_COPYRIGHT_LINE) ) { let trimmedText = fileText; - // Attempt to trim accceptable lines + // Attempt to trim acceptable lines while ( ACCEPTABLE_LINES.test(trimmedText) && !trimmedText.startsWith(C_STYLE_COPYRIGHT_LINE) From 0156f82e7334f9ef31ff60b579baa4ba10bcd226 Mon Sep 17 00:00:00 2001 From: Satya Rohith Date: Tue, 30 Apr 2024 16:59:39 +0530 Subject: [PATCH 5/5] fix(ext/node): support multiple message listeners on MessagePort (#23600) Closes https://github.com/denoland/deno/issues/23561 --- ext/node/polyfills/worker_threads.ts | 12 ++++++++++-- tests/unit_node/worker_threads_test.ts | 21 +++++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/ext/node/polyfills/worker_threads.ts b/ext/node/polyfills/worker_threads.ts index e88e6368b55358..71999dd6244307 100644 --- a/ext/node/polyfills/worker_threads.ts +++ b/ext/node/polyfills/worker_threads.ts @@ -484,9 +484,17 @@ function webMessagePortToNodeMessagePort(port: MessagePort) { // deno-lint-ignore no-explicit-any const _listener = (ev: any) => listener(ev.data); if (name == "message") { - port.onmessage = _listener; + if (port.onmessage === null) { + port.onmessage = _listener; + } else { + port.addEventListener("message", _listener); + } } else if (name == "messageerror") { - port.onmessageerror = _listener; + if (port.onmessageerror === null) { + port.onmessageerror = _listener; + } else { + port.addEventListener("messageerror", _listener); + } } else if (name == "close") { port.addEventListener("close", _listener); } else { diff --git a/tests/unit_node/worker_threads_test.ts b/tests/unit_node/worker_threads_test.ts index bc2becd66664d9..f46d982fe643c4 100644 --- a/tests/unit_node/worker_threads_test.ts +++ b/tests/unit_node/worker_threads_test.ts @@ -515,3 +515,24 @@ Deno.test({ await worker.terminate(); }, }); + +Deno.test({ + name: + "[node/worker_threads] MessagePort.on all message listeners are invoked", + async fn() { + const output: string[] = []; + const deferred = Promise.withResolvers(); + const { port1, port2 } = new workerThreads.MessageChannel(); + port1.on("message", (msg) => output.push(msg)); + port1.on("message", (msg) => output.push(msg + 2)); + port1.on("message", (msg) => { + output.push(msg + 3); + deferred.resolve(); + }); + port2.postMessage("hi!"); + await deferred.promise; + assertEquals(output, ["hi!", "hi!2", "hi!3"]); + port2.close(); + port1.close(); + }, +});