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 start a Polaris config that folks can opt in to when using gjs/gts? #2830

Closed
NullVoxPopuli opened this issue Mar 3, 2023 · 4 comments · Fixed by #2844
Closed

Comments

@NullVoxPopuli
Copy link
Contributor

I think we need to change defaults when using gjs/gts, for example

several rules throw false negatives because templates can reference things imported in JS, but not defined in the template. for example:

import { overInvalidatingClock, clock } from './-resources';

const SomeClocks = <template>
  {{clock}}
  {{overInvalidatingClock}}
</template>;

this throws errors for the following rules:

  • no-curly-component-invocation
  • no-implicit-this

the behavior of checking if locals are defined or not is handled be eslint / eslint-plugin-ember via transform.

So, for ember-template-lint and a polaris config, I think we should start disabling some rules specifically for gjs/gts files -- and we could use this for the polaris config:

'use strict';

module.exports = {
  extends: 'recommended',
  overrides: [
    {
      files: ['**/*.gjs', '**/*.gts'],
      rules: {
        'no-curly-component-invocation': 'off',
        'no-implicit-this': 'off',
      },
    },
  ],
};

Related: #2786

@bmish
Copy link
Member

bmish commented Mar 14, 2023

Do we need a polaris config? Why not just make those overrides part of the recommended config so everyone gets them atuomatically?

NullVoxPopuli added a commit to NullVoxPopuli/ember-template-lint that referenced this issue Mar 14, 2023
…template-lint#2830

Normally this would be a breaking change under any other circumstances, but gjs/gts folks need to disable these rules anyway, so this is more a bugfix.
@NullVoxPopuli
Copy link
Contributor Author

yeah, I suppose that makes sense -- this sort of thing can only reduce bugs people face atm: #2844

@bmish
Copy link
Member

bmish commented Mar 15, 2023

Just to confirm, it doesn't make any sense to use these rules for these file types, right? If that's right, I think it makes sense to treat this as a bug fix.

And are there likely to be other rules that don't make sense for these file types?

@NullVoxPopuli
Copy link
Contributor Author

If that's right, I think it makes sense to treat this as a bug fix.

correct, and there is a corresponding eslint-plugin-ember PR for resolving the general collective question of "which locals are not in scope?" (already merged):

And are there likely to be other rules that don't make sense for these file types?

potentially! We'll just need to keep iterating I suppose and get more testers

bmish added a commit that referenced this issue Mar 15, 2023
…for `gjs` / `gts` files (#2844)Co-authored-by: Bryan Mishkin <698306+bmish@users.noreply.github.com>

* Start adding gjs/gts overrides to the recommended config, per: #2830

Normally this would be a breaking change under any other circumstances, but gjs/gts folks need to disable these rules anyway, so this is more a bugfix.

* Update recommended.js

* Remove trailing invisible characters

---------

Co-authored-by: Bryan Mishkin <698306+bmish@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants