refactor: switch to eslint flat config - #577
Conversation
| // eslintConfigEslint[3] is eslint-plugin-n's recommended-script config | ||
| ...eslintConfigEslint.slice(0, 3), | ||
| { | ||
| files: ["**/*.js"], | ||
| ...nodeRecommendedModule | ||
| }, | ||
| { | ||
| files: ["**/*.cjs"], | ||
| ...nodeRecommendedScript | ||
| }, | ||
| ...eslintConfigEslint.slice(4), |
There was a problem hiding this comment.
This is not very convenient, perhaps eslint-config-eslint should export two configurations, one for CommonJS and one for ES Modules? Or 3 configurations: common, CommonJS-specific, and ESM-specific.
There was a problem hiding this comment.
👍 to 3 configs: the eslint-plugin-n is not really needed for web projects like eslint.org.
There was a problem hiding this comment.
Is there a reason we can't just do ...eslintConfigESLint?
There was a problem hiding this comment.
Is there a reason we can't just do
...eslintConfigESLint?
We can't because it's a configuration for CommonJS modules.
| onToken: token => { | ||
| onToken(token) { |
There was a problem hiding this comment.
This is because in the new eslint-config-eslint the option avoidExplicitReturnArrows: true is set.
| function translateTemplateTokens() { | ||
| tokens.push(convertTemplatePart(that._tokens, that._code)); | ||
| that._tokens = []; | ||
| } | ||
| const translateTemplateTokens = () => { | ||
| tokens.push(convertTemplatePart(this._tokens, this._code)); | ||
| this._tokens = []; | ||
| }; |
There was a problem hiding this comment.
Small refactor to avoid disabling no-underscore-dangle, and I think this is a common way to avoid that = this since ES6.
| /** | ||
| * @fileoverview Generate `<test-name>.result.js` from `<test-name>.src.js` in | ||
| * `tests/fixtures/ecma-version/<number>/**` directory. | ||
| * @author Nicholas C. Zakas | ||
| * @author Toru Nagashima | ||
| * | ||
| * Usage: | ||
| * node tools/update-ecma-version-tests.js <number> | ||
| * | ||
| * @author Nicholas C. Zakas | ||
| * @author Toru Nagashima | ||
| */ |
There was a problem hiding this comment.
Some eslint-plugin-jsdoc rules were reporting errors in the structure, so I moved the usage description under the file overview.
| function getTestFilenames(directory) { | ||
| return shelljs.find(directory).filter(filename => | ||
| filename.indexOf(".src.js") > -1).map(filename => | ||
| filename.includes(".src.js")).map(filename => |
There was a problem hiding this comment.
unicorn/prefer-includes
nzakas
left a comment
nzakas
left a comment
There was a problem hiding this comment.
Overall LGTM. Just left a couple notes.
| @@ -0,0 +1,54 @@ | |||
| import eslintConfigEslint from "eslint-config-eslint"; | |||
There was a problem hiding this comment.
Consistent ESLint capitalization
| import eslintConfigEslint from "eslint-config-eslint"; | |
| import eslintConfigESLint from "eslint-config-eslint"; |
| // eslintConfigEslint[3] is eslint-plugin-n's recommended-script config | ||
| ...eslintConfigEslint.slice(0, 3), | ||
| { | ||
| files: ["**/*.js"], | ||
| ...nodeRecommendedModule | ||
| }, | ||
| { | ||
| files: ["**/*.cjs"], | ||
| ...nodeRecommendedScript | ||
| }, | ||
| ...eslintConfigEslint.slice(4), |
There was a problem hiding this comment.
Is there a reason we can't just do ...eslintConfigESLint?
| settings: { | ||
| jsdoc: { | ||
| mode: "typescript" | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
I've made a PR to move it to eslint-config-eslint.
There was a problem hiding this comment.
👍
We'll remove it from here when we release eslint-config-eslint v9.
|
Re-approved latest changes. This is ready for merge but would like @mdjermanovic to do that to make sure I didn't miss things from other conversations. |
|
It's good now with eslint-config-eslint v8. I'll merge this now and update the config when we release eslint-config-eslint v9 with changes from eslint/eslint#17338 and eslint/eslint#17336. |
3 participants
Switches to
eslint.config.js, and newly releasedeslint-config-eslintv8.Marked as
refactorbecause this includes changes in production code.