Skip to content
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

feat(unstable/npm): initial type checking of npm specifiers #16332

Merged
merged 51 commits into from
Oct 21, 2022
Merged
Show file tree
Hide file tree
Changes from 39 commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
1490d8d
Saving work. Going to separate this into multiple prs.
dsherret Sep 28, 2022
e8c83b4
Updates.
dsherret Sep 28, 2022
38e8ec7
Updates.
dsherret Sep 28, 2022
1a074dd
Merge branch 'main' into npm_types_01
dsherret Sep 28, 2022
140a24e
Moar.
dsherret Sep 28, 2022
cf489f7
Committing for backup.
dsherret Sep 29, 2022
90e3b5f
.js files to .d.ts resolution
dsherret Sep 29, 2022
58cccc7
Merge branch 'main' into npm_types_01
dsherret Sep 30, 2022
784631b
Backup code. @types workingish
dsherret Oct 3, 2022
04de5e8
Support for `/// <reference types="npm:@types/node" />` and more
dsherret Oct 5, 2022
597f81c
Merge branch 'main' into npm_types_01
dsherret Oct 5, 2022
e72ff33
Committing broken work.
dsherret Oct 5, 2022
c0a0a0d
Updates.
dsherret Oct 5, 2022
4cf33c0
Fix ambient modules.
dsherret Oct 6, 2022
18c4447
Fix duplicate declaration issues.
dsherret Oct 6, 2022
fcbb624
Add tests for globals.
dsherret Oct 6, 2022
25f4ed8
Merge branch 'main' into npm_types_01
dsherret Oct 6, 2022
02e64f0
Working on importing cjs from esm with typescript
dsherret Oct 7, 2022
0612b09
Fix issue.
dsherret Oct 7, 2022
6689c5d
Merge branch 'main' into npm_types_01
dsherret Oct 17, 2022
8e73280
Format after merge.
dsherret Oct 17, 2022
40e56cb
Update.
dsherret Oct 17, 2022
e0620eb
Update with code from ts pr fixing issue
dsherret Oct 17, 2022
cc53537
More fixes.
dsherret Oct 17, 2022
ceebdf1
Only surface diagnostics in packages when using `--all`
dsherret Oct 17, 2022
3806a15
Add tests for ambient module
dsherret Oct 17, 2022
b2414d0
Fix clippy.
dsherret Oct 17, 2022
468114d
Update deno_std
dsherret Oct 17, 2022
8027ea5
Fixes.
dsherret Oct 17, 2022
9193fd0
Remove noisy diff.
dsherret Oct 17, 2022
af4248c
Update fork code...
dsherret Oct 17, 2022
7e0cc47
Merge branch 'main' into npm_types_01
dsherret Oct 17, 2022
cffcfd9
Fix test on CI.
dsherret Oct 18, 2022
03d7acd
lsp specifier tracking
dsherret Oct 18, 2022
29cd1ea
Add instructions for upgrading typescript
dsherret Oct 19, 2022
b12d21a
Quick fix for caching npm specifier.
dsherret Oct 19, 2022
3815e65
Commit broken tests because I need to fix some stuff in main
dsherret Oct 19, 2022
058bee4
fix(lsp): allow caching deps in non-saved files
dsherret Oct 19, 2022
b20b4ca
Simplify storing cjs in tsc.
dsherret Oct 19, 2022
d3cce89
Update typescript diagnostics about running `npm install @types/node`…
dsherret Oct 20, 2022
0e65153
Fix lsp issues by using a document registry
dsherret Oct 20, 2022
4750263
Merge branch 'main' into npm_types_01
dsherret Oct 20, 2022
1781aed
Update cli/dts/README.md
dsherret Oct 20, 2022
4718557
Updates based on review
dsherret Oct 20, 2022
9385717
Review updates.
dsherret Oct 20, 2022
d6dca27
Ignore lint diagnostics.
dsherret Oct 20, 2022
29f4263
Add lsp esm completion test
dsherret Oct 20, 2022
abe24d6
Warn when can't set package requirements.
dsherret Oct 20, 2022
dde97ac
Merge branch 'main' into npm_types_01
dsherret Oct 20, 2022
2fc715b
Merge branch 'main' into npm_types_01
dsherret Oct 21, 2022
6fa290b
Update to ignore diagnostics about deno:///missing_dependency.d.ts
dsherret Oct 21, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions cli/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,11 @@ fn create_compiler_snapshot(
false
}

#[op]
fn op_is_node_file() -> bool {
false
}

#[op]
fn op_script_version(
_state: &mut OpState,
Expand Down Expand Up @@ -266,6 +271,7 @@ fn create_compiler_snapshot(
op_build_info::decl(),
op_cwd::decl(),
op_exists::decl(),
op_is_node_file::decl(),
op_load::decl(),
op_script_version::decl(),
])
Expand Down
18 changes: 14 additions & 4 deletions cli/dts/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,26 @@ The files in this directory are mostly from the TypeScript repository. We
currently (unfortunately) have a rather manual process for upgrading TypeScript.
It works like this currently:

1. Checkout typescript repo in a separate directory.
2. Copy typescript.js into Deno repo.
3. Copy d.ts files into dts directory.
1. Checkout denoland/typescript repo in a separate directory.
dsherret marked this conversation as resolved.
Show resolved Hide resolved
1. Add Microsoft/TypeScript as a remote and fetch its latest tags
1. Checkout a new branch based on this tag.
1. Cherry pick the custom commit we made in a previous release to the new one.
1. This commit has a "deno.ts" file in it. Read the instructions in it.
bartlomieju marked this conversation as resolved.
Show resolved Hide resolved
1. Copy typescript.js into Deno repo.
1. Copy d.ts files into dts directory.

So that might look something like this:

```
git clone https://github.com/microsoft/TypeScript.git
git clone https://github.com/denoland/TypeScript.git
cd typescript
git remote add upstream https://github.com/Microsoft/TypeScript
git fetch upstream
git checkout v3.9.7
git checkout -b branch_v3.9.7
git cherry pick <previous-release-branch-commit-we-did>
npm install
gulp local
rsync lib/typescript.js ~/src/deno/cli/tsc/00_typescript.js
rsync --exclude=protocol.d.ts --exclude=tsserverlibrary.d.ts --exclude=typescriptServices.d.ts lib/*.d.ts ~/src/deno/cli/dts/
```
42 changes: 33 additions & 9 deletions cli/lsp/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,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
Expand All @@ -622,6 +624,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 {
Expand Down Expand Up @@ -690,16 +693,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,
Expand Down Expand Up @@ -757,6 +761,7 @@ impl DenoDiagnostic {
code.as_str(),
"import-map-remap"
| "no-cache"
| "no-cache-npm"
| "no-cache-data"
| "no-assert-type"
| "redirect"
Expand All @@ -777,6 +782,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),
Expand Down Expand Up @@ -847,8 +853,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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That this handle situation when user has --node-modules-dir flag specified?

Copy link
Member Author

@dsherret dsherret Oct 20, 2022

Choose a reason for hiding this comment

The 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
Expand Down Expand Up @@ -882,6 +900,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, ..
Expand Down Expand Up @@ -938,8 +962,8 @@ async fn generate_deno_diagnostics(
&mut diagnostics,
snapshot,
specifier,
&dependency_key,
&dependency,
dependency_key,
dependency,
);
}
}
Expand Down
Loading