Skip to content

feat(migrations): ngclass directives to class bindings#62983

Closed
aparzi wants to merge 3 commits intoangular:mainfrom
aparzi:feat-ngclass-to-class
Closed

feat(migrations): ngclass directives to class bindings#62983
aparzi wants to merge 3 commits intoangular:mainfrom
aparzi:feat-ngclass-to-class

Conversation

@aparzi
Copy link
Copy Markdown
Contributor

@aparzi aparzi commented Aug 4, 2025

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.dev application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Added Migration Feature to migrate ngClass to class

Issue Number: #61661

What is the new behavior?

It'll migrate ngClass to class bindings to follow the style guide Prefer class over ngClass

Now

<div [class.admin]="isAdmin" [class.dense]="density === 'high'">

Before

<div [ngClass]="{admin: isAdmin, dense: density === 'high'}">

Does this PR introduce a breaking change?

  • Yes
  • No

@pullapprove pullapprove bot requested a review from mmalerba August 4, 2025 12:20
@angular-robot angular-robot bot added detected: feature PR contains a feature commit area: migrations Issues related to `ng update`/`ng generate` migrations labels Aug 4, 2025
@ngbot ngbot bot added this to the Backlog milestone Aug 4, 2025
@aparzi aparzi force-pushed the feat-ngclass-to-class branch from 81c69f2 to c96deb3 Compare August 4, 2025 12:21
@aparzi
Copy link
Copy Markdown
Contributor Author

aparzi commented Aug 4, 2025

Hi @JeanMeche,
this PR adds migration for ngClass to the class directive. There's definitely room for improvement; I'm looking forward to your feedback.

JeanMeche
JeanMeche reviewed Aug 4, 2025
View reviewed changes
JeanMeche
JeanMeche reviewed Aug 4, 2025
View reviewed changes
JeanMeche
JeanMeche reviewed Aug 4, 2025
View reviewed changes
JeanMeche
JeanMeche reviewed Aug 4, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@JeanMeche JeanMeche left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for taking a look at this.
Let's start with aligning the test cases with the expectations.

We'll go through the implementation after.

@aparzi aparzi force-pushed the feat-ngclass-to-class branch from e17d83c to 7eed6fb Compare August 4, 2025 19:47
@aparzi aparzi requested a review from JeanMeche August 4, 2025 19:48
@aparzi
Copy link
Copy Markdown
Contributor Author

aparzi commented Aug 4, 2025

Hi @JeanMeche, I fix all your feedbacks. Except for related to the possible flag, I've only just currently update the relevant test case. Let me know if it's OK or if we want to proceed with a flag (if it's worth it).

JeanMeche
JeanMeche reviewed Aug 4, 2025
View reviewed changes
JeanMeche
JeanMeche reviewed Aug 4, 2025
View reviewed changes
JeanMeche
JeanMeche reviewed Aug 4, 2025
View reviewed changes
@aparzi aparzi force-pushed the feat-ngclass-to-class branch from 7eed6fb to fa20d78 Compare August 4, 2025 23:12
@aparzi
Copy link
Copy Markdown
Contributor Author

aparzi commented Aug 4, 2025

@JeanMeche
Updated PR with --migrate-space-separated-key the test cases seem to work. I just need to capture the option from the terminal, then we should be good to go.

@aparzi aparzi force-pushed the feat-ngclass-to-class branch from fa20d78 to c2a775c Compare August 5, 2025 07:54
@aparzi aparzi requested a review from JeanMeche August 5, 2025 07:54
@aparzi
Copy link
Copy Markdown
Contributor Author

aparzi commented Aug 5, 2025

@JeanMeche I updated this PR with custom option

@JeanMeche JeanMeche changed the title Feat ngclass to class feat(migrations): ngclass directives to class bindings Aug 5, 2025
JeanMeche
JeanMeche approved these changes Aug 20, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@JeanMeche JeanMeche left a comment

Choose a reason for hiding this comment

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

It's probably not perfect but I'm satisfied with what we have now !

thePunderWoman
thePunderWoman approved these changes Aug 20, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@thePunderWoman thePunderWoman left a comment

Choose a reason for hiding this comment

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

LGTM!

@aparzi aparzi force-pushed the feat-ngclass-to-class branch from 8326345 to a549ec1 Compare August 21, 2025 08:02
@thePunderWoman thePunderWoman added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Aug 21, 2025
@JeanMeche JeanMeche removed the action: merge The PR is ready for merge by the caretaker label Aug 21, 2025
aparzi added 2 commits August 21, 2025 17:51
@aparzi
feat(migrations): add migration to convert ngClass to use class
21182e0 
feat angular#61661 - add migration to convert ngClass to use class
@aparzi
refactor(migrations): centralize parseTemplate method
cdd2e13 
refactor - Moved parseTemplate logic to a shared utility function to improve code reuse and maintainability
@aparzi aparzi force-pushed the feat-ngclass-to-class branch from 6589fd0 to cdd2e13 Compare August 21, 2025 15:52
@aparzi aparzi requested a review from JeanMeche August 21, 2025 15:57
@JeanMeche JeanMeche added the action: merge The PR is ready for merge by the caretaker label Aug 21, 2025
@AndrewKushnir
Copy link
Copy Markdown
Contributor

AndrewKushnir commented Aug 21, 2025

This PR was merged into the repository. The changes were merged into the following branches:

AndrewKushnir pushed a commit that referenced this pull request Aug 21, 2025
@aparzi @AndrewKushnir
refactor(migrations): centralize parseTemplate method (#62983)
7a336c1 
refactor - Moved parseTemplate logic to a shared utility function to improve code reuse and maintainability

PR Close #62983
This was referenced Aug 29, 2025
Closed
Closed
@angular-automatic-lock-bot
Copy link
Copy Markdown

angular-automatic-lock-bot bot commented Sep 22, 2025

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@JeanMeche JeanMeche JeanMeche approved these changes

@thePunderWoman thePunderWoman thePunderWoman approved these changes

+1 more reviewer

@BBosman BBosman BBosman left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

action: merge The PR is ready for merge by the caretaker adev: preview area: migrations Issues related to `ng update`/`ng generate` migrations detected: feature PR contains a feature commit target: major This PR is targeted for the next major release

Projects

None yet

Milestone

Backlog

Development

Successfully merging this pull request may close these issues.

5 participants

@aparzi @JeanMeche @AndrewKushnir @thePunderWoman @BBosman