Improve gts gjs configuration example #1980
Conversation
|
@NullVoxPopuli would like your opinion too |
README.md
Outdated
| @@ -70,6 +70,8 @@ module.exports = { | |||
| plugins: ['ember'], | |||
| extends: [ | |||
| 'eslint:recommended', | |||
| 'plugin:ember/recommended', | |||
| 'plugin:ember/gjs-recommended', | |||
There was a problem hiding this comment.
both gjs and gts for gts files? 🤷 why not the one that matches the extension?
There was a problem hiding this comment.
Can also be done. Just means we have to add rules to 2 configs
| @@ -53,7 +53,7 @@ learn more [here](https://github.com/ember-template-imports/ember-template-impor | |||
| module.exports = { | |||
| overrides: [ | |||
| { | |||
| files: ['**/*.{js,ts,gjs,gts}'], | |||
| files: ['**/*.{js,ts}'], | |||
| plugins: ['ember'], | |||
| extends: [ | |||
There was a problem hiding this comment.
The configs in this example seem pretty redundant. I don't think we should be applying the same config multiple times.
eslint:recommended and plugin:ember/recommended can be applied outside an override. Then we can have a gts override which only applies the gts config. And likewise for a gjs override.
There was a problem hiding this comment.
Outside-of-overrides is going away in eslint 9
There be OnlyOverrides soonp
So, the sooner we can start teaching people to favor overrides, the better.
The eslint folks did a whole blog series on this, and i endorse it.
Top level config is just... Bad (tldr)
There was a problem hiding this comment.
Fair point. However, I think we should stick to showing the old .eslintrc.js format for now, instead of a mix of both formats.
https://eslint.org/docs/latest/use/configure/migration-guide#importing-plugins-and-custom-parsers
There was a problem hiding this comment.
Since it's new config entirely, why bother adding to something that will be removed?
There was a problem hiding this comment.
Normally, you should follow the patterns already in place in the codebase. The README has another example right above this using the old config format. So I'd prefer to keep things consistent. We can migrate the docs over to the new config format all at the same time for consistency, but that seems out-of-scope for this PR.
There was a problem hiding this comment.
There is also another part to consider, which i should have included in the example.
It's when setting a top level parser. E.g typescript.
Because the first parser set, will be used and cannot be overwritten anymore
There was a problem hiding this comment.
I opened #1983 for more details.
should stick to showing the old .eslintrc.js format for now, instead of a mix of both formats.
overrides only is still "old format", too!
ormally, you should follow the patterns already in place in the codebase
ye, but we're about to do a major so now's the chance!
new config format all at the same time for consistency,
I feel I may have been misunderstood.
I'm not suggesting documenting the new config format, just the pattern of using overrides only with the old format because it makes migrating to the new format easier later.
that seems out-of-scope for this PR.
roger roger
Because the first parser set, will be used and cannot be overwritten anymore
another reason to avoid top level config!
| plugins: ['ember'], | ||
| parser: '@typescript-eslint/parser', |
There was a problem hiding this comment.
Shouldn't TS parser only be applied to TS files?
There was a problem hiding this comment.
ye --- its not what the ember blueprint does --- but it should, imo (more cases for overrides only)
There was a problem hiding this comment.
also, the example on typescript-eslint shows it on top level...
|
needs rebase it looks like |
bmish
changed the title
No description provided.