Skip to content

fix(core): fix removal of a container reference used in the component…#60210

Closed
aparzi wants to merge 1 commit intoangular:mainfrom
aparzi:fix-issue-56565
Closed

fix(core): fix removal of a container reference used in the component…#60210
aparzi wants to merge 1 commit intoangular:mainfrom
aparzi:fix-issue-56565

Conversation

@aparzi
Copy link
Copy Markdown
Contributor

@aparzi aparzi commented Mar 5, 2025

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix

What is the current behavior?

ng generate @angular/core:control-flow migration removes the container reference even if it is used in component file

Issue Number: #56565

What is the new behavior?

During migration a container reference was deleted even though it was used in the component file.

Does this PR introduce a breaking change?

  • Yes
  • No

@pullapprove pullapprove bot requested a review from crisbeto March 5, 2025 00:08
@angular-robot angular-robot bot added the area: core Issues related to the framework runtime label Mar 5, 2025
@ngbot ngbot bot added this to the Backlog milestone Mar 5, 2025
@JeanMeche
Copy link
Copy Markdown
Member

JeanMeche commented Mar 5, 2025 •
edited
Loading

Could you please capture the fix in a unit test in packages/core/schematics/test/control_flow_migration_spec.ts.
You can run the tests with yarn bazel test //packages/core/schematics/test/....

Thank you.

@aparzi aparzi force-pushed the fix-issue-56565 branch from 91f5ba3 to 731f698 Compare March 5, 2025 08:45
@aparzi
Copy link
Copy Markdown
Contributor Author

aparzi commented Mar 5, 2025

Could you please capture the fix in a unit test in packages/core/schematics/test/control_flow_migration_spec.ts. You can run the tests with yarn bazel test //packages/core/schematics/test/....

Thank you.

Hi @JeanMeche I Integrated unit test, based on the issue template

JeanMeche
JeanMeche reviewed Mar 5, 2025
View reviewed changes
@aparzi aparzi force-pushed the fix-issue-56565 branch from 731f698 to b96e19a Compare March 5, 2025 17:23
@aparzi
Copy link
Copy Markdown
Contributor Author

aparzi commented Mar 5, 2025 •
edited
Loading

Hi @JeanMeche,
I updated the PR with managing ViewChildren. I created a dedicated function so as to keep the management of the different decorators separate.

Thanks.

@aparzi aparzi requested a review from JeanMeche March 6, 2025 14:55
@alan-agius4 alan-agius4 added action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release labels Mar 7, 2025
@aparzi aparzi force-pushed the fix-issue-56565 branch from b96e19a to 6d87898 Compare March 7, 2025 10:59
@JeanMeche JeanMeche requested a review from thePunderWoman March 7, 2025 11:05
JeanMeche
JeanMeche reviewed Mar 7, 2025
View reviewed changes
@aparzi aparzi force-pushed the fix-issue-56565 branch from 6d87898 to 0f10584 Compare March 9, 2025 00:13
@aparzi aparzi requested a review from JeanMeche March 9, 2025 00:14
JeanMeche
JeanMeche reviewed Mar 9, 2025
View reviewed changes
JeanMeche
JeanMeche approved these changes Mar 9, 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.

Beside the nit, LGTM.

Would also like to have @thePunderWoman's approval on this!

@aparzi
Copy link
Copy Markdown
Contributor Author

aparzi commented Mar 9, 2025

Beside the nit, LGTM.

Would also like to have @thePunderWoman's approval on this!

Ok @JeanMeche I'll make the improvement as soon as possible. Thanks

@aparzi aparzi force-pushed the fix-issue-56565 branch from 0f10584 to 20ccd8e Compare March 9, 2025 09:58
@aparzi aparzi requested a review from JeanMeche March 9, 2025 10:01
@crisbeto crisbeto removed their request for review March 9, 2025 11:42
JeanMeche
JeanMeche reviewed Mar 9, 2025
View reviewed changes
@aparzi aparzi force-pushed the fix-issue-56565 branch from 20ccd8e to cb030f0 Compare March 9, 2025 20:06
@aparzi aparzi requested a review from JeanMeche March 9, 2025 20:06
@aparzi
fix(core): fix removal of a container reference used in the component…
827aecf 
… file

During migration a container reference was deleted even though it was used in the component file.
@aparzi aparzi force-pushed the fix-issue-56565 branch from cb030f0 to 827aecf Compare March 9, 2025 22:56
thePunderWoman
thePunderWoman approved these changes Mar 10, 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! Thank you for taking the time to fix this and to add the unit tests.

@thePunderWoman thePunderWoman removed the action: review The PR is still awaiting reviews from at least one requested reviewer label Mar 10, 2025
@thePunderWoman thePunderWoman added the action: merge The PR is ready for merge by the caretaker label Mar 10, 2025
@thePunderWoman thePunderWoman removed the request for review from JeanMeche March 10, 2025 16:32
@AndrewKushnir AndrewKushnir added the action: presubmit The PR is in need of a google3 presubmit label Mar 10, 2025
@AndrewKushnir
Copy link
Copy Markdown
Contributor

AndrewKushnir commented Mar 10, 2025

Presubmit.

@thePunderWoman
Copy link
Copy Markdown
Contributor

thePunderWoman commented Mar 11, 2025

Caretaker note: presubmit failures are flakes. This is safe to merge.

@thePunderWoman thePunderWoman removed the action: presubmit The PR is in need of a google3 presubmit label Mar 11, 2025
@AndrewKushnir
Copy link
Copy Markdown
Contributor

AndrewKushnir commented Mar 11, 2025

This PR was merged into the repository by commit de2bfc0.

The changes were merged into the following branches: main, 19.2.x

AndrewKushnir pushed a commit that referenced this pull request Mar 11, 2025
@aparzi @AndrewKushnir
fix(core): fix removal of a container reference used in the component…
b144dd9 
… file (#60210)

During migration a container reference was deleted even though it was used in the component file.

PR Close #60210
@pkozlowski-opensource pkozlowski-opensource mentioned this pull request Mar 26, 2025
Closed
@angular-automatic-lock-bot
Copy link
Copy Markdown

angular-automatic-lock-bot bot commented Apr 11, 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 Apr 11, 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

Assignees

No one assigned

Labels

action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime target: patch This PR is targeted for the next patch release

Projects

None yet

Milestone

Backlog

Development

Successfully merging this pull request may close these issues.

5 participants

@aparzi @JeanMeche @AndrewKushnir @thePunderWoman @alan-agius4