search

ember-cli / eslint-plugin-ember

An ESLint plugin that provides set of rules for Ember applications based on commonly known good practices.
MIT License
262 stars 204 forks source link

`no-empty-glimmer-component-classes` reports violation for glimmer component when `this` reference in template #1262

Open Mifrill opened 3 years ago

Mifrill commented 3 years ago

Glimmer component required js file when template contains this reference usage. In this case, we need to create a js file with empty-glimmer-component-class and it is not an error, right?

But we have eslint offense:

yarn lint:js
yarn run v1.21.1
$ eslint . --cache

/demo-ember-no-empty-glimmer-component-classes-bug-when-this-in-template/app/components/checkbox.js
  4:16  error  Do not create empty backing classes for Glimmer components  ember/no-empty-glimmer-component-classes

✖ 1 problem (1 error, 0 warnings)

error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

it force to skip this rule, which looks incorrect:

// eslint-disable-next-line ember/no-empty-glimmer-component-classes

Details: https://github.com/Mifrill/demo-ember-no-empty-glimmer-component-classes-bug-when-this-in-template/blob/main/app/components/checkbox.hbs

https://github.com/Mifrill/demo-ember-no-empty-glimmer-component-classes-bug-when-this-in-template/blob/main/app/components/checkbox.js

"ember-source": "~3.24.0",
"eslint": "^7.17.0",
"eslint-config-prettier": "^7.1.0",
"eslint-plugin-ember": "^10.1.1",
"eslint-plugin-node": "^11.1.0",
"eslint-plugin-prettier": "^3.3.1",

Related to:

Repo with bug reproduce: https://github.com/Mifrill/demo-ember-no-empty-glimmer-component-classes-bug-when-this-in-template

jelhan commented 3 years ago

Looking at your example given, I would question if the backing JavaScript class should be empty. I think it should explicitly declare the isChecked property, which is used in the template. It should also be decorated with @tracked. Current code relies on {{toogle}} helper using Ember.set(). Otherwise the property wouldn't be tracked.

Mifrill commented 3 years ago

:thinking: interesting point, my thought was about: declare the tracked property only if going to use it in js file itself, because it's working fine in a template without tracked decorator in js