fix(mdc-migration): migrate components selectively bug

[llorenspujol]

Fixes mdc-migration to migrate only the selected components.

Fixes #26426

[wagnermaciel]

LGTM

[mmalerba]

@llorenspujol It looks like this change caused it to stop migrating legacy-core when using --components all. I've added a commit with the changes to the golden test so you can see what changed (easier to look at the github diff than the test output). In order to merge we'll need to fix. You can run yarn bazel run //integration/mdc-migration:test.approve to re-approve the golden test changes after making changes. Once fixed properly, it should just negate the changes in the commit I added.

[llorenspujol]

@llorenspujol It looks like this change caused it to stop migrating legacy-core when using --components all. I've added a commit with the changes to the golden test so you can see what changed (easier to look at the github diff than the test output). In order to merge we'll need to fix. You can run yarn bazel run //integration/mdc-migration:test.approve to re-approve the golden test changes after making changes. Once fixed properly, it should just negate the changes in the commit I added.

Yes, I see, thanks for the commit. What is happening here is that MatLegacyOptionModule comes from 'legacy-core', not 'legacy-option', so it fails.
I just went straightforward, and to solve it, I always replace imports that come from 'legacy-core'. This solution is not 100% perfect, since if one user is not migrating any component related to MatOptionModule, it will either way migrate it. I think it is an edge case thought, since it may be very strange using the MatOptionModule alone, but it can happen.

The perfect solution may be one that resolves this test:

it('should not replace only mat legacy option module if not specified', async () => {
    await runMigrationTest(
      `
        import {NgModule} from '@angular/core';
        import {MatLegacyOptionModule, LEGACY_VERSION} from '@angular/material/legacy-core';
        @NgModule({imports: [MatLegacyOptionModule]})
        export class AppModule {}
      `,
      `
        import {NgModule} from '@angular/core';
        import {MatLegacyOptionModule} from '@angular/material/legacy-core';
        import {VERSION} from '@angular/material/core';
        @NgModule({imports: [MatLegacyOptionModule]})
        export class AppModule {}
      `,
    );
  });

In this test the LEGACY_VERSION is migrated, but not the MatLegacyOptionModule. IMO I think is really an edge case, and we can go with it as it is. That being said, if this test needs to be solved for X reasons, we can do it, I just want to first validate the task before putting any more effort on it!

[michaelfaith]

One thought, is you could have a separate const array that consists of the options.components that are exported from legacy-core (it may just be option in this case) and change the first condition to check if

(lastImportName  === 'legacy-core' && CORE_COMPONENTS.includes(componentMigrator.component))

[mmalerba]

I think it should be fine to only migrate core symbols when option or optgroup is chosen for migration. A check like the one @michaelfaith suggested should work for that

[llorenspujol]

Agreed, it makes more sense this way. I added the necessary changes to only migrate core symbols if option or optgroup is chosen for migration. The code is basically as @michaelfaith pointed out 👌

[michaelfaith]

I think all of the feedback items have been addressed on this. Or is there more to do?

[mmalerba]

LGTM

[angular-automatic-lock-bot]

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