Skip to content

refactor(migrations): simplify integration of tsurge migrations into the CLI#60386

Closed
crisbeto wants to merge 1 commit intoangular:mainfrom
crisbeto:tsurge-simply
Closed

refactor(migrations): simplify integration of tsurge migrations into the CLI#60386
crisbeto wants to merge 1 commit intoangular:mainfrom
crisbeto:tsurge-simply

Conversation

@crisbeto
Copy link
Copy Markdown
Member

Currently when we reuse a Tsurge migration is reused externally, there's some glue code that needs to be executed in a specific order. The code gets copied between the different migrations which is error-prone and means that bugs may have to be fixed several times.

These changes move the common steps out into a separate function so that only the migration-specific logic (mostly instantiation and logging) is left in the schematic.

@crisbeto crisbeto 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 Mar 14, 2025
@crisbeto crisbeto requested a review from devversion March 14, 2025 10:07
@angular-robot angular-robot bot added the area: migrations Issues related to `ng update`/`ng generate` migrations label Mar 14, 2025
@ngbot ngbot bot added this to the Backlog milestone Mar 14, 2025
…the CLI

Currently when we reuse a Tsurge migration is reused externally, there's some glue code that needs to be executed in a specific order. The code gets copied between the different migrations which is error-prone and means that bugs may have to be fixed several times.

These changes move the common steps out into a separate function so that only the migration-specific logic (mostly instantiation and logging) is left in the schematic.
Copy link
Copy Markdown
Member

@devversion devversion left a comment

Choose a reason for hiding this comment

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

LGTM. thanks for proactively helping with this!

getMigration: () => new UnusedImportsMigration(),
tree,
beforeProgramCreation: (tsconfigPath) => {
context.logger.info(`Preparing analysis for ${tsconfigPath}`);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should this be potentially a default log?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I was debating whether to have default logs, but I went with this approach because some migrations should be silent (e.g. InjectFlags).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sounds reasonable.

replacements = [];

for (const {info} of programInfos) {
const result = await migration.migrate(globalMeta, info);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In the future we could probably parallelize that.

@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 Mar 14, 2025
@pkozlowski-opensource
Copy link
Copy Markdown
Member

This PR was merged into the repository by commit c147a0d.

The changes were merged into the following branches: main

JoostK pushed a commit to JoostK/angular that referenced this pull request Apr 7, 2025
…the CLI (angular#60386)

Currently when we reuse a Tsurge migration is reused externally, there's some glue code that needs to be executed in a specific order. The code gets copied between the different migrations which is error-prone and means that bugs may have to be fixed several times.

These changes move the common steps out into a separate function so that only the migration-specific logic (mostly instantiation and logging) is left in the schematic.

PR Close angular#60386
atscott pushed a commit that referenced this pull request Apr 8, 2025
…the CLI (#60386) (#60776)

Currently when we reuse a Tsurge migration is reused externally, there's some glue code that needs to be executed in a specific order. The code gets copied between the different migrations which is error-prone and means that bugs may have to be fixed several times.

These changes move the common steps out into a separate function so that only the migration-specific logic (mostly instantiation and logging) is left in the schematic.

PR Close #60386

PR Close #60776
@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 Apr 14, 2025
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: major This PR is targeted for the next major release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants