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

[DRAFT] feat: skip type checking by default #11810

Closed
wants to merge 2 commits into from

Conversation

kitsonk
Copy link
Contributor

@kitsonk kitsonk commented Aug 23, 2021

Closes: #11340

This PR:

  • adds the --check flag
  • deprecates the --no-check flag
  • does not type check code by default on subcommands
  • logs a warning to stderr that the behaviour has changed and indicates that --check can be used to type check

The tests still need to be updated for not type checking by default.

A couple of points of discussion:

  • Right now, deno test does not type check by default. It be more logical to implicitly enable --check for deno test. This does mean this subcommand would differ in default flags than other subcommands, and we would not remote --no-check for deno test.
  • The behavior of deno cache reflects the others, skipping type checking by default. We need to determine how it all relates to deno check #11786 (deno check). I don't think deno check would replace the use of --check flag, as there are lots of subcommands where people would not want to have to invoke 2 subcommands just to check their documents, as well as there is the challenge that we implicitly type check worker scripts contextually, so unless we add flags to deno check for the type of code it is checking, some code will fail.
  • The way I am managing the warn_check flag in ProgramState feels hacky to me, but I don't have any great ideas how better to manage it. We need it, as there are certain commands where the methods get called > 1 times, causing multiple warnings to be printed if we don't have a flag.

@caspervonb
Copy link
Contributor

caspervonb commented Aug 23, 2021

Right now, deno test does not type check by default. It be more logical to implicitly enable --check for deno test. This does mean this subcommand would differ in default flags than other subcommands, and we would not remote --no-check for deno test.

Consistency with the common options is better here imo. As with run, type checking failing doesn't mean that the actual runtime behaviour will be affected.

The no-run option for deno test would also become redundant as deno test --no-run would be the same as deno check.

@nayeemrmn nayeemrmn mentioned this pull request Aug 23, 2021
@kitsonk kitsonk added this to the 1.14.0 milestone Aug 24, 2021
@kitsonk kitsonk removed this from the 1.14.0 milestone Sep 2, 2021
@kitsonk kitsonk added the maybe 2.0 a potential feature for Deno 2.0 that needs further discussion label Sep 2, 2021
@CLAassistant
Copy link

CLAassistant commented Oct 15, 2021

CLA assistant check
All committers have signed the CLA.

@bartlomieju
Copy link
Member

@kitsonk should we close this PR in favor of #12766?

@kitsonk kitsonk closed this Nov 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maybe 2.0 a potential feature for Deno 2.0 that needs further discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Skip type checking by default
4 participants