fix(core): fix ng generate @angular/core:output-migration by aparzi · Pull Request #60626 · angular/angular

aparzi · 2025-03-29T16:17:35Z

output-migration command not keep type if @output declaring without initializer

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/main/contributing-docs/commit-message-guidelines.md
Tests for the changes have been added (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

Bugfix

What is the current behavior?

The output-migration command not keep type if @output declaring without initializer

Issue Number: #59248

What is the new behavior?

The output-migration command keep type

Does this PR introduce a breaking change?

Yes
No

packages/core/schematics/migrations/output-migration/output-replacements.ts

packages/core/schematics/test/output_migration_spec.ts

JeanMeche

Looks like we're going in the right direction with this.

Can we also add a test that covers the migration of @Output() eventMovement = new EventEmitter<IResponse>(); ?

packages/core/schematics/test/output_migration_spec.ts

JeanMeche · 2025-03-29T16:42:10Z

Also please look at the linting failures, we have a formatting issue.

Note: The currently broken tests are flakes and related to this change.

aparzi · 2025-03-29T16:46:08Z

Looks like we're going in the right direction with this.

Can we also add a test that covers the migration of @Output() eventMovement = new EventEmitter<IResponse>(); ?

Sure

packages/core/schematics/migrations/output-migration/output-replacements.ts

aparzi · 2025-03-29T20:37:12Z

Also please look at the linting failures, we have a formatting issue.

Note: The currently broken tests are flakes and related to this change.

fix lint errors

JeanMeche · 2025-03-29T20:41:37Z

We're still missing the test to cover @Output() eventMovement = new EventEmitter<IResponse>();

aparzi · 2025-03-29T20:45:14Z

We're still missing the test to cover @Output() eventMovement = new EventEmitter<IResponse>();

I added this test:
@Output() eventMovement: EventEmitter = new EventEmitter<IResponse>();

do I change it?

JeanMeche · 2025-03-29T20:46:26Z

My bad, it's an oversight :)

JeanMeche

LGTM.

However, this is outside of my review scope. I'll let Pawel do a 2nd review.

aparzi · 2025-03-31T07:33:15Z

I checked also inputs migration command and seems that behaviour it'ok. That command doesn't have the same bug that the output did.

packages/core/schematics/test/output_migration_spec.ts

pkozlowski-opensource

Thnx so much for the bug fix, appreciate it ❤️

The logic LGTM, just want to ask you to move tests around.

aparzi · 2025-03-31T17:17:30Z

Thnx so much for the bug fix, appreciate it ❤️

The logic LGTM, just want to ask you to move tests around.

@pkozlowski-opensource I have updated with what was requested. Thanks for your availability

For the rest it's a pleasure

@output

output-migration command not keep type if @output declaring without initializer

@output

output-migration command not keep type if @output declaring without initializer PR Close #60626

thePunderWoman · 2025-04-02T14:26:10Z

This PR was merged into the repository by commit cdbc6e8.

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

angular-automatic-lock-bot · 2025-05-03T00:12:49Z

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

pullapprove bot requested a review from alxhub March 29, 2025 16:17

angular-robot bot added the area: core Issues related to the framework runtime label Mar 29, 2025

ngbot bot added this to the Backlog milestone Mar 29, 2025

JeanMeche requested review from pkozlowski-opensource and removed request for alxhub March 29, 2025 16:23

JeanMeche reviewed Mar 29, 2025

View reviewed changes

packages/core/schematics/migrations/output-migration/output-replacements.ts Outdated Show resolved Hide resolved

JeanMeche reviewed Mar 29, 2025

View reviewed changes

packages/core/schematics/test/output_migration_spec.ts Outdated Show resolved Hide resolved

JeanMeche reviewed Mar 29, 2025

View reviewed changes

packages/core/schematics/test/output_migration_spec.ts Outdated Show resolved Hide resolved

JeanMeche reviewed Mar 29, 2025

View reviewed changes

packages/core/schematics/migrations/output-migration/output-replacements.ts Outdated Show resolved Hide resolved

aparzi force-pushed the fix-issue-59248 branch from ea0704a to af84609 Compare March 29, 2025 20:35

aparzi requested a review from JeanMeche March 29, 2025 20:36

JeanMeche approved these changes Mar 29, 2025

View reviewed changes

pkozlowski-opensource reviewed Mar 31, 2025

View reviewed changes

packages/core/schematics/test/output_migration_spec.ts Outdated Show resolved Hide resolved

pkozlowski-opensource reviewed Mar 31, 2025

View reviewed changes

packages/core/schematics/test/output_migration_spec.ts Outdated Show resolved Hide resolved

pkozlowski-opensource requested changes Mar 31, 2025

View reviewed changes

aparzi force-pushed the fix-issue-59248 branch from af84609 to 5f942e0 Compare March 31, 2025 17:15

aparzi requested a review from pkozlowski-opensource March 31, 2025 17:29

aparzi force-pushed the fix-issue-59248 branch from 5f942e0 to ff4cbfe Compare March 31, 2025 17:31

pkozlowski-opensource approved these changes Apr 2, 2025

View reviewed changes

pkozlowski-opensource added action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release labels Apr 2, 2025

fix(core): fix ng generate @angular/core:output-migration

6461aab

output-migration command not keep type if @output declaring without initializer

thePunderWoman force-pushed the fix-issue-59248 branch from ff4cbfe to 6461aab Compare April 2, 2025 14:11

thePunderWoman closed this in cdbc6e8 Apr 2, 2025

thePunderWoman pushed a commit that referenced this pull request Apr 2, 2025

fix(core): fix ng generate @angular/core:output-migration (#60626)

8f68d1b

output-migration command not keep type if @output declaring without initializer PR Close #60626

pkozlowski-opensource mentioned this pull request Apr 2, 2025

output-migration Removes types from output #59248

Closed

angular-automatic-lock-bot bot locked and limited conversation to collaborators May 3, 2025

Conversation

aparzi commented Mar 29, 2025

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Uh oh!

Uh oh!

Uh oh!

JeanMeche left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JeanMeche commented Mar 29, 2025

Uh oh!

aparzi commented Mar 29, 2025

Uh oh!

Uh oh!

aparzi commented Mar 29, 2025

Uh oh!

JeanMeche commented Mar 29, 2025

Uh oh!

aparzi commented Mar 29, 2025

Uh oh!

JeanMeche commented Mar 29, 2025

Uh oh!

JeanMeche left a comment

Choose a reason for hiding this comment

Uh oh!

aparzi commented Mar 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pkozlowski-opensource left a comment

Choose a reason for hiding this comment

Uh oh!

aparzi commented Mar 31, 2025

Uh oh!

thePunderWoman commented Apr 2, 2025

Uh oh!

angular-automatic-lock-bot bot commented May 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aparzi commented Mar 31, 2025 •

edited

Loading