linting is too slow

[boneskull]

The output of the lint job for my latest PR:

Run yarn lint
  yarn lint
  shell: /usr/bin/bash -e {0}
Checking formatting...
All matched files use Prettier code style!
Done in 8m 11s

This is far too slow. My suggested solution(s):

1. For packages using typescript-eslint, do not run tsc; tseslint invokes tsc anyway and it is redundant.
2. Do not run eslint individually for each package. Run it once, in the workspace root.
3. One of:
   a. Run the prettier check once in the workspace root
   b. (Controversial) Omit the prettier check entirely. Use lint-staged and husky as a pre-commit hook to run prettier --write on the staged files (also eslint --fix).
   c. Do a., but also add the pre-commit hook.
4. Ensure we're on the latest versions of tseslint and ESLint and follow its best practices for performant linting.
5. If we must lint in workspaces individually (why?), then parallelize them. This might not help CI as much as it would in a dev environment.

[turadg]

Agreed. Here's some numeric analysis, with sample size 1.

Latest CI had total duration 11m33s. lint was the bottleneck at 11m22s and the next slowest (viable release) was just 6m.

The lint job spent 8m33s on on yarn lint and 2m33s building API docs. @kriskowal pointed out we can pull out API docs to another job. This PR updates typescript-eslint and dropped yarn lint time to 6ms14s,

• typecheck tests and bump typescript-eslint #2674

Looking over the other suggestions,

1. For packages using typescript-eslint, do not run tsc; tseslint invokes tsc anyway and it is redundant.
   Not redundant. I confirmed by inducing a type error in bundle-source/src/fs.js and it only being flagged by tsc. Then I undid that and induced an eslint error (which only eslint caught) to confirm typescript-eslint ran on it. Beyond that, we need the freedom to opt in/out of typescript-eslint by glob, not just by package.

2. Do not run eslint individually for each package. Run it once, in the workspace root.

This would help, but I don't think by a lot. ESlint runs on files mostly in isolation. A big drawback is a the change in workflow with this repo. Given the large overlap of developers with agoric-sdk it would require a change there too or a divergence.

4. One of:
   a. Run the prettier check once in the workspace root
   That's the status quo. There is (and should be) no variation in whitespace config between packages so it's a repo-level check.

b. (Controversial) Omit the prettier check entirely. Use lint-staged and husky as a pre-commit hook to run prettier --write on the staged files (also eslint --fix).
c. Do a., but also add the pre-commit hook.

Two reasons not to do that:

1. Easy to circumvent intentionally (skip hooks) or accidentally (bad rebase), leaking problems into trunk.
2. Slows down commits

5. Ensure we're on the latest versions of tseslint and ESLint and follow its best practices for performant linting.

Indubitably. tseslint needs review and I've filed ESLint v9,

• typecheck tests and bump typescript-eslint #2674
• upgrade ESlint to v9 Agoric/agoric-sdk#10642

6. If we must lint in workspaces individually (why?), then parallelize them. This might not help CI as much as it would in a dev environment.

Why packages are linted individually… I wasn't here for the decision but I imagine it had something to do with "lint what I'm working on" and "let me configure this differently here". There are better ways meet those, imo, but it's not worth the disruption.

If we do the above then lint will no longer be a bottleneck for CI.

If you want your dev environment to lint faster, we could consider more parallelization. But that deserves its own issue because the trade-offs and considerations are different than for CI. I'm updating this issue title to be about CI.

Acceptance criteria

I propose that we can close this issue when the lint job is within 10% of the next slowest job. I think we'll get that through,

• typecheck tests and bump typescript-eslint #2674
• split out the API docs job

[boneskull]

@turadg

re: packages linted individually. I have a feeling it will actually be significantly more performant to run it once from the workspace root; not because of ESLint proper, but tseslint. It needs to bootstrap TS for each project, and it may not be able to take advantage of caching. This is compounded by the fact that the projects are also not incrementally built--it may be that tseslint has to recompile entire dependency trees with tsc each time it runs against a single workspace. regardless, when we call eslint we're not just bootstrapping eslint--it's tsc too.

(but I haven't profiled it so who knows? how to profile multiple processes in serial and aggregate the results, anyway?)

I mean, there are several choices made here that have a negative performance impact. Ostensibly they are done for DX reasons.

I'm trying not to be a blowhard here, but: my experience suggests that concern about the workflow changing is a concern about change or learning a new workflow instead of any fundamental shortcoming of an incrementally-built project-based monorepo. I've worked in several of these (including LavaMoat) and have never felt that it negatively impacts my productivity (some recent version of tsc has also offered improvements in incremental builds so that it no longer bails on the first error; I recall this being one of the complaints). Further, we're not compiling TS to JS so generally aren't worried about things like sourcemaps and the debugging experience. Rather, these performance issues (and tangential friction like #2665) do make things difficult for me.

---

re: commit hook. how slow is too slow? Formatting staged files on commit is not something I've ever known to feel slow. I typically run eslint --fix and prettier --write as a commit hook in my projects, and these won't take longer than a few seconds even on the larger repos. Running tests ... that would probably be too slow. Anyway, this has bitten me countless times. The lint script runs prettier --check, but I don't run lint because it's too slow, so it breaks in CI instead, and I have to go back and fix it--which ultimately wastes more time than it would have if we always ran it as a pre-commit hook. 🤷 That's really the idea. Not automatically fixing it is sort of "penny wise, pound foolish", IMO.

---

re: title change. I meant what I wrote: linting is too slow. CI linting is even slower. 😄