Skip to content

fix(eslint-plugin-template): [i18n] Avoid exception in i18n rule with allowMarkupInContent=false #2327

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Mar 22, 2025
Merged

fix(eslint-plugin-template): [i18n] Avoid exception in i18n rule with allowMarkupInContent=false #2327

merged 4 commits into from
Mar 22, 2025

Conversation

m-akinc
Copy link
Contributor

@m-akinc m-akinc commented Mar 18, 2025

The i18n rule in eslint-plugin-template results in an exception when the option allowMarkupInContent is set to false and a template includes an element with no i18n attribute. There was previously no test case without an i18n attribute. Fixed the issue and added a test case.

@nx-cloud Nx Cloud
Copy link

nx-cloud bot commented Mar 18, 2025 •
edited
Loading

View your CI Pipeline Execution ↗ for commit 579368f.

Command Status Duration Result
nx run-many -t e2e-suite --parallel 1 ✅ Succeeded 31s View ↗
nx run-many -t test --codeCoverage ✅ Succeeded 1m 3s View ↗
nx run-many -t build,typecheck,check-rule-docs,... ✅ Succeeded 38s 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 22s View ↗
nx run-many -t build ✅ Succeeded 11s View ↗

☁️ Nx Cloud last updated this comment at 2025-03-19 17:30:22 UTC

@m-akinc m-akinc changed the title Avoid exception in i18n rule with allowMarkupInContent=false fix: Avoid exception in i18n rule with allowMarkupInContent=false Mar 19, 2025
@codecov Codecov
Copy link

codecov bot commented Mar 19, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.33%. Comparing base (8039d52) to head (579368f).
Report is 9 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #2327   +/-   ##
=======================================
  Coverage   92.33%   92.33%           
=======================================
  Files         181      181           
  Lines        3367     3367           
  Branches      685      685           
=======================================
  Hits         3109     3109           
  Misses        200      200           
  Partials       58       58
Flag Coverage Δ
unittest 92.33% <100.00%> (ø)

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

Files with missing lines Coverage Δ
packages/eslint-plugin-template/src/rules/i18n.ts 92.64% <100.00%> (ø)
...s/eslint-plugin-template/tests/rules/i18n/cases.ts 100.00% <ø> (ø)
🚀 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.

@m-akinc m-akinc changed the title fix: Avoid exception in i18n rule with allowMarkupInContent=false fix(eslint-plugin-template): [i18n] Avoid exception in i18n rule with allowMarkupInContent=false Mar 19, 2025
@m-akinc
Copy link
Contributor Author

m-akinc commented Mar 19, 2025 •
edited
Loading

Adding @JamesHenry as reviewer. If this is incorrect, please add the correct reviewer instead. Never mind! I can't add reviewers. 😅

@m-akinc m-akinc mentioned this pull request Mar 20, 2025
Open
JamesHenry
JamesHenry approved these changes Mar 22, 2025
View reviewed changes
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.

Thank you!

@JamesHenry JamesHenry merged commit f24dc33 into angular-eslint:main Mar 22, 2025
8 checks passed
@m-akinc
Copy link
Contributor Author

m-akinc commented Mar 24, 2025

Thank you!

My pleasure. @JamesHenry any chance we could get this cherry-picked into a new version 18 release?

@m-akinc
Copy link
Contributor Author

m-akinc commented Apr 4, 2025 •
edited
Loading

@JamesHenry I don't mean to be a pest, but in case you missed it or forgot, I wanted to ping you again about getting a Angular-18-compatible release with this fix. Would that be possible?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JamesHenry JamesHenry JamesHenry approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@m-akinc @JamesHenry