Skip to content

Commit

Permalink
feat(publish): type check on publish (#22506)
Browse files Browse the repository at this point in the history
Supersedes #22501 and also fixes that issue.
  • Loading branch information
dsherret authored Feb 21, 2024
1 parent 061ee9d commit 9166d8a
Show file tree
Hide file tree
Showing 23 changed files with 106 additions and 22 deletions.
28 changes: 28 additions & 0 deletions cli/args/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2394,6 +2394,8 @@ fn publish_subcommand() -> Command {
.help("Allow publishing with slow types")
.action(ArgAction::SetTrue),
)
.arg(check_arg(/* type checks by default */ true))
.arg(no_check_arg())
})
}

Expand Down Expand Up @@ -3823,6 +3825,9 @@ fn vendor_parse(flags: &mut Flags, matches: &mut ArgMatches) {
}

fn publish_parse(flags: &mut Flags, matches: &mut ArgMatches) {
flags.type_check_mode = TypeCheckMode::Local; // local by default
no_check_arg_parse(flags, matches);
check_arg_parse(flags, matches);
config_args_parse(flags, matches);

flags.subcommand = DenoSubcommand::Publish(PublishFlags {
Expand Down Expand Up @@ -8542,4 +8547,27 @@ mod tests {
let r = flags_from_vec(svec!["deno", "jupyter", "--install", "--kernel",]);
r.unwrap_err();
}

#[test]
fn publish_args() {
let r = flags_from_vec(svec![
"deno",
"publish",
"--dry-run",
"--allow-slow-types",
"--token=asdf",
]);
assert_eq!(
r.unwrap(),
Flags {
subcommand: DenoSubcommand::Publish(PublishFlags {
token: Some("asdf".to_string()),
dry_run: true,
allow_slow_types: true,
}),
type_check_mode: TypeCheckMode::Local,
..Flags::default()
}
);
}
}
35 changes: 23 additions & 12 deletions cli/graph_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,9 @@ impl ModuleGraphCreator {
loader: None,
})
.await?;
if self.options.type_check_mode().is_true() {
self.type_check_graph(graph.clone()).await?;
}
self.module_graph_builder.build_fast_check_graph(
&mut graph,
BuildFastCheckGraphOptions {
Expand Down Expand Up @@ -337,23 +340,31 @@ impl ModuleGraphCreator {

if self.options.type_check_mode().is_true() {
// provide the graph to the type checker, then get it back after it's done
let graph = self
.type_checker
.check(
graph,
check::CheckOptions {
build_fast_check_graph: true,
lib: self.options.ts_type_lib_window(),
log_ignored_options: true,
reload: self.options.reload_flag(),
},
)
.await?;
let graph = self.type_check_graph(graph).await?;
Ok(graph)
} else {
Ok(Arc::new(graph))
}
}

async fn type_check_graph(
&self,
graph: ModuleGraph,
) -> Result<Arc<ModuleGraph>, AnyError> {
self
.type_checker
.check(
graph,
check::CheckOptions {
build_fast_check_graph: true,
lib: self.options.ts_type_lib_window(),
log_ignored_options: true,
reload: self.options.reload_flag(),
type_check_mode: self.options.type_check_mode(),
},
)
.await
}
}

pub struct BuildFastCheckGraphOptions {
Expand Down
1 change: 1 addition & 0 deletions cli/module_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ impl ModuleLoadPreparer {
log_ignored_options: false,
reload: self.options.reload_flag()
&& !roots.iter().all(|r| reload_exclusions.contains(r)),
type_check_mode: self.options.type_check_mode(),
},
)
.await?;
Expand Down
13 changes: 11 additions & 2 deletions cli/tools/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ pub struct CheckOptions {
/// If true, valid `.tsbuildinfo` files will be ignored and type checking
/// will always occur.
pub reload: bool,
/// Mode to type check with.
pub type_check_mode: TypeCheckMode,
}

pub struct TypeChecker {
Expand Down Expand Up @@ -97,6 +99,7 @@ impl TypeChecker {
mut graph: ModuleGraph,
options: CheckOptions,
) -> Result<(Arc<ModuleGraph>, Diagnostics), AnyError> {
debug_assert_ne!(options.type_check_mode, TypeCheckMode::None);
if graph.roots.is_empty() {
return Ok((graph.into(), Default::default()));
}
Expand All @@ -120,8 +123,8 @@ impl TypeChecker {
}
}

let type_check_mode = options.type_check_mode;
let ts_config = ts_config_result.ts_config;
let type_check_mode = self.cli_options.type_check_mode();
let maybe_check_hash = match self.npm_resolver.check_state_hash() {
Some(npm_check_hash) => {
match get_check_hash(
Expand Down Expand Up @@ -300,7 +303,13 @@ fn get_check_hash(
}

hasher.write_str(module.specifier.as_str());
hasher.write_str(&module.source);
hasher.write_str(
// the fast check module will only be set when publishing
module
.fast_check_module()
.map(|s| s.source.as_ref())
.unwrap_or(&module.source),
);
}
Module::Node(_) => {
// the @types/node package will be in the resolved
Expand Down
3 changes: 3 additions & 0 deletions cli/tools/registry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use crate::args::jsr_url;
use crate::args::CliOptions;
use crate::args::Flags;
use crate::args::PublishFlags;
use crate::args::TypeCheckMode;
use crate::cache::LazyGraphSourceParser;
use crate::cache::ParsedSourceCache;
use crate::factory::CliFactory;
Expand Down Expand Up @@ -768,6 +769,8 @@ async fn build_and_check_graph_for_publish(
lib: cli_options.ts_type_lib_window(),
log_ignored_options: false,
reload: cli_options.reload_flag(),
// force type checking this
type_check_mode: TypeCheckMode::Local,
},
)
.await?;
Expand Down
9 changes: 9 additions & 0 deletions tests/integration/publish_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,15 @@ itest!(successful {
http_server: true,
});

itest!(no_check {
args: "publish --token 'sadfasdf' --no-check",
// still type checks the slow types output though
output: "publish/successful_no_check.out",
cwd: Some("publish/successful"),
envs: env_vars_for_jsr_tests(),
http_server: true,
});

itest!(node_specifier {
args: "publish --token 'sadfasdf'",
output: "publish/node_specifier.out",
Expand Down
1 change: 1 addition & 0 deletions tests/testdata/publish/allow_slow_types.out
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
Check file:///[WILDCARD]mod.ts
Warning Publishing a library with slow types is not recommended. This may lead to poor type checking performance for users of your package, may affect the quality of automatic documentation generation, and your package will not be shipped with a .d.ts file for Node.js users.
Publishing @foo/bar@1.1.0 ...
Successfully published @foo/bar@1.1.0
Expand Down
1 change: 1 addition & 0 deletions tests/testdata/publish/deno_jsonc.out
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
Check file:///[WILDCARD]/publish/deno_jsonc/mod.ts
Checking for slow types in the public API...
Check file:///[WILDCARD]/publish/deno_jsonc/mod.ts
Publishing @foo/bar@1.0.0 ...
Expand Down
4 changes: 3 additions & 1 deletion tests/testdata/publish/deno_jsonc/mod.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import http from "@std/http";

export function foobar(): { fileServer(): void } {
return http.fileServer;
return {
fileServer: http.fileServer,
};
}
5 changes: 3 additions & 2 deletions tests/testdata/publish/dry_run.out
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
Check file:///[WILDCARD]/mod.ts
Checking for slow types in the public API...
Check [WILDCARD]
Check file:///[WILDCARD]/mod.ts
Simulating publish of @foo/bar@1.0.0 with files:
[WILDCARD]deno.json (140B)
[WILDCARD]mod.ts (114B)
[WILDCARD]mod.ts (137B)
[WILDCARD]std_http.ts (119B)
Warning Aborting due to --dry-run
1 change: 1 addition & 0 deletions tests/testdata/publish/has_slow_types.out
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
Check file:///[WILDCARD]/mod.ts
Checking for slow types in the public API...
error[missing-explicit-return-type]: missing explicit return type in the public API
--> [WILDCARD]mod.ts:2:17
Expand Down
1 change: 1 addition & 0 deletions tests/testdata/publish/invalid_import.out
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ Download http://localhost:4545/welcome.ts
Download http://localhost:4545/echo.ts
Download http://localhost:4545/npm/registry/chalk
Download http://localhost:4545/npm/registry/chalk/chalk-5.0.1.tgz
Check file:///[WILDCARD]/mod.ts
Checking for slow types in the public API...
Check file://[WILDCARD]mod.ts
error[invalid-external-import]: invalid import to a non-JSR 'http' specifier
Expand Down
1 change: 1 addition & 0 deletions tests/testdata/publish/invalid_path.out
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
Check file://[WILDCARD]mod.ts
Checking for slow types in the public API...
Check file://[WILDCARD]mod.ts
error[invalid-path]: package path must not contain whitespace (found ' ')
Expand Down
2 changes: 1 addition & 1 deletion tests/testdata/publish/javascript_decl_file.out
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
Checking for slow types in the public API...
Check file:///[WILDCARD]/javascript_decl_file/mod.js
Checking for slow types in the public API...
Publishing @foo/bar@1.0.0 ...
Successfully published @foo/bar@1.0.0
Visit http://127.0.0.1:4250/@foo/bar@1.0.0 for details
3 changes: 2 additions & 1 deletion tests/testdata/publish/node_specifier.out
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
Checking for slow types in the public API...
Download http://localhost:4545/npm/registry/@types/node
Download http://localhost:4545/npm/registry/@types/node/node-[WILDCARD].tgz
Check file:///[WILDCARD]/mod.ts
Checking for slow types in the public API...
Check file:///[WILDCARD]/publish/node_specifier/mod.ts
Publishing @foo/bar@1.0.0 ...
Successfully published @foo/bar@1.0.0
Expand Down
1 change: 1 addition & 0 deletions tests/testdata/publish/successful.out
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
Check file:///[WILDCARD]/publish/successful/mod.ts
Checking for slow types in the public API...
Check file:///[WILDCARD]/publish/successful/mod.ts
Publishing @foo/bar@1.0.0 ...
Expand Down
4 changes: 3 additions & 1 deletion tests/testdata/publish/successful/mod.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import http from "@std/http";

export function foobar(): { fileServer(): void } {
return http.fileServer;
return {
fileServer: http.fileServer,
};
}
5 changes: 5 additions & 0 deletions tests/testdata/publish/successful_no_check.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Checking for slow types in the public API...
Check file:///[WILDCARD]/publish/successful/mod.ts
Publishing @foo/bar@1.0.0 ...
Successfully published @foo/bar@1.0.0
Visit http://127.0.0.1:4250/@foo/bar@1.0.0 for details
1 change: 1 addition & 0 deletions tests/testdata/publish/symlink.out
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
Check [WILDCARD]mod.ts
Checking for slow types in the public API...
Check [WILDCARD]mod.ts
warning[unsupported-file-type]: unsupported file type 'symlink'
Expand Down
5 changes: 3 additions & 2 deletions tests/testdata/publish/unanalyzable_dynamic_import.out
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
Check file://[WILDCARD]/mod.ts
Checking for slow types in the public API...
Check file://[WILDCARD]/mod.ts
warning[unanalyzable-dynamic-import]: unable to analyze dynamic import
--> [WILDCARD]mod.ts:1:7
--> [WILDCARD]mod.ts:2:7
|
1 | await import("asd " + asd);
2 | await import("asd " + asd);
| ^^^^^^^^^^^^^^^^^^^^ the unanalyzable dynamic import

info: after publishing this package, imports from the local import map do not work
Expand Down
1 change: 1 addition & 0 deletions tests/testdata/publish/unanalyzable_dynamic_import/mod.ts
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
const asd = "asd";
await import("asd " + asd);
2 changes: 2 additions & 0 deletions tests/testdata/publish/workspace.out
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
Publishing a workspace...
Check file:///[WILDCARD]/workspace/foo/mod.ts
Check file:///[WILDCARD]/workspace/bar/mod.ts
Checking for slow types in the public API...
Check file:///[WILDCARD]/workspace/foo/mod.ts
Check file:///[WILDCARD]/workspace/bar/mod.ts
Expand Down
1 change: 1 addition & 0 deletions tests/testdata/publish/workspace_individual.out
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
Check file:///[WILDCARD]/workspace/bar/mod.ts
Checking for slow types in the public API...
Check file:///[WILDCARD]/workspace/bar/mod.ts
Publishing @foo/bar@1.0.0 ...
Expand Down

0 comments on commit 9166d8a

Please sign in to comment.