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

Should we provide a config that disables some rules from @typescript-eslint/eslint-plugin? #2110

Open
NullVoxPopuli opened this issue Mar 15, 2024 · 11 comments

Comments

@NullVoxPopuli
Copy link
Contributor

NullVoxPopuli commented Mar 15, 2024

For example, no-use-before-define is often wrong if you define multiple components in the same file.

const Foo = <template><Bar /></template>;
const Bar = <template>hi</template>;

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:

image

@patricklx
Copy link
Contributor

patricklx commented Mar 15, 2024

this? #2107

mmm, this might be different .
But it wil be eager later. Because scope is passes directly, right?

@NullVoxPopuli
Copy link
Contributor Author

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', 
  ],
},

@patricklx
Copy link
Contributor

patricklx commented Mar 15, 2024

I think the error is correct though.
it will be compiled to something like

Foo =  precompileTemplate(`...`,  {
  scope: [Bar]
})

@patricklx
Copy link
Contributor

I think it was pretty clear that they will not change plugin:@typescript-eslint/recommended behaviour. (Lets discuss this part in #2107)

@NullVoxPopuli
Copy link
Contributor Author

it wilz be compiled to something like

nay, it's

scope: () => ({ Bar })

@patricklx
Copy link
Contributor

patricklx commented Mar 15, 2024

it wilz be compiled to something like

nay, it's

scope: () => ({ Bar })

hmm, i took another look, you are right:)

But it looks like the rule isn't looking that exactly anyway:
eslint playground

@NullVoxPopuli
Copy link
Contributor Author

ah! so the eslint rule is wrong!

@patricklx
Copy link
Contributor

I looks like that's actually the behaviour it intended to do:

This rule will warn when it encounters a reference to an identifier that has not yet been declared.

https://eslint.org/docs/latest/rules/no-use-before-define#rule-details

@NullVoxPopuli
Copy link
Contributor Author

yeah -- but .. it's safe!

@patricklx
Copy link
Contributor

i think this can be closed then? as the rule does work correctly as per its own definition

@NullVoxPopuli
Copy link
Contributor Author

It works incorrectly for how we need it to tho 🤔

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

No branches or pull requests

2 participants