-
Notifications
You must be signed in to change notification settings - Fork 5.7k
feat(unstable/npm): initial type checking of npm specifiers #16332
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
1490d8d
e8c83b4
38e8ec7
1a074dd
140a24e
cf489f7
90e3b5f
58cccc7
784631b
04de5e8
597f81c
e72ff33
c0a0a0d
4cf33c0
18c4447
fcbb624
25f4ed8
02e64f0
0612b09
6689c5d
8e73280
40e56cb
e0620eb
cc53537
ceebdf1
3806a15
b2414d0
468114d
8027ea5
9193fd0
af4248c
7e0cc47
cffcfd9
03d7acd
29cd1ea
b12d21a
3815e65
058bee4
b20b4ca
d3cce89
0e65153
4750263
1781aed
4718557
9385717
d6dca27
29f4263
abe24d6
dde97ac
2fc715b
6fa290b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -458,6 +458,13 @@ async fn generate_lint_diagnostics( | |
break; | ||
} | ||
|
||
// ignore any npm package files | ||
if let Some(npm_resolver) = &snapshot.maybe_npm_resolver { | ||
if npm_resolver.in_npm_package(document.specifier()) { | ||
continue; | ||
} | ||
} | ||
|
||
let version = document.maybe_lsp_version(); | ||
diagnostics_vec.push(( | ||
document.specifier().clone(), | ||
|
@@ -597,6 +604,8 @@ pub enum DenoDiagnostic { | |
NoCacheBlob, | ||
/// A data module was not found in the cache. | ||
NoCacheData(ModuleSpecifier), | ||
/// A remote npm package reference was not found in the cache. | ||
NoCacheNpm(NpmPackageReference, ModuleSpecifier), | ||
/// A local module was not found on the local file system. | ||
NoLocal(ModuleSpecifier), | ||
/// The specifier resolved to a remote specifier that was redirected to | ||
|
@@ -622,6 +631,7 @@ impl DenoDiagnostic { | |
Self::NoCache(_) => "no-cache", | ||
Self::NoCacheBlob => "no-cache-blob", | ||
Self::NoCacheData(_) => "no-cache-data", | ||
Self::NoCacheNpm(_, _) => "no-cache-npm", | ||
Self::NoLocal(_) => "no-local", | ||
Self::Redirect { .. } => "redirect", | ||
Self::ResolutionError(err) => match err { | ||
|
@@ -690,16 +700,17 @@ impl DenoDiagnostic { | |
}), | ||
..Default::default() | ||
}, | ||
"no-cache" | "no-cache-data" => { | ||
"no-cache" | "no-cache-data" | "no-cache-npm" => { | ||
let data = diagnostic | ||
.data | ||
.clone() | ||
.ok_or_else(|| anyhow!("Diagnostic is missing data"))?; | ||
let data: DiagnosticDataSpecifier = serde_json::from_value(data)?; | ||
let title = if code == "no-cache" { | ||
format!("Cache \"{}\" and its dependencies.", data.specifier) | ||
} else { | ||
"Cache the data URL and its dependencies.".to_string() | ||
let title = match code.as_str() { | ||
"no-cache" | "no-cache-npm" => { | ||
format!("Cache \"{}\" and its dependencies.", data.specifier) | ||
} | ||
_ => "Cache the data URL and its dependencies.".to_string(), | ||
}; | ||
lsp::CodeAction { | ||
title, | ||
|
@@ -757,6 +768,7 @@ impl DenoDiagnostic { | |
code.as_str(), | ||
"import-map-remap" | ||
| "no-cache" | ||
| "no-cache-npm" | ||
| "no-cache-data" | ||
| "no-assert-type" | ||
| "redirect" | ||
|
@@ -777,6 +789,7 @@ impl DenoDiagnostic { | |
Self::NoCache(specifier) => (lsp::DiagnosticSeverity::ERROR, format!("Uncached or missing remote URL: \"{}\".", specifier), Some(json!({ "specifier": specifier }))), | ||
Self::NoCacheBlob => (lsp::DiagnosticSeverity::ERROR, "Uncached blob URL.".to_string(), None), | ||
Self::NoCacheData(specifier) => (lsp::DiagnosticSeverity::ERROR, "Uncached data URL.".to_string(), Some(json!({ "specifier": specifier }))), | ||
Self::NoCacheNpm(pkg_ref, specifier) => (lsp::DiagnosticSeverity::ERROR, format!("Uncached or missing npm package: \"{}\".", pkg_ref.req), Some(json!({ "specifier": specifier }))), | ||
Self::NoLocal(specifier) => (lsp::DiagnosticSeverity::ERROR, format!("Unable to load a local module: \"{}\".\n Please check the file path.", specifier), None), | ||
Self::Redirect { from, to} => (lsp::DiagnosticSeverity::INFORMATION, format!("The import of \"{}\" was redirected to \"{}\".", from, to), Some(json!({ "specifier": from, "redirect": to }))), | ||
Self::ResolutionError(err) => (lsp::DiagnosticSeverity::ERROR, err.to_string(), None), | ||
|
@@ -847,8 +860,20 @@ fn diagnose_resolved( | |
.push(DenoDiagnostic::NoAssertType.to_lsp_diagnostic(&range)), | ||
} | ||
} | ||
} else if NpmPackageReference::from_specifier(specifier).is_ok() { | ||
// ignore npm specifiers for now | ||
} else if let Ok(pkg_ref) = NpmPackageReference::from_specifier(specifier) | ||
{ | ||
if let Some(npm_resolver) = &snapshot.maybe_npm_resolver { | ||
// show diagnostics for npm package references that aren't cached | ||
if npm_resolver | ||
.resolve_package_folder_from_deno_module(&pkg_ref.req) | ||
.is_err() | ||
Comment on lines
+865
to
+869
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That this handle situation when user has There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nope. That's not supported at the moment because there's no way to provide that to the lsp at the moment (see second point in pr description). Edit: opened an issue. |
||
{ | ||
diagnostics.push( | ||
DenoDiagnostic::NoCacheNpm(pkg_ref, specifier.clone()) | ||
.to_lsp_diagnostic(&range), | ||
); | ||
} | ||
} | ||
} else { | ||
// When the document is not available, it means that it cannot be found | ||
// in the cache or locally on the disk, so we want to issue a diagnostic | ||
|
@@ -882,6 +907,12 @@ fn diagnose_dependency( | |
dependency_key: &str, | ||
dependency: &deno_graph::Dependency, | ||
) { | ||
if let Some(npm_resolver) = &snapshot.maybe_npm_resolver { | ||
if npm_resolver.in_npm_package(referrer) { | ||
return; // ignore, surface typescript errors instead | ||
} | ||
} | ||
|
||
if let Some(import_map) = &snapshot.maybe_import_map { | ||
if let Resolved::Ok { | ||
specifier, range, .. | ||
|
@@ -938,8 +969,8 @@ async fn generate_deno_diagnostics( | |
&mut diagnostics, | ||
snapshot, | ||
specifier, | ||
&dependency_key, | ||
&dependency, | ||
dependency_key, | ||
dependency, | ||
); | ||
} | ||
} | ||
|
Uh oh!
There was an error while loading. Please reload this page.