ember-template-lint / ember-template-lint

Linter for Ember or Handlebars templates
MIT License
266 stars 235 forks source link

Request to add rule to ban `tagName` and `classNames` attributes to components #2043

Open tylerbecks opened 3 years ago

tylerbecks commented 3 years ago

In conjunction with require-tagless-components from eslint, it would be helpful to have a rule to ban passing these as props to a component

lifeart commented 3 years ago

it's quite hard to detect "legacy" component, this names are absolutely fine for custom glimmer components

bertdeblock commented 3 years ago

There's also the no-classic-components rule to enforce migrating to Glimmer components. No use of classic components will also mean no use of @tagName and @classNames, unless they are custom arguments on a Glimmer component, which is indeed okay.

Maybe it could be useful to have a more generic rule which people can use to disallow specific arguments in their project? So in this case you would define something like disallowList: ['tagName', 'classNames'].

tylerbecks commented 3 years ago

We are in the midst of migrating a large codebase at LinkedIn (1.6 million lines of code), so the migration will take some time. This rule would be really helpful while we're on the path to Glimmer, but still using classic components.

No use of classic components will also mean no use of @tagName and @classNames, unless they are custom arguments on a Glimmer component, which is indeed okay.

@bertdeblock Does Glimmer ban these methods? Does the component throw if these attributes are passed in?

Maybe it could be useful to have a more generic rule which people can use to disallow specific arguments in their project? So in this case you would define something like disallowList: ['tagName', 'classNames'].

YES! I LOVE this idea. THIS is a much better suggestion than simply banning tagName.

bertdeblock commented 3 years ago

Glimmer components don't have support for tagName and classNames as they don't have a wrapping element like classic components do. Everything DOM related needs to be defined inside the component template itself. Glimmer components don't throw if these names are used as arguments though, as they are still valid argument names. A generic rule would be flexible but it might have some downsides as well. It would be more difficult to provide a meaningful error saying why you shouldn't be using a specific argument, but error messages could be made configurable as well I guess. I'll leave this up for the maintainers to decide 😄.

lifeart commented 3 years ago

I like idea about custom error messages for preselected tags.

{
  "tagName": "You should not use {name}, because it's related to legacy codebase",
  "classNames": { 
     "msg": "{name} is banned for usage",
     "url": "See documentation for it"
   }
}
bertdeblock commented 3 years ago

FYI, https://github.com/ember-template-lint/ember-template-lint/issues/1870 seems like a similar issue.