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

Migrate from CLIEngine to the new ESLint class. #22756

Merged
merged 2 commits into from Nov 18, 2021

Conversation

elas7
Copy link
Contributor

@elas7 elas7 commented Nov 14, 2021

Summary

In ESLint 8, CLIEngine was removed. To migrate from version 7 to 8, we need to replace CLIEngine with the new ESLint class.

More information here:

How did you test this change?

I tested the following commands before and after the migration:

  • For yarn lint and yarn linc, the output was the same.
  • For yarn lint --fix and yarn linc --fix, the output was the same and the fixable errors got fixed.

Pass example:

$ yarn linc
yarn run v1.22.17
$ node ./scripts/tasks/linc.js
Linting changed files...
> git merge-base HEAD main
> git diff --name-only --diff-filter=ACMRTUB c0c71a868560b3042847722659579418bfe2d7e1
> git ls-files --others --exclude-standard

Lint passed for changed files.
✨  Done in 3.38s.

Error example:

$ yarn linc
yarn run v1.22.17
$ node ./scripts/tasks/linc.js
Linting changed files...
> git merge-base HEAD main
> git diff --name-only --diff-filter=ACMRTUB c0c71a868560b3042847722659579418bfe2d7e1
> git ls-files --others --exclude-standard

/Users/XXX/projects/react/scripts/devtools/publish-release.js
  60:7   error  'a' is assigned a value but never used  no-unused-vars
  60:11  error  Multiple spaces found before ';'        no-multi-spaces

✖ 2 problems (2 errors, 0 warnings)
  1 error and 0 warnings potentially fixable with the `--fix` option.

Lint failed for changed files.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Comment on lines +20 to +21
// eslint-disable-next-line no-unused-vars
const {_, ...cliOptions} = minimist(process.argv.slice(2));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to add destructuring to prevent passing _, which minimist generates by default. Otherwise, ESlint returns this error:

yarn linc
yarn run v1.22.17
$ node ./scripts/tasks/linc.js
Linting changed files...
Lint passed for changed files.
(node:84712) UnhandledPromiseRejectionWarning: Error: Invalid Options:
- Unknown options: _
    at processOptions (/Users/XXX/projects/react/node_modules/eslint/lib/eslint/eslint.js:273:15)
    at new ESLint (/Users/XXX/projects/react/node_modules/eslint/lib/eslint/eslint.js:416:34)

Comment on lines +16 to +17
// eslint-disable-next-line no-unused-vars
const {_, ...cliOptions} = minimist(process.argv.slice(2));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

Comment on lines +48 to +55
const errorCount = results.reduce(
(count, result) => count + result.errorCount,
0
);
const warningCount = results.reduce(
(count, result) => count + result.warningCount,
0
);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new class no longer returns a global report with errorCount and warningCount, so I calculated them manually.

@sizebot
Copy link

sizebot commented Nov 14, 2021

Comparing: c0c71a8...0987cfd

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 129.68 kB 129.68 kB = 41.46 kB 41.46 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 134.66 kB 134.66 kB = 42.94 kB 42.94 kB
facebook-www/ReactDOM-prod.classic.js = 424.11 kB 424.11 kB = 78.18 kB 78.18 kB
facebook-www/ReactDOM-prod.modern.js = 412.66 kB 412.66 kB = 76.43 kB 76.43 kB
facebook-www/ReactDOMForked-prod.classic.js = 424.11 kB 424.11 kB = 78.18 kB 78.18 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 0987cfd

@gaearon gaearon merged commit b32b677 into facebook:main Nov 18, 2021
@gaearon
Copy link
Collaborator

gaearon commented Nov 18, 2021

thank you

@elas7 elas7 deleted the eslint-cliengine-migration branch November 18, 2021 21:16
zhengjitf pushed a commit to zhengjitf/react that referenced this pull request Apr 15, 2022
* Migrate from CLIEngine to the new ESLint class.

* fix output property
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants