feat(init): add check task to run all static analysis

[deer]

closes #784

The author of 784 has very kindly provided a repo to reproduce the issue. I agree, the error message when starting the project is horrible:

deno task start
Task start deno run -A --watch=static/,routes/ dev.ts
Check file:///Users/reed/code/temp/issue-fresh-dx/dev.ts
Watcher Process started.
The manifest has been generated for 3 routes and 1 islands.
error: Uncaught (in promise) SyntaxError: Unexpected reserved word at file:///Users/reed/code/temp/issue-fresh-dx/islands/Counter.tsx:12:5
  await import(entrypoint);
  ^
    at async dev (https://deno.land/x/fresh@1.2.0/src/dev/mod.ts:140:3)
    at async file:///Users/reed/code/temp/issue-fresh-dx/dev.ts:5:1
Watcher Process failed. Restarting on file change...

It does highlight the problematic file Counter.tsx, but the error shown isn't super clear. I don't understand why we're not type checking projects before. After I add the check command to the start task I get the following:

deno task start
Task start deno check main.ts && deno run -A --watch=static/,routes/ dev.ts
Check file:///Users/reed/code/temp/issue-fresh-dx/main.ts
error: TS7006 [ERROR]: Parameter 'p' implicitly has an 'any' type.
  const wrongFunction = (p) => {
                         ^
    at file:///Users/reed/code/temp/issue-fresh-dx/islands/Counter.tsx:11:26

TS1308 [ERROR]: 'await' expressions are only allowed within async functions and at the top levels of modules.
    await p();
    ~~~~~
    at file:///Users/reed/code/temp/issue-fresh-dx/islands/Counter.tsx:12:5

TS1356 [ERROR]:     Did you mean to mark this function as 'async'?
      const wrongFunction = (p) => {
                            ~~~~~~~~
        at file:///Users/reed/code/temp/issue-fresh-dx/islands/Counter.tsx:11:25

Found 2 errors.

This makes it abundantly clear what's wrong with the project. If someone doesn't want to type check their project, it should be decently clear about how to remove the deno check main.ts && part from the task. But the other way -- figuring out how to add this isn't so obvious. (I only got it working after a few attempts, e.g. deno check dev.ts doesn't catch the error. How checking main.ts checks the other files in the project is still a mystery to me. As another hurdle to figuring out how to do this on your own: https://deno.land/manual@v1.35.0/tools doesn't mention check, so this is difficult to find.)

[sigmaSd]

On the other hand this will make startup slow, deno disabled typechecking by default everywhere for this reason

Also is there a reason to not just use deno run --check .. dev.ts , I'm not 100 % sure but in my usage I think it feels like it typechecks more stuff then just deno check file

Maybe at least we should have start:fast or something that doesn't type check

[sigmaSd]

Also I remember the argument for disabling type-checking by default was that the editor should have the checks via the lsp already and the uesr shouldn't need to pay the costly startup time for the same redundant diagnostics. I guess the mentality is the user should fix errors before running the file

[marvinhagemeister]

Agree that the error message thrown by V8 is confusing and not as user friendly as it should be. I'm a little worried about introducing a type-check step when launching the server as it slows down the startup sequence very noticeably. For smaller projects that is likely not much of a concern, but with bigger ones you tend to end up in situations where type checking takes more than 30s.

I feel like another alternative is custom linting rules. I've added one that catches when an await keyword is used in a non-async function here denoland/deno_lint#1176 . That way users get immediate feedback in their editors and it won't affect the startup time.

[deer]

Why not use deno run --check ....? Because this is a photo of me:

So two things:

1. great point on this possibly being slow for large projects
2. thanks for showing me the --check flag for run. I suppose I should go through the commands listed from doing deno --help and then the subcommands as well, just to see the full spectrum of things out there.

The point about "the user should fix errors before running the file" is, I think, easily countered with "have you ever met humans before?" 😂 But more seriously:

1. they might make a change that causes an error in another file and not be aware of it
2. knowledge of best practices is, by definition, tribal knowledge. Newcomers will not have this knowledge until they've cut themselves enough (like I've done here).

In general I'm in favor of providing a safe space that gives friendly error messages. Do we agree with that? If so, is there some way that we could enable this by default and let people opt out if necessary? Given that this is a change to the init script, I don't think it will impact existing projects. And if an experienced user is creating a new project and doesn't want to type check in dev mode, they can easily delete the relevant part from the start command. But it's not so easy to come across this idea (type checking before starting) as a newbie.

Of course it's perfectly ok to just close this as well -- it is just a request after all.

[deer]

I've reworked this to introduce a safe command. check seems like the better choice, but I don't want to cause confusion with the built in deno check command.

Fun fact: if you do this in a newly created project, it doesn't pass. When we build the _404.tsx file, there's an empty line at the top, so deno fmt --check complains. PR incoming.

[marvinhagemeister]

I like check as the task name more than safe. With safe I'm not sure what it means and it seems less descriptive to me.

[deer]

i like check better as well, and if you think it won't be confusing, then great. now people can just deno task check in their projects 🙌

[deer]

I realized we also need to check .tsx files as well 🤦 . And it seems useful enough to have in the fresh source itself, no?

[iuioiua]

Shall we align this with #1538, using deno task check:types?

[deer]

Shall we align this with #1538, using deno task check:types?

I don't think so. I find myself doing all of these things, and I wanted a shortcut. I could imagine other people do the same as well, and also want a shortcut. So "check:types" isn't descriptive of linting and formatting and type checking. And if you're suggesting removing linting/formatting, then I'm also not in favor of that 😅 , since I want all at once.

[deer]

error: TS2614 [ERROR]: Module '"https://deno.land/x/puppeteer@16.2.0/mod.ts"' has no exported member 'Page'. Did you mean to use 'import Page from "https://deno.land/x/puppeteer@16.2.0/mod.ts"' instead?
Page,

Another strange CI failure, which didn't happen on any of the other variants in this run...

[iuioiua]

To clarify, what I had in mind was to have a check:types task and an all-encompassing check task that uses check:types within it. However, I retract this suggestion as I doubt the average dev wants this. It's more of a personal preference for me. Either way, LGTM!

[marvinhagemeister]

Ah just realized, we might want to wrap the globs in single quotes ' to be compatible with all shells. I wonder if something like '**/*.{[tj]sx?}' would work to check all files in one go.

[deer]

Cool idea. Sadly:

deno task check
Task check deno fmt --check && deno lint && deno check '**/*.{[tj]sx?}'
Checked 16 files
Checked 12 files
error: Module not found "file:///Users/reed/code/temp/1454/**/*.%7B[tj]sx%3F%7D".

And with a simpler version:

deno task check
Task check deno fmt --check && deno lint && deno check '**/*.{ts,tsx}'
Checked 16 files
Checked 12 files
error: Module not found "file:///Users/reed/code/temp/1454/**/*.%7Bts,tsx%7D".

Even simpler:

deno task check
Task check deno fmt --check && deno lint && deno check '**/*.ts' && deno check '**/*.tsx'
Checked 16 files
Checked 12 files
error: Module not found "file:///Users/reed/code/temp/1454/**/*.ts".

I would just keep it as is for now, and if we want to get fancier, then we could build on #1422 which introduces a small cli script we can add to. Then we could do the fanciest of checks and just invoke it via deno task cli myUltraFancyCheck and not have to worry about different shells and globs and all of this. Just nice controllable typescript.

[iuioiua]

I would just keep it as is for now, and if we want to get fancier, then we could build on #1422 which introduces a small cli script we can add to. Then we could do the fanciest of checks and just invoke it via deno task cli myUltraFancyCheck and not have to worry about different shells and globs and all of this. Just nice controllable typescript.

I'd suggest keeping the checks simple. The checks in this PR do the job great! 🙂