-
-
Notifications
You must be signed in to change notification settings - Fork 36.1k
Dev: Migrate to ESLint 9, flat config #32471
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
Conversation
| "lint-addons": "eslint examples/jsm --ext .js --ignore-pattern libs --ignore-pattern ifc", | ||
| "lint-examples": "eslint examples --ext .html", | ||
| "lint-docs": "eslint docs --ignore-pattern prettify.js", | ||
| "lint-editor": "eslint editor --ignore-pattern libs", | ||
| "lint-playground": "eslint playground --ignore-pattern libs", | ||
| "lint-manual": "eslint manual --ignore-pattern 3rdparty --ignore-pattern prettify.js --ignore-pattern shapefile.js", | ||
| "lint-test": "eslint test --ignore-pattern vendor", | ||
| "lint-utils": "eslint utils --ignore-pattern prettify --ignore-pattern fuse", | ||
| "lint-addons": "eslint examples/jsm", | ||
| "lint-examples": "eslint examples", | ||
| "lint-docs": "eslint docs", | ||
| "lint-editor": "eslint editor", | ||
| "lint-playground": "eslint playground", | ||
| "lint-manual": "eslint manual", | ||
| "lint-test": "eslint test", | ||
| "lint-utils": "eslint utils", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since ignore patterns are included in the config, now, it's possible we can merge these commands together and let the user specify any sub folder checks themselves. I can look into adding these back if they're preferred, though.
My "lint" scripts are set to eslint . so the whole project is validated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Mugen87 I see you've 👍 the comment - do you want me to change anything here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would keep the commands for now. The 👍 was for the simplification of the commands in package.json^^.
| 'no-async-promise-executor': 'off', | ||
| 'no-empty': 'off', | ||
|
|
||
| // 'jsdoc/check-types': 'error', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When this is enabled there are some errors in the JSDoc comments that are caught. I'll leave it to another contribution to re-enable and fix these. It may be worth enabling the JSDoc "recommended" settings above to see if there are any rules worth keeping from there, too.
| 'no-useless-escape': 'off', | ||
| 'no-case-declarations': 'off', | ||
| 'no-cond-assign': 'off', | ||
| 'getter-return': 'off', | ||
| 'no-async-promise-executor': 'off', | ||
| 'no-empty': 'off', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I turned these off because they seemed to be new rules and were causing lint errors / warnings. It may be worth considering adjusting the codebase and turning these on.
| BigInt: 'readonly', | ||
| BigUint64Array: 'readonly' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These lines have been newly added
📦 Bundle sizeFull ESM build, minified and gzipped.
🌳 Bundle size after tree-shakingMinimal build including a renderer, camera, empty scene, and dependencies.
|
|
This is needed. For some reason I can't run lint now. It seems to be collecting another lint file further up a directory. |
|
@Mugen87 Is this okay to merge? Will you be able to take a look at some of the jsdoc configs afterward?
This must be something else related to your setup. The project has been using eslint 8.x with the legacy config without issue for awhile now. |
I have installed an eslint 9 config in a top level directory. And three is checked out in a sub directory. It was working until I added that and installed eslint 9 in my project. When I moved the eslint config which exists two directories up. The error goes away. VSCode lint syncs up with three requirements apart from an indent config I had to change from space to tab as I use 4 spaces everywhere else so it's auto linting now for PR's. |
Yes, I'll check out what's going on. |
Related issue: -
Description
The ESLint 9 configs have moved to a new flat config format. I've migrated some of my other projects so I thought I'd try it here, trying to keep the rules the same as much as I can. Settings like "jsdoc" rules have since changed to a separate plugin and in the future some of the styling rules will change to a separate code style plugin. There is an open issue at the mdcs plugin here to migrate, as well.