feat(eslint-plugin-template): [no-inline-styles] add rule

[pumano]

It's (subjectively) good practice to separate styles from the view layout, when possible. This rule detects inline styles in angular templates and help to keep all css in one place (for example in .css files, or use tailwindcss classes). Also inline styles has different priority in CSS and that cause some bugs.

I also found some rule https://github.com/Intellicode/eslint-plugin-react-native/blob/master/docs/rules/no-inline-styles.md but that rule for JSX, and I created own using @angular-eslint tooling and in using angular-eslint style.

Previously I create that rule as custom rule for my nx workspace rules (for our team needs), but have idea to share it with community.

[nx-cloud]

☁️ Nx Cloud Report

CI is running/has finished running commands for commit e9b7c79. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

---

✅ Successfully ran 7 targets

• Node v16.18.1 - nx run-many --target=integration-test --all --withRegistry
• Node v16.18.1 - nx run-many --target=test --all --parallel --codeCoverage
• Node v16.18.1 - nx run-many --target=typecheck --all --parallel
• Node v16.18.1 - nx run-many --target=build --all --parallel
• Node v16.18.1 - nx run-many --target=check-rule-docs --all --parallel
• Node v14.21.1 - nx run-many --target=test --all --parallel
• Node v14.21.1 - nx run-many --target=build --all --parallel

---

Sent with 💌 from NxCloud.

[pumano]

@JamesHenry could you check this PR please :)

[JamesHenry]

Thinking out load, should this rule also prohibit ngStyle usage (based on my understanding of your intent with it)?

Maybe that could be configured via an option on the rule? 🤔 WDYT?

[pumano]

Thinking out load, should this rule also prohibit ngStyle usage (based on my understanding of your intent with it)?

Maybe that could be configured via an option on the rule? 🤔 WDYT?

Nice idea @JamesHenry, but I have no experience to create rule with params, I will try to add this tomorrow, that's really needed

[jerone]

And what about the style prefix: https://angular.io/guide/class-binding#binding-to-a-single-style

<nav [style.background-color]="expression"></nav>

Aldo, I would actually allow this.
Maybe also enabling this case via an option.

[pumano]

And what about the style prefix: https://angular.io/guide/class-binding#binding-to-a-single-style

<nav [style.background-color]="expression"></nav>

Aldo, I would actually allow this. Maybe also enabling this case via an option.

Looks like it's not possible to block it, because we can't obtain that is style binding, only binding attribute aka background-color, while when [attr.style] directly binding to attributes.

Only possible do it via array of all css properties for forbid it, and that looks complicated story. If AST provide that
style from [style.background-color] somehow, it's easy to support.

[pumano]

Thinking out load, should this rule also prohibit ngStyle usage (based on my understanding of your intent with it)?

Maybe that could be configured via an option on the rule? 🤔 WDYT?

ngStyle support added via options

[jerone]

I understand this might be a bit out of scope for the first iteration of this feature. Might be a good addition for later.
Anyways, some information for future iterations:

Looks like it's not possible to block it, because we can't obtain that is style binding

It looks like there is an property BoundAttribute.keySpan.details that contains the full string. Maybe we can find BoundAttribute that start with style.* value.

Only possible do it via array of all css properties for forbid it

It appears that eslint-plugin-css is using such list: known-css-properties.

[pumano]

@jerone thank you for your findings, I will try to add it too.

[pumano]

@jerone check please, I add support for [style.cssproperty] style binding too.

[pumano]

@jerone I apply your suggestion

[pumano]

@JamesHenry all cases are implemented

[JamesHenry]

@pumano as well as the above you'll please need to sync up with latest main

[pumano]

@JamesHenry I apply your suggestion and rebase from main

[codecov]

Codecov Report

Merging #1162 (e9b7c79) into main (d753626) will increase coverage by 0.09%.
The diff coverage is 96.77%.

@@            Coverage Diff             @@
##             main    #1162      +/-   ##
==========================================
+ Coverage   88.07%   88.16%   +0.09%     
==========================================
  Files         159      161       +2     
  Lines        3053     3085      +32     
  Branches      491      498       +7     
==========================================
+ Hits         2689     2720      +31     
  Misses        253      253              
- Partials      111      112       +1     

Flag	Coverage Δ
unittest	88.16% <96.77%> (+0.09%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
packages/eslint-plugin-template/src/index.ts	69.23% <ø> (+0.60%)	⬆️
...lint-plugin-template/src/rules/no-inline-styles.ts	96.00% <96.00%> (ø)
...gin-template/tests/rules/no-inline-styles/cases.ts	100.00% <100.00%> (ø)

[JamesHenry]

Thanks a lot @pumano!