Skip to content

Commit

Permalink
feat(unstable): fast subset type checking of JSR dependencies (#21873)
Browse files Browse the repository at this point in the history
  • Loading branch information
dsherret authored Jan 10, 2024
1 parent 515a34b commit 70ac061
Show file tree
Hide file tree
Showing 42 changed files with 714 additions and 149 deletions.
8 changes: 4 additions & 4 deletions Cargo.lock

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

4 changes: 2 additions & 2 deletions cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ deno_ast = { workspace = true, features = ["bundler", "cjs", "codegen", "dep_gra
deno_cache_dir = "=0.6.1"
deno_config = "=0.6.5"
deno_core = { workspace = true, features = ["include_js_files_for_snapshotting"] }
deno_doc = { version = "=0.89.0", features = ["html"] }
deno_doc = { version = "=0.89.1", features = ["html"] }
deno_emit = "=0.33.0"
deno_graph = "=0.63.0"
deno_graph = "=0.63.2"
deno_lint = { version = "=0.53.0", features = ["docs"] }
deno_lockfile.workspace = true
deno_npm = "=0.15.3"
Expand Down
13 changes: 1 addition & 12 deletions cli/args/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,6 @@ pub struct VendorFlags {

#[derive(Clone, Debug, Eq, PartialEq)]
pub struct PublishFlags {
pub directory: String,
pub token: Option<String>,
}

Expand Down Expand Up @@ -2372,19 +2371,10 @@ Remote modules and multiple modules may also be specified:
fn publish_subcommand() -> Command {
Command::new("publish")
.hide(true)
.about("Unstable preview feature: Publish a package")
.about("Unstable preview feature: Publish the current working directory's package or workspace")
// TODO: .long_about()
.defer(|cmd| {
cmd.arg(
Arg::new("directory")
.help(
"The directory to the package, or workspace of packages to publish",
)
.default_missing_value(".")
.value_hint(ValueHint::DirPath)
.required(true),
)
.arg(
Arg::new("token")
.long("token")
.help("The API token to use when publishing. If unset, interactive authentication is be used")
Expand Down Expand Up @@ -3821,7 +3811,6 @@ fn vendor_parse(flags: &mut Flags, matches: &mut ArgMatches) {

fn publish_parse(flags: &mut Flags, matches: &mut ArgMatches) {
flags.subcommand = DenoSubcommand::Publish(PublishFlags {
directory: matches.remove_one::<String>("directory").unwrap(),
token: matches.remove_one("token"),
});
}
Expand Down
8 changes: 8 additions & 0 deletions cli/tests/integration/jsr_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,14 @@ itest!(deps_info {
http_server: true,
});

itest!(subset_type_graph {
args: "check --all jsr/subset_type_graph/main.ts",
output: "jsr/subset_type_graph/main.check.out",
envs: env_vars_for_jsr_tests(),
http_server: true,
exit_code: 1,
});

itest!(version_not_found {
args: "run jsr/version_not_found/main.ts",
output: "jsr/version_not_found/main.out",
Expand Down
64 changes: 59 additions & 5 deletions cli/tests/integration/publish_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,75 @@ pub fn env_vars_for_registry() -> Vec<(String, String)> {
}

itest!(no_token {
args: "publish publish/missing_deno_json",
args: "publish",
cwd: Some("publish/missing_deno_json"),
output: "publish/no_token.out",
exit_code: 1,
});

itest!(missing_deno_json {
args: "publish --token 'sadfasdf' $TESTDATA/publish/missing_deno_json",
args: "publish --token 'sadfasdf'",
output: "publish/missing_deno_json.out",
cwd: Some("publish/missing_deno_json"),
copy_temp_dir: Some("publish/missing_deno_json"),
exit_code: 1,
temp_cwd: true,
});

itest!(invalid_fast_check {
args: "publish --token 'sadfasdf'",
output: "publish/invalid_fast_check.out",
cwd: Some("publish/invalid_fast_check"),
copy_temp_dir: Some("publish/invalid_fast_check"),
exit_code: 1,
temp_cwd: true,
});

itest!(javascript_missing_decl_file {
args: "publish --token 'sadfasdf'",
output: "publish/javascript_missing_decl_file.out",
cwd: Some("publish/javascript_missing_decl_file"),
copy_temp_dir: Some("publish/javascript_missing_decl_file"),
envs: env_vars_for_registry(),
exit_code: 0,
temp_cwd: true,
});

itest!(javascript_decl_file {
args: "publish --token 'sadfasdf'",
output: "publish/javascript_decl_file.out",
cwd: Some("publish/javascript_decl_file"),
copy_temp_dir: Some("publish/javascript_decl_file"),
envs: env_vars_for_registry(),
exit_code: 0,
temp_cwd: true,
});

itest!(successful {
args: "publish --token 'sadfasdf' $TESTDATA/publish/successful",
args: "publish --token 'sadfasdf'",
output: "publish/successful.out",
cwd: Some("publish/successful"),
copy_temp_dir: Some("publish/successful"),
envs: env_vars_for_registry(),
http_server: true,
temp_cwd: true,
});

itest!(workspace_all {
args: "publish --unstable-workspaces --token 'sadfasdf'",
output: "publish/workspace.out",
cwd: Some("publish/workspace"),
copy_temp_dir: Some("publish/workspace"),
envs: env_vars_for_registry(),
http_server: true,
temp_cwd: true,
});

itest!(workspace_individual {
args: "publish --unstable-workspaces --token 'sadfasdf'",
output: "publish/workspace_individual.out",
cwd: Some("publish/workspace/bar"),
copy_temp_dir: Some("publish/workspace"),
envs: env_vars_for_registry(),
http_server: true,
temp_cwd: true,
Expand All @@ -43,7 +97,7 @@ fn ignores_directories() {
"name": "@foo/bar",
"version": "1.0.0",
"exclude": [ "ignore" ],
"exports": "main_included.ts"
"exports": "./main_included.ts"
}));

let ignored_dirs = vec![
Expand All @@ -68,7 +122,6 @@ fn ignores_directories() {
.arg("--log-level=debug")
.arg("--token")
.arg("sadfasdf")
.arg(temp_dir)
.run();
output.assert_exit_code(0);
let output = output.combined_output();
Expand All @@ -81,4 +134,5 @@ fn publish_context_builder() -> TestContextBuilder {
TestContextBuilder::new()
.use_http_server()
.envs(env_vars_for_registry())
.use_temp_cwd()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// add some statements that will be removed by the subset
// type graph so that we can test that the source map works
console.log(1);
console.log(2);
console.log(3);

export class Foo {
method(): number {
return Math.random();
}
}

// this won't be type checked against because the subset
// type graph omit this code because it's not part of the
// public API.
const invalidTypeCheck: number = "";
console.log(invalidTypeCheck);
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"exports": {
".": "./mod.ts"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"versions": {
"0.1.0": {}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
export class Foo {
method() {
return Math.random();
}
}

// This will be analyzed because the method above is missing an
// explicit type which is required for the subset type graph to take
// effect. So the entire source file will be type checked against,
// causing a type error here.
const invalidTypeCheck: number = "";
console.log(invalidTypeCheck);
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"exports": {
".": "./mod.ts"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"versions": {
"0.1.0": {}
}
}
45 changes: 45 additions & 0 deletions cli/tests/testdata/jsr/subset_type_graph/main.check.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
Download http://localhost:4545/jsr/registry/@denotest/subset_type_graph/meta.json
Download http://localhost:4545/jsr/registry/@denotest/subset_type_graph_invalid/meta.json
Download http://localhost:4545/jsr/registry/@denotest/subset_type_graph/0.1.0_meta.json
Download http://localhost:4545/jsr/registry/@denotest/subset_type_graph_invalid/0.1.0_meta.json
[UNORDERED_START]
Download http://localhost:4545/jsr/registry/@denotest/subset_type_graph/0.1.0/mod.ts
Download http://localhost:4545/jsr/registry/@denotest/subset_type_graph_invalid/0.1.0/mod.ts
[UNORDERED_END]
Check file:///[WILDCARD]/subset_type_graph/main.ts
error: TS2322 [ERROR]: Type 'string' is not assignable to type 'number'.
const invalidTypeCheck: number = "";
~~~~~~~~~~~~~~~~
at http://localhost:4545/jsr/registry/@denotest/subset_type_graph_invalid/0.1.0/mod.ts:11:7

TS2322 [ERROR]: Type 'number' is not assignable to type 'string'.
const error1: string = new Foo1().method();
~~~~~~
at file:///[WILDCARD]/subset_type_graph/main.ts:5:7

TS2322 [ERROR]: Type 'number' is not assignable to type 'string'.
const error2: string = new Foo2().method();
~~~~~~
at file:///[WILDCARD]/subset_type_graph/main.ts:6:7

TS2551 [ERROR]: Property 'method2' does not exist on type 'Foo'. Did you mean 'method'?
new Foo1().method2();
~~~~~~~
at file:///[WILDCARD]/subset_type_graph/main.ts:12:12

'method' is declared here.
method(): number {
~~~~~~
at http://localhost:4545/jsr/registry/@denotest/subset_type_graph/0.1.0/mod.ts:8:3

TS2551 [ERROR]: Property 'method2' does not exist on type 'Foo'. Did you mean 'method'?
new Foo2().method2();
~~~~~~~
at file:///[WILDCARD]/subset_type_graph/main.ts:13:12

'method' is declared here.
method() {
~~~~~~
at http://localhost:4545/jsr/registry/@denotest/subset_type_graph_invalid/0.1.0/mod.ts:2:3

Found 5 errors.
13 changes: 13 additions & 0 deletions cli/tests/testdata/jsr/subset_type_graph/main.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { Foo as Foo1 } from "jsr:@denotest/[email protected]";
import { Foo as Foo2 } from "jsr:@denotest/[email protected]";

// these will both raise type checking errors
const error1: string = new Foo1().method();
const error2: string = new Foo2().method();
console.log(error1);
console.log(error2);

// now raise some errors that will show the original code and
// these should source map to the original
new Foo1().method2();
new Foo2().method2();
8 changes: 8 additions & 0 deletions cli/tests/testdata/publish/invalid_fast_check.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
Checking fast check type graph for errors...

Missing explicit return type in the public API.
at file:///[WILDCARD]/publish/invalid_fast_check/mod.ts:2:17

Fixing these fast check errors is required to make the code fast check compatible which enables type checking your package's TypeScript code with the same performance as if you had distributed declaration files. Do any of these errors seem too restrictive or incorrect? Please open an issue if so to help us improve: https://github.com/denoland/deno/issues

error: Had 1 fast check error.
7 changes: 7 additions & 0 deletions cli/tests/testdata/publish/invalid_fast_check/deno.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "@foo/bar",
"version": "1.1.0",
"exports": {
".": "./mod.ts"
}
}
4 changes: 4 additions & 0 deletions cli/tests/testdata/publish/invalid_fast_check/mod.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// requires an explicit type annotation of `number`
export function getRandom() {
return Math.random();
}
6 changes: 6 additions & 0 deletions cli/tests/testdata/publish/javascript_decl_file.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Checking fast check type graph for errors...
Ensuring type checks...
Check file:///[WILDCARD]/javascript_decl_file/mod.js
Publishing @foo/[email protected] ...
Successfully published @foo/[email protected]
Visit http://127.0.0.1:4250/@foo/[email protected] for details
7 changes: 7 additions & 0 deletions cli/tests/testdata/publish/javascript_decl_file/deno.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "@foo/bar",
"version": "1.0.0",
"exports": {
".": "./mod.js"
}
}
1 change: 1 addition & 0 deletions cli/tests/testdata/publish/javascript_decl_file/mod.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export function getRandom(): number;
5 changes: 5 additions & 0 deletions cli/tests/testdata/publish/javascript_decl_file/mod.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
/// <reference types="./mod.d.ts" />

export function getRandom() {
return Math.random();
}
6 changes: 6 additions & 0 deletions cli/tests/testdata/publish/javascript_missing_decl_file.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Checking fast check type graph for errors...
Warning Package '@foo/bar' is a JavaScript package without a corresponding declaration file. This may lead to a non-optimal experience for users of your package. For performance reasons, it's recommended to ship a corresponding TypeScript declaration file or to convert to TypeScript.
Ensuring type checks...
Publishing @foo/[email protected] ...
Successfully published @foo/[email protected]
Visit http://127.0.0.1:4250/@foo/[email protected] for details
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "@foo/bar",
"version": "1.0.0",
"exports": {
".": "./mod.js",
"./other": "./other.js"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export function getRandom() {
return Math.random();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export function other() {
return Math.random();
}
3 changes: 3 additions & 0 deletions cli/tests/testdata/publish/missing_deno_json/main.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export function add(a: number, b: number): number {
return a + b;
}
3 changes: 3 additions & 0 deletions cli/tests/testdata/publish/successful.out
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
Checking fast check type graph for errors...
Ensuring type checks...
Check file:///[WILDCARD]/publish/successful/mod.ts
Publishing @foo/[email protected] ...
Successfully published @foo/[email protected]
Visit http://127.0.0.1:4250/@foo/[email protected] for details
2 changes: 1 addition & 1 deletion cli/tests/testdata/publish/successful/deno.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@
".": "./mod.ts"
},
"imports": {
"@std/http": "jsr:@std/http@1"
"@std/http": "./std_http.ts"
}
}
2 changes: 1 addition & 1 deletion cli/tests/testdata/publish/successful/mod.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import http from "@std/http";

export function foobar() {
export function foobar(): { fileServer(): void } {
return http.fileServer;
}
Loading

0 comments on commit 70ac061

Please sign in to comment.