-
-
Notifications
You must be signed in to change notification settings - Fork 204
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
Should we provide a config that disables some rules from @typescript-eslint/eslint-plugin
?
#2110
Comments
this? #2107 mmm, this might be different . |
yeah, that PR adds rules to a config, which I don't think we should do. I imagine the TS config should end up looking like this: {
// this is the exact same as what's in the README
files: ['**/*.gts'],
parser: 'ember-eslint-parser',
plugins: ['ember'],
extends: [
'eslint:recommended',
// hopefully this either adds the rules to the config (so that 2107 isn't needed)
// or we give up on legacy configs and only recommend flat config
'plugin:@typescript-eslint/recommended',
'plugin:ember/recommended',
// this would detect if @typescript-eslint is present,
// and make sure no-use-before-define is turned off
'plugin:ember/recommended-gts',
],
}, |
I think the error is correct though.
|
I think it was pretty clear that they will not change |
nay, it's
|
hmm, i took another look, you are right:) But it looks like the rule isn't looking that exactly anyway: |
ah! so the eslint rule is wrong! |
I looks like that's actually the behaviour it intended to do:
https://eslint.org/docs/latest/rules/no-use-before-define#rule-details |
yeah -- but .. it's safe! |
i think this can be closed then? as the rule does work correctly as per its own definition |
It works incorrectly for how we need it to tho 🤔 |
For example, no-use-before-define is often wrong if you define multiple components in the same file.
would trigger the error.
But this is totally valid, because component rendering is not eager like const-evaluation normally is.
cc @patricklx, in case you have ideas
example in vanilla:
The text was updated successfully, but these errors were encountered: