Skip to content

fix(eslint-plugin-template): [prefer-template-literal] handle nested and concatenations in template literal #2466

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 7 commits into from
Jun 2, 2025
Merged

fix(eslint-plugin-template): [prefer-template-literal] handle nested and concatenations in template literal #2466

merged 7 commits into from
Jun 2, 2025
+490 −171

Conversation

g-drouard
Copy link
Contributor

@g-drouard g-drouard commented May 27, 2025 •
edited
Loading

resolves #2462

Explanation

Autofix steps of a + ' ' + b + ' ' + c are:

  1. `${a + ' ' + b} ` + c
  2. `${a + ' ' + b} ${c}`
  3. `${`${a} ` + b} ${c}`
  4. `${`${a} ${b}`} ${c}`

This fix handle nested and concatenations in template literal to avoid this result.

Angular bug with quotes in nested template literals with @let (including in Angular 20)

Example : @let bugWithQuote = `${`'`}`; (same with back quote and double quote)

Parsing error : X [ERROR] NG5002: Parser Error: Lexer Error: Unexpected character [@] at column 17 in expression [`${`'`}`;

I will write an Angular issue.

Bug when the first line breaks

This is the cause of all others bugs reported from #2425.

I will create an issue.

Works

<ng-container
    *ngTemplateOutlet="selector;
        context: {
            name: 'test-' + item.id
            value: 42
        }
    "
/>


<div></div>

Current fix :

<ng-container
    *ngTemplateOutlet="selector;
        context: {
            name:  `test-${item.id}`
            value: 42
        }
    "
/>


<div></div>

Doesn't work

<ng-container
    *ngTemplateOutlet="
        selector;
        context: {
            name: 'test-' + item.id
            value: 42
        }
    "
/>


<div></div>

Current fix :

<ng-container
    *ngTemplateOutlet="
        selector;
        context: {
            name: 'test-' + `test-${}`div></div>

Explanation

When the first line breaks, sourceSpan positions are wrong.
I added tests and investigated it for several hours, but I don't have enough knowledge of how it works to fix this.

@nx-cloud Nx Cloud
Copy link

nx-cloud bot commented May 27, 2025 •
edited
Loading

View your CI Pipeline Execution ↗ for commit 49631e6.

Command Status Duration Result
nx run-many -t e2e-suite --parallel 1 ✅ Succeeded 34s View ↗
nx run-many -t test --codeCoverage ✅ Succeeded 58s View ↗
nx run-many -t build,typecheck,check-rule-docs,... ✅ Succeeded 1m 8s View ↗
nx-cloud record -- pnpm nx sync:check ✅ Succeeded 2s View ↗
nx-cloud record -- pnpm format-check ✅ Succeeded 6s View ↗
nx run-many -t test ✅ Succeeded 24s View ↗
nx run-many -t build ✅ Succeeded 12s View ↗

☁️ Nx Cloud last updated this comment at 2025-06-02 08:38:37 UTC

@codecov Codecov
Copy link

codecov bot commented May 27, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 97.87234% with 1 line in your changes missing coverage. Please review.

Project coverage is 92.91%. Comparing base (74a854c) to head (49631e6).
Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
...ugin-template/src/rules/prefer-template-literal.ts 97.87% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2466      +/-   ##
==========================================
+ Coverage   92.85%   92.91%   +0.06%     
==========================================
  Files         200      200              
  Lines        4169     4220      +51     
  Branches      973      993      +20     
==========================================
+ Hits         3871     3921      +50     
- Misses        229      230       +1     
  Partials       69       69
Flag Coverage Δ
unittest 92.91% <97.87%> (+0.06%) ⬆️

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

Files with missing lines Coverage Δ
...int-plugin-template/src/utils/literal-primitive.ts 100.00% <ø> (ø)
...plate/tests/rules/prefer-template-literal/cases.ts 100.00% <ø> (ø)
...ugin-template/src/rules/prefer-template-literal.ts 98.64% <97.87%> (+0.31%) ⬆️

... and 5 files with indirect coverage changes

🚀 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.

@g-drouard g-drouard changed the title fix(eslint-plugin-template): [prefer-telmplate-literal] handle nested and concatenations in template literal fix(eslint-plugin-template): [prefer-template-literal] handle nested and concatenations in template literal May 27, 2025
@g-drouard g-drouard marked this pull request as ready for review June 1, 2025 01:20
@JamesHenry
Copy link
Member

Please could you add a failing commented out test for the Angular issue and link to the Angular issue on GitHub and also link to the bugs in question in your existing commented out tests? And add a TODO comment if they simply require more investigation on our side, just so they are easier to find and track as follow ups

JamesHenry
JamesHenry requested changes Jun 1, 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.

Thanks!

@JamesHenry
Copy link
Member

I'm going to do it now so that I can get it out in the final v19 release

@JamesHenry
Copy link
Member

Apart from this one, please let me know what the Angular issue is:

Angular bug with quotes in nested template literals with @let (including in Angular 20)
Example : @let bugWithQuote = ${'}; (same with back quote and double quote)

Parsing error : X [ERROR] NG5002: Parser Error: Lexer Error: Unexpected character [@] at column 17 in expression [${'};

I will write an Angular issue.

@JamesHenry
Copy link
Member

@g-drouard Hmm I didn't get a parse error locally on v19 a2be68f

@JamesHenry JamesHenry merged commit 7930f40 into angular-eslint:main Jun 2, 2025
8 checks passed
@JamesHenry
Copy link
Member

Ah I do on the Angular 20 branch, I had to comment it out: https://github.com/angular-eslint/angular-eslint/pull/2448/files#diff-2989d9c9a6837b274845d66ef14d3d3d8cdd4ea1d6aa0323b4c8b9f035059a13R26

Has it already been reported to the Angular Team?

@g-drouard
Copy link
Contributor Author

Sorry for the delay in my response, I was sick.

I create an Angular issue.

I reproduce the bug well on version 19 or 20 of Angular. It's very strange that the rule test doesn't trigger an error.

Regarding the line break bug, I have no idea if the problem comes from the Angular parser, the Angular-eslint parser, or even the rule code.
Do I create an angular-eslint issue for this bug or continue investigating?

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

@JamesHenry JamesHenry JamesHenry requested changes

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

Successfully merging this pull request may close these issues.

[prefer-template-literal] auto-fix does not properly handle multiple concatenations
2 participants
@g-drouard @JamesHenry