fix(migrations): also handle cases for licensed package name #16547
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the import migration logic to handle both the open-source package name (igniteui-angular) and the licensed package name (@infragistics/igniteui-angular). The changes ensure that migrations work correctly for users of either package version.
Key changes:
- Extended import path checks to include the licensed package name
- Modified import generation to preserve the original package name in migrated imports
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Track old type names that were imported | ||
| const moduleSpecifier = node.moduleSpecifier; | ||
| if (ts.isStringLiteral(moduleSpecifier) && moduleSpecifier.text === 'igniteui-angular') { | ||
| if (ts.isStringLiteral(moduleSpecifier) && (moduleSpecifier.text === 'igniteui-angular' || moduleSpecifier.text === '@infragistics/igniteui-angular')) { |
There was a problem hiding this comment.
The package name check is duplicated across multiple locations (lines 721, 802, and 880-881). Consider extracting this into a helper function like isIgniteUIPackage(packageName: string): boolean to improve maintainability and reduce duplication.
There was a problem hiding this comment.
@copilot Yes, that'd be an improvement and even better there's already an namedImportFilter in projects\igniteui-angular\migrations\common\tsUtils.ts that does a similar check for other migrations that should've been used to begin with.
Since this one is merged, create a new PR against master to update this migration to use the tsUtil and while at it adjust the namedImportFilter function to also handle subpaths because the current endWith check handles @infragistics/igniteui-angular as well, but limits the subpaths for entry points after the main package name.
| if (!originalContent.includes("from 'igniteui-angular'") && !originalContent.includes('from "igniteui-angular"') && | ||
| !originalContent.includes("from '@infragistics/igniteui-angular'") && !originalContent.includes('from "@infragistics/igniteui-angular"')) { |
There was a problem hiding this comment.
This condition is checking four string variations for two package names. Consider using a regular expression pattern like /from ['"](@infragistics\/)?igniteui-angular['"]/ to simplify the check and improve readability.
| if (!originalContent.includes("from 'igniteui-angular'") && !originalContent.includes('from "igniteui-angular"') && | |
| !originalContent.includes("from '@infragistics/igniteui-angular'") && !originalContent.includes('from "@infragistics/igniteui-angular"')) { | |
| if (!/from\s+['"](@infragistics\/)?igniteui-angular['"]/.test(originalContent)) { |
Closes #
Additional information (check all that apply):
Checklist:
feature/README.MDupdates for the feature docsREADME.MDCHANGELOG.MDupdates for newly added functionalityng updatemigrations for the breaking changes (migrations guidelines)