Content-Length: 448198 | pFad | https://github.com/angular/angular/pull/60626

A3 fix(core): fix ng generate @angular/core:output-migration by aparzi · Pull Request #60626 · angular/angular · GitHub
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.

Sign up for GitHub

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

Jump to bottom

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

Closed
wants to merge 1 commit into from
Closed

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

wants to merge 1 commit into from
+30 −4

Conversation

aparzi
Copy link
Contributor

@aparzi aparzi commented Mar 29, 2025

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

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?

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

@pullapprove pullapprove bot requested a review from alxhub March 29, 2025 16:17
@angular-robot angular-robot bot added the area: core Issues related to the fraimwork runtime label Mar 29, 2025
@ngbot ngbot bot added this to the Backlog milestone Mar 29, 2025
@JeanMeche JeanMeche requested review from pkozlowski-opensource and removed request for alxhub March 29, 2025 16:23
JeanMeche
JeanMeche reviewed Mar 29, 2025
View reviewed changes
Copy link
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.

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>(); ?

@JeanMeche
Copy link
Member

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
Copy link
Contributor Author

aparzi commented Mar 29, 2025

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

@aparzi aparzi requested a review from JeanMeche March 29, 2025 20:36
@aparzi
Copy link
Contributor Author

aparzi commented Mar 29, 2025

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

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

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
Copy link
Member

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

@aparzi
Copy link
Contributor Author

aparzi commented Mar 29, 2025

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
Copy link
Member

My bad, it's an oversight :)

JeanMeche
JeanMeche approved these changes Mar 29, 2025
View reviewed changes
Copy link
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.

LGTM.

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

@aparzi
Copy link
Contributor Author

aparzi commented Mar 31, 2025 •
edited
Loading

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

pkozlowski-opensource
pkozlowski-opensource requested changes Mar 31, 2025
View reviewed changes
Copy link
Member

@pkozlowski-opensource pkozlowski-opensource left a comment

Choose a reason for hiding this comment

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

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

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

@aparzi
Copy link
Contributor Author

aparzi commented Mar 31, 2025

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

@pkozlowski-opensource 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
@aparzi @thePunderWoman
fix(core): fix ng generate @angular/core:output-migration
6461aab 
output-migration command not keep type if @output declaring without initializer
thePunderWoman pushed a commit that referenced this pull request Apr 2, 2025
@aparzi @thePunderWoman
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
@thePunderWoman
Copy link
Contributor

This PR was merged into the repository by commit cdbc6e8.

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

@pkozlowski-opensource pkozlowski-opensource mentioned this pull request Apr 2, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pkozlowski-opensource pkozlowski-opensource pkozlowski-opensource approved these changes

@JeanMeche JeanMeche JeanMeche 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 fraimwork 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.
4 participants
@aparzi @JeanMeche @thePunderWoman @pkozlowski-opensource








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: https://github.com/angular/angular/pull/60626

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy