Skip to content
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

fix(eslint-plugin-template): [prefer-self-closing-tags] consider ng-content and ng-template elements #1573

Conversation

json-derulo
Copy link
Contributor

@json-derulo json-derulo commented Nov 6, 2023

Currently the prefer-self-closing-tags rule doesn't report errors on empty <ng-content></ng-content> and <ng-template></ng-template> elements, which is fixed by this pull request.

Closes #1567

Copy link

nx-cloud bot commented Nov 6, 2023

☁️ Nx Cloud Report

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

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 7 targets

Sent with 💌 from NxCloud.

@json-derulo json-derulo force-pushed the self-closing-tags-fixes branch 2 times, most recently from 38d5f20 to ea26ccc Compare November 6, 2023 18:23
Copy link

codecov bot commented Nov 6, 2023

Codecov Report

Merging #1573 (2c2a4c5) into main (59b3204) will increase coverage by 0.01%.
The diff coverage is 95.45%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1573      +/-   ##
==========================================
+ Coverage   89.55%   89.57%   +0.01%     
==========================================
  Files         166      166              
  Lines        3113     3127      +14     
  Branches      526      530       +4     
==========================================
+ Hits         2788     2801      +13     
  Misses        195      195              
- Partials      130      131       +1     
Flag Coverage Δ
unittest 89.57% <95.45%> (+0.01%) ⬆️

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

Files Coverage Δ
...late/tests/rules/prefer-self-closing-tags/cases.ts 100.00% <ø> (ø)
...gin-template/src/rules/prefer-self-closing-tags.ts 97.29% <95.45%> (-2.71%) ⬇️

@json-derulo json-derulo force-pushed the self-closing-tags-fixes branch 3 times, most recently from 65b954e to d343e5e Compare November 6, 2023 19:00
@JamesHenry
Copy link
Member

That's because this didn't used to be supported in Angular:

image

I don't know when they stopped throwing the above parse error for these but happy to learn that they did.

@JamesHenry JamesHenry changed the base branch from main to next-major-release/v17 November 7, 2023 16:20
@JamesHenry JamesHenry merged commit 8e97d20 into angular-eslint:next-major-release/v17 Nov 7, 2023
15 checks passed
@JamesHenry
Copy link
Member

Thanks @json-derulo I've added this to the v17 branch just in case this change in behaviour is particularly disruptive for larger codebases

@json-derulo json-derulo deleted the self-closing-tags-fixes branch November 7, 2023 17:05
@JamesHenry
Copy link
Member

@json-derulo I'm afraid I've had to revert for the two plugins, it causes compilation issues with typescript-eslint v6

@json-derulo
Copy link
Contributor Author

@JamesHenry Could you explain in more detail what the problem with typescript-eslint v6 is? I checked out the next-major-release/v17 branch (which still includes my commit) and after installing the dependencies, I ran lint, test & build. Everything seems to work fine for me.

@JamesHenry
Copy link
Member

JamesHenry commented Nov 7, 2023

It includes your commit but then the aforementioned revert to the plugins by me: 2498238

The issue is typescript-eslint/typescript-eslint#7605

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.

[@angular-eslint/template/prefer-self-closing-tags] ng-content should be marked as self-closing tag
2 participants