Skip to content

fix(common): use === operator to match NgSwitch cases#51504

Closed
pkozlowski-opensource wants to merge 1 commit intoangular:mainfrom
pkozlowski-opensource:switch_3_equals
Closed

fix(common): use === operator to match NgSwitch cases#51504
pkozlowski-opensource wants to merge 1 commit intoangular:mainfrom
pkozlowski-opensource:switch_3_equals

Conversation

@pkozlowski-opensource
Copy link
Copy Markdown
Member

Exploratory PR to see the impact of modifying the ngSwitch directive from using == in comparisions to using ===.

@angular-robot angular-robot bot added the detected: breaking change PR contains a commit with a breaking change label Oct 4, 2023
@pkozlowski-opensource pkozlowski-opensource added the area: common Issues related to APIs in the @angular/common package label Oct 4, 2023
@ngbot ngbot bot added this to the Backlog milestone Oct 4, 2023
@pkozlowski-opensource pkozlowski-opensource added action: review The PR is still awaiting reviews from at least one requested reviewer target: major This PR is targeted for the next major release labels Oct 4, 2023
@pkozlowski-opensource pkozlowski-opensource changed the title refactor(common): explore switching switch to === fix(common): use === operator to match NgSwitch cases Oct 4, 2023
@pkozlowski-opensource pkozlowski-opensource marked this pull request as ready for review October 4, 2023 12:47
@pkozlowski-opensource
Copy link
Copy Markdown
Member Author

Marking as blocked since it will need a G3 patch

Copy link
Copy Markdown
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

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

👍

@pullapprove pullapprove bot requested a review from AndrewKushnir October 5, 2023 15:00
Copy link
Copy Markdown
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

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

Reviewed-for: public-api

@pullapprove pullapprove bot requested a review from atscott October 5, 2023 15:00
@pkozlowski-opensource pkozlowski-opensource self-assigned this Oct 9, 2023
@pkozlowski-opensource pkozlowski-opensource force-pushed the switch_3_equals branch 2 times, most recently from 38ba5c6 to fcec4ea Compare October 9, 2023 10:04
@JoostK
Copy link
Copy Markdown
Member

JoostK commented Oct 9, 2023

This PR fixes #33873

@pkozlowski-opensource pkozlowski-opensource added the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label Oct 9, 2023
This change adjust the equality comparator used by NgSwitch - now it
defaults to === from previously used ==. This change is based on the
following reasoning:
- align behaviour with the built-in switch block);
- improve performance (avoid type coercion);
- enable better type-checking.

BREAKING CHANGE:

the NgSwitch directive now defaults to the === equality operator,
migrating from the previously used ==. NgSwitch expressions and / or
individual condition values need adjusting to this stricter equality
check. The added warning message should help pinpointing NgSwitch
usages where adjustements are needed.

Fixes angular#33873
@pkozlowski-opensource pkozlowski-opensource added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note labels Oct 9, 2023
@pkozlowski-opensource
Copy link
Copy Markdown
Member Author

Adding the "cleanup" label to re-work a G3 patch

Copy link
Copy Markdown
Member

@alxhub alxhub left a comment

Choose a reason for hiding this comment

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

Reviewed-for: public-api

@pkozlowski-opensource pkozlowski-opensource added action: merge The PR is ready for merge by the caretaker merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews action: review The PR is still awaiting reviews from at least one requested reviewer labels Oct 9, 2023
@pkozlowski-opensource
Copy link
Copy Markdown
Member Author

caretaker note: this PR will require CL https://critique.corp.google.com/cl/571889513

@atscott
Copy link
Copy Markdown
Contributor

atscott commented Oct 9, 2023

This PR was merged into the repository by commit 28a5925.

@atscott atscott closed this in 28a5925 Oct 9, 2023
@angular-automatic-lock-bot
Copy link
Copy Markdown

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 Nov 9, 2023
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
This change adjust the equality comparator used by NgSwitch - now it
defaults to === from previously used ==. This change is based on the
following reasoning:
- align behaviour with the built-in switch block);
- improve performance (avoid type coercion);
- enable better type-checking.

BREAKING CHANGE:

the NgSwitch directive now defaults to the === equality operator,
migrating from the previously used ==. NgSwitch expressions and / or
individual condition values need adjusting to this stricter equality
check. The added warning message should help pinpointing NgSwitch
usages where adjustements are needed.

Fixes angular#33873

PR Close angular#51504
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: common Issues related to APIs in the @angular/common package detected: breaking change PR contains a commit with a breaking change merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: major This PR is targeted for the next major release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants