8000 feat(cli): create initial, proof-of-concept CLI by bradzacher · Pull Request #4359 · typescript-eslint/typescript-eslint · GitHub
[go: up one dir, main page]
Skip to content

feat(cli): create initial, proof-of-concept CLI #4359

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

feat(cli): create initial, proof-of-concept CLI #4359

wants to merge 1 commit into from

Conversation

bradzacher
Copy link
Member
@bradzacher bradzacher commented Dec 27, 2021 •
edited
Loading

PR Checklist

Overview

This adds an initial, proof-of-concept version of a typescript-eslint CLI.

It offers the following commands:

  • ts-eslint lint -p <tsconfig path> to lint one or more tsconfigs
  • ts-eslint env to dump package versions for issues

Help Dump

Collapsed for brevity 
$ yarn ts-eslint --help

Usage: ts-eslint -p ./tsconfig.json

Commands:
  ts-eslint env   Prints information about your environment to provide when reporting an issue.                                       [aliases: environment, env-info, support-info]
  ts-eslint lint  Lint your project.

Options:
      --version              Show version number                                                                                                                           [boolean]
      --logLevel             Control the log level                                                                                                       [string] [default: "error"]
      --reporter             Control how the console output is rendered                                                                                    [string] [default: "ink"]
      --help                 Show help                                                                                                                                     [boolean]
  -p, --project, --projects  Path to a tsconfig, relative to the CWD. Can also specify a glob pattern - ensure you wrap in quotes to prevent CLI expansion of the glob.
                                                                                                                                                                  [array] [required]
      --cwd                  The path to the current working directory to use for the run.                             [string] [default: "<your CWD here>"]
$ yarn ts-eslint lint --help

ts-eslint lint

Lint your project.

Options:
      --version              Show version number                                                                                                                           [boolean]
      --logLevel             Control the log level                                                                                                       [string] [default: "error"]
      --reporter             Control how the console output is rendered                                                                                    [string] [default: "ink"]
      --help                 Show help                                                                                                                                     [boolean]
  -p, --project, --projects  Path to a tsconfig, relative to the CWD. Can also specify a glob pattern - ensure you wrap in quotes to prevent CLI expansion of the glob.
                                                                                                                                                                  [array] [required]
      --cwd                  The path to the current working directory to use for the run.                             [string] [default: "<your CWD here>"]
$ yarn ts-eslint env

| package                               | version |
| ------------------------------------- | ------- |
| node                                  | 16.6.0  |
| @typescript-eslint/cli                | 5.8.1   |
| @typescript-eslint/eslint-plugin      | 5.8.1   |
| @typescript-eslint/experimental-utils | 5.8.1   |
| @typescript-eslint/parser             | 5.8.1   |
| @typescript-eslint/scope-manager      | 5.8.1   |
| @typescript-eslint/typescript-estree  | 5.8.1   |
| typescript                            | 4.5.2   |

Performance Comparison

This repo - ~25% faster 
$ time yarn ts-eslint -p ./tsconfig.eslint.json -p "packages/*/tsconfig.json" --reporter plain

126.74s user
9.47s system
30.321 total

vs

$ time yarn eslint . --ext .js,.jsx,.ts,.tsx

56.73s user
4.23s system
47.373 total

Initial testing shows a very good speedup on our repo! It consistently is ~15-20s faster than a pure ESLint run - which is a huge win, even on our (relatively) small repo.

tldraw - ~44% faster

Notes:

  • https://github.com/tldraw/tldraw
  • not a massive project
      < 8000 li>only 448 TS files linted
    • 7 total tsconfigs
  • a little ways behind on versions (they were on 4.33.0).
  • doesn't use type-aware linting
    • In order to make this a fair comparison I enabled it and added the recommended-with-typechecking config
  • has lerna setup, so I used that to run eslint across their repo.
eslintrc.js I used 
module.exports = {
  root: true,
  parser: '@typescript-eslint/parser',
  plugins: ['@typescript-eslint'],
  extends: [
    'eslint:recommended',
    'plugin:@typescript-eslint/recommended',
    'plugin:@typescript-eslint/recommended-requiring-type-checking',
  ],
  overrides: [
    {
      // enable the rule specifically for TypeScript files
      files: ['*.ts', '*.tsx'],
      rules: {
        '@typescript-eslint/explicit-module-boundary-types': [0],
      },
      parserOptions: {
        project: [
          'packages/*/tsconfig.json',
          'apps/vscode/extension/tsconfig.json',
          'apps/www/tsconfig.json',
        ],
        tsconfigRootDir: __dirname,
      },
    },
  ],
}
$ time yarn lerna run --no-bail lint

115.22s user
11.88s system
1:17.26 total

vs

$ time node ./node_modules/@typescript-eslint/cli/bin/ts-eslint.js -p packages/*/tsconfig.json -p apps/vscode/extension/tsconfig.json -p apps/www/tsconfig.json

95.47s user
7.05s system
43.405 total

Again - this shows a clear runtime win, though this time it is greater at ~34s.

chrome devtools-frontend - no significant wins

Notes:

$ time node scripts/test/run_lint_check_js.mjs

99.61s user
5.42s system
1:04.70 total

vs

$ time node ../typescript-eslint/packages/cli/bin/ts-eslint.js -p ./config/typescript/tsconfig.eslint.json 

99.45s user
5.52s system
1:04.60 total

It's expected that there's no major change in lint time here as there is only one tsconfig being linted - meaning it cannot take advantage of parallelisation.

To test further - I added one tsconfig to the root of each of the "packages" and tried again

the tsconfig 
{
  "extends": "../tsconfig.json",
  "include": ["."],
  "exclude": ["third_party"]
}
$ time node ./typescript-eslint/packages/cli/bin/ts-eslint.js -p ./front_end/tsconfig.eslint.json -p ./test/tsconfig.eslint.json -p inspector_overlay/tsconfig.eslint.json 

130.59s user
8.68s system
57.977 total

Unfortunately - this did not yield any significant wins. Again, this is to be expected to some degree. Out of the 1590 files being linted - 1095 (68%) of them are in just one tsconfig - which ultimately takes the majority (or rather all) of the time collect type information for.

TODO:

  • implement env command to log the packages table for issues
  • implement lint command to lint the codebase
    • lint each project in a separate worker logic using jest-worker
    • implement -p option for specifying tsconfig paths
  • implement "loggers" to control CLI output (--format)
    • implement json logger
    • implement ink logger
    • implement "plain" logger that mirrors ESLint's output

Stretch goals:

  • implement -f option for specifying files/folders/globs
    • leverage FileEnumerator to gather the list of files
    • traverse the tree to find a relevant tsconfig? Or maybe just rely on the pre-configured parserOptions.project for the files?

@nx-cloud
Copy link
nx-cloud bot commented Dec 27, 2021 •
edited
Loading

☁️ Nx Cloud Report

CI ran the following commands for commit 85efd19. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

✅ Successfully ran 48 targets

Sent with 💌 from NxCloud.

@typescript-eslint
Copy link
Contributor

Thanks for the PR, @bradzacher!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitors per day.

@netlify
Copy link
netlify bot commented Dec 27, 2021 •
edited
Loading

✔️ Deploy Preview for typescript-eslint ready!

🔨 Explore the source changes: 85efd19

🔍 Inspect the deploy log: https://app.netlify.com/sites/typescript-eslint/deploys/61d69af6d1916e000700497d

😎 Browse the preview: https://deploy-preview-4359--typescript-eslint.netlify.app

@bradzacher bradzacher added the enhancement New feature or request label Dec 27, 2021
@codecov
Copy link
codecov bot commented Dec 28, 2021 •
edited
Loading

Codecov Report

Merging #4359 (85efd19) into main (8d710a0) will decrease coverage by 2.31%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #4359      +/-   ##
==========================================
- Coverage   94.26%   91.95%   -2.32%     
==========================================
  Files         169      346     +177     
  Lines        9440    11622    +2182     
  Branches     2940     3298     +358     
==========================================
+ Hits         8899    10687    +1788     
- Misses        321      675     +354     
- Partials
8000
      220      260      +40
Flag Coverage Δ
unittest 91.95% <ø> (-2.32%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ackages/experimental-utils/src/ts-eslint/Linter.ts 100.00% <ø> (ø)
...e-manager/src/definition/FunctionNameDefinition.ts 100.00% <0.00%> (ø)
packages/type-utils/src/requiresQuoting.ts 0.00% <0.00%> (ø)
packages/scope-manager/src/scope/ScopeBase.ts 91.61% <0.00%> (ø)
...experimental-utils/src/ts-eslint-scope/Variable.ts 100.00% <0.00%> (ø)
packages/visitor-keys/src/visitor-keys.ts 100.00% <0.00%> (ø)
...ckages/experimental-utils/src/ast-utils/helpers.ts 66.66% <0.00%> (ø)
packages/scope-manager/src/lib/es2020.string.ts 100.00% <0.00%> (ø)
...e-manager/src/definition/TSModuleNameDefinition.ts 100.00% <0.00%> (ø)
packages/scope-manager/src/scope/TypeScope.ts 100.00% <0.00%> (ø)
... and 168 more

@bradzacher bradzacher marked this pull request as draft December 28, 2021 07:00
@bradzacher bradzacher added the DO NOT MERGE PRs which should not be merged yet label Dec 28, 2021
@bradzacher bradzacher mentioned this pull request Dec 29, 2021
Closed
@bradzacher bradzacher force-pushed the create-poc-cli branch 2 times, most recently from 9ad2008 to 826d957 Compare January 6, 2022 06:42
@bradzacher bradzacher mentioned this pull request May 7, 2022
Closed
@JoshuaKGoldberg
Copy link
Member

Closing this old draft for housekeeping since there are merge conflicts and it's taking up space in the open PRs list. Nothing bad will happen in my housekeeping if this is re-opened. Don't mind me. 😄

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
DO NOT MERGE PRs which should not be merged yet enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[RFC] Building a CLI for typescript-eslint
2 participants
@bradzacher @JoshuaKGoldberg
0