Skip to content

fix(eslint-plugin-template): [attributes-order] order i18n attributes #2307

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

Merged
merged 1 commit into from
Mar 22, 2025

Conversation

reduckted
Copy link
Contributor

Fixes #1519

i18n attributes are not exposed in the template AST. Instead, the element or attribute that the i18n attribute is associated with has an i18n property that stores an I18nMeta object, but that object does not define the span of the attribute, so it cannot be used to determine whether the attribute is in the correct order.

This PR changes how attributes are sourced when there is i18n data. If the element or any attribute or input has i18n metadata, then instead of getting the attributes from the template AST, Angular's HtmlParser is used to parse the element to get the raw attributes before they were manipulated into the template AST.

This technique is only used when i18n metadata exists to avoid re-parsing the attributes when they are all present in the template AST. Because the HTML parser just parses them as attributes, we need to classify them manually. This is done by looking at what the attribute starts with, and in some cases, what it ends with. For example, if it starts with * then it's a structural directive.

One difference between the attributes from the HTML parser and the AST is that structural directives from the AST don't include the leading * in their span. There were a few different places in the rule that accounted for this. I've refactored that logic into a single function so that the logic (which now needs to account for the attribute coming from the HTML parser) is all in one place.

Copy link

nx-cloud bot commented Mar 9, 2025

View your CI Pipeline Execution ↗ for commit a5b7329.

Command Status Duration Result
nx run-many -t e2e-suite --parallel 1 ✅ Succeeded 23s View ↗
nx run-many -t test --codeCoverage ✅ Succeeded 1m 10s View ↗
nx run-many -t build,typecheck,check-rule-docs,... ✅ Succeeded 42s View ↗
nx-cloud record -- pnpm nx sync:check ✅ Succeeded 4s View ↗
nx-cloud record -- pnpm format-check ✅ Succeeded 5s View ↗
nx run-many -t test ✅ Succeeded 23s View ↗
nx run-many -t build ✅ Succeeded 13s View ↗

☁️ Nx Cloud last updated this comment at 2025-03-09 09:55:36 UTC

Copy link

codecov bot commented Mar 9, 2025

Codecov Report

Attention: Patch coverage is 92.98246% with 4 lines in your changes missing coverage. Please review.

Project coverage is 92.36%. Comparing base (b55ccf1) to head (a5b7329).
Report is 14 commits behind head on main.

Files with missing lines Patch % Lines
...lint-plugin-template/src/rules/attributes-order.ts 92.98% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2307      +/-   ##
==========================================
+ Coverage   92.33%   92.36%   +0.03%     
==========================================
  Files         181      181              
  Lines        3367     3407      +40     
  Branches      685      699      +14     
==========================================
+ Hits         3109     3147      +38     
- Misses        200      201       +1     
- Partials       58       59       +1     
Flag Coverage Δ
unittest 92.36% <92.98%> (+0.03%) ⬆️

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

Files with missing lines Coverage Δ
...gin-template/tests/rules/attributes-order/cases.ts 100.00% <ø> (ø)
...lint-plugin-template/src/rules/attributes-order.ts 94.61% <92.98%> (+0.12%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Member

@JamesHenry JamesHenry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@JamesHenry JamesHenry merged commit 5c3c774 into angular-eslint:main Mar 22, 2025
7 checks passed
@reduckted reduckted deleted the bugfix/1519 branch March 22, 2025 09:32
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

Successfully merging this pull request may close these issues.

[attributes-order] autofix does not include sorting i18n attributes along with others.
2 participants