Skip to content

Fixes for edge cases in inject migration#58959

Closed
crisbeto wants to merge 6 commits intoangular:mainfrom
crisbeto:more-inject-edge-cases
Closed

Fixes for edge cases in inject migration#58959
crisbeto wants to merge 6 commits intoangular:mainfrom
crisbeto:more-inject-edge-cases

Conversation

@crisbeto
Copy link
Copy Markdown
Member

@crisbeto crisbeto commented Nov 28, 2024

Includes the following fixes for some edge cases in the inject migration:

fix(migrations): class content being deleted in some edge cases

Fixes that the inject migration was deleting the class' content if a property exists after the constructor that is being rewritten.

fix(migrations): correctly strip away parameters surrounded by comments in inject migration

Fixes that the inject migration was sometimes producing invalid code if there are comments around the parameters.

fix(migrations): inject migration aggressively removing imports

Fixes that the inject migration would remove imports even if they're still used by classes that aren't being migrated.

fix(migrations): preserve type literals and tuples in inject migrations

Updates the inject migration to preserve type literals and tuple types, based on some internal feedback.

fix(migrations): don't migrate classes with parameters that can't be injected

Updates the inject migrations to skip over classes that have uninjectable class parameters.

fix(migrations): inject migration dropping code if everything except super is removed

Fixes that in some cases the internal version of the migration was dropping code when all the statements after the super call are deleted.

Fixes that the inject migration was deleting the class' content if a property exists after the constructor that is being rewritten.
…ts in inject migration

Fixes that the inject migration was sometimes producing invalid code if there are comments around the parameters.
Fixes that the inject migration would remove imports even if they're still used by classes that aren't being migrated.
Updates the inject migration to preserve type literals and tuple types, based on some internal feedback.
…injected

Updates the inject migrations to skip over classes that have uninjectable class parameters.
@crisbeto crisbeto 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 Nov 28, 2024
@crisbeto crisbeto requested a review from devversion November 28, 2024 18:44
@crisbeto crisbeto changed the title Fixes edge cases in inject migration Fixes for edge cases in inject migration Nov 28, 2024
@angular-robot angular-robot bot added the area: migrations Issues related to `ng update`/`ng generate` migrations label Nov 28, 2024
@ngbot ngbot bot added this to the Backlog milestone Nov 28, 2024
…super is removed

Fixes that in some cases the internal version of the migration was dropping code when all the statements after the `super` call are deleted.
@crisbeto crisbeto 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 Nov 29, 2024
@pkozlowski-opensource
Copy link
Copy Markdown
Member

This PR was merged into the repository by commit 35165d1.

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

pkozlowski-opensource pushed a commit that referenced this pull request Dec 2, 2024
Fixes that the inject migration was deleting the class' content if a property exists after the constructor that is being rewritten.

PR Close #58959
pkozlowski-opensource pushed a commit that referenced this pull request Dec 2, 2024
…ts in inject migration (#58959)

Fixes that the inject migration was sometimes producing invalid code if there are comments around the parameters.

PR Close #58959
pkozlowski-opensource pushed a commit that referenced this pull request Dec 2, 2024
Fixes that the inject migration would remove imports even if they're still used by classes that aren't being migrated.

PR Close #58959
pkozlowski-opensource pushed a commit that referenced this pull request Dec 2, 2024
…ns (#58959)

Updates the inject migration to preserve type literals and tuple types, based on some internal feedback.

PR Close #58959
pkozlowski-opensource pushed a commit that referenced this pull request Dec 2, 2024
…injected (#58959)

Updates the inject migrations to skip over classes that have uninjectable class parameters.

PR Close #58959
pkozlowski-opensource pushed a commit that referenced this pull request Dec 2, 2024
…super is removed (#58959)

Fixes that in some cases the internal version of the migration was dropping code when all the statements after the `super` call are deleted.

PR Close #58959
pkozlowski-opensource pushed a commit that referenced this pull request Dec 2, 2024
…ts in inject migration (#58959)

Fixes that the inject migration was sometimes producing invalid code if there are comments around the parameters.

PR Close #58959
pkozlowski-opensource pushed a commit that referenced this pull request Dec 2, 2024
Fixes that the inject migration would remove imports even if they're still used by classes that aren't being migrated.

PR Close #58959
pkozlowski-opensource pushed a commit that referenced this pull request Dec 2, 2024
…ns (#58959)

Updates the inject migration to preserve type literals and tuple types, based on some internal feedback.

PR Close #58959
pkozlowski-opensource pushed a commit that referenced this pull request Dec 2, 2024
…injected (#58959)

Updates the inject migrations to skip over classes that have uninjectable class parameters.

PR Close #58959
pkozlowski-opensource pushed a commit that referenced this pull request Dec 2, 2024
…super is removed (#58959)

Fixes that in some cases the internal version of the migration was dropping code when all the statements after the `super` call are deleted.

PR Close #58959
@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 Jan 2, 2025
PrajaktaB27 pushed a commit to PrajaktaB27/angular that referenced this pull request Feb 7, 2025
…lar#58959)

Fixes that the inject migration was deleting the class' content if a property exists after the constructor that is being rewritten.

PR Close angular#58959
PrajaktaB27 pushed a commit to PrajaktaB27/angular that referenced this pull request Feb 7, 2025
…ts in inject migration (angular#58959)

Fixes that the inject migration was sometimes producing invalid code if there are comments around the parameters.

PR Close angular#58959
PrajaktaB27 pushed a commit to PrajaktaB27/angular that referenced this pull request Feb 7, 2025
…lar#58959)

Fixes that the inject migration would remove imports even if they're still used by classes that aren't being migrated.

PR Close angular#58959
PrajaktaB27 pushed a commit to PrajaktaB27/angular that referenced this pull request Feb 7, 2025
…ns (angular#58959)

Updates the inject migration to preserve type literals and tuple types, based on some internal feedback.

PR Close angular#58959
PrajaktaB27 pushed a commit to PrajaktaB27/angular that referenced this pull request Feb 7, 2025
…injected (angular#58959)

Updates the inject migrations to skip over classes that have uninjectable class parameters.

PR Close angular#58959
PrajaktaB27 pushed a commit to PrajaktaB27/angular that referenced this pull request Feb 7, 2025
…super is removed (angular#58959)

Fixes that in some cases the internal version of the migration was dropping code when all the statements after the `super` call are deleted.

PR Close angular#58959
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: migrations Issues related to `ng update`/`ng generate` migrations target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants