-
Notifications
You must be signed in to change notification settings - Fork 109
refactor(ui-codemods): add ui metapackage to icon codemod and simplif… #2472
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| import React from 'react' | ||
| import { | ||
| Button, | ||
| IconSearchLine, | ||
| IconAddLine, | ||
| IconInfoLine, | ||
| Text | ||
| } from '@instructure/ui' | ||
|
|
||
| function MyComponent() { | ||
| return ( | ||
| <div> | ||
| <Button renderIcon={<IconSearchLine />}>Click me</Button> | ||
| <Button renderIcon={IconInfoLine}>Info</Button> | ||
| <IconAddLine /> | ||
| <Text>Hello</Text> | ||
| </div> | ||
| ) | ||
| } | ||
|
|
||
| export default MyComponent |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| import React from 'react' | ||
| import { | ||
| Button, | ||
| SearchInstUIIcon, | ||
| PlusInstUIIcon, | ||
| InfoInstUIIcon, | ||
| Text | ||
| } from '@instructure/ui' | ||
|
|
||
| function MyComponent() { | ||
| return ( | ||
| <div> | ||
| <Button renderIcon={<SearchInstUIIcon />}>Click me</Button> | ||
| <Button renderIcon={InfoInstUIIcon}>Info</Button> | ||
| <PlusInstUIIcon /> | ||
| <Text>Hello</Text> | ||
| </div> | ||
| ) | ||
| } | ||
|
|
||
| export default MyComponent |
This file was deleted.
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,3 @@ | ||
| [ | ||
| "No mapping found for icon: IconFakeIconThatDoesNotExistLine. Please migrate manually." | ||
| "No mapping found for \"IconFakeIconThatDoesNotExistLine\". Please migrate manually." | ||
| ] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,144 +22,108 @@ | |
| * SOFTWARE. | ||
| */ | ||
|
|
||
| import type { Transform } from 'jscodeshift' | ||
| import type { Collection, JSCodeshift, Transform } from 'jscodeshift' | ||
| import type { InstUICodemod } from './utils/instUICodemodExecutor' | ||
| import instUICodemodExecutor from './utils/instUICodemodExecutor' | ||
| import { printWarning } from './utils/codemodHelpers' | ||
| import { isImportSpecifier } from './utils/codemodTypeCheckers' | ||
| import { | ||
| findEveryImport, | ||
| printWarning, | ||
| renameImportAndUsages | ||
| } from './utils/codemodHelpers' | ||
| import fs from 'fs' | ||
| import path from 'path' | ||
| import { fileURLToPath } from 'url' | ||
|
|
||
| const __filename = fileURLToPath(import.meta.url) | ||
| const __dirname = path.dirname(__filename) | ||
|
|
||
| const IMPORT_PATHS = [ | ||
| '@instructure/ui-icons', | ||
| '@instructure/ui-icons/es/svg', | ||
| '@instructure/ui' | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the key part of this PR. |
||
| ] | ||
|
|
||
| const mappingData = JSON.parse( | ||
| fs.readFileSync( | ||
| path.join(__dirname, '../../ui-icons/src/lucide/mapping.json'), | ||
| 'utf-8' | ||
| ) | ||
| ) | ||
| const iconMapping = mappingData.mappingOverrides as Record<string, string> | ||
|
|
||
| const ICON_IMPORT_PATHS = [ | ||
| '@instructure/ui-icons', | ||
| '@instructure/ui-icons/es/svg' | ||
| ] | ||
| const NEW_ICON_IMPORT_PATH = '@instructure/ui-icons' | ||
| const iconMappings = mappingData.mappingOverrides as Record<string, string> | ||
|
|
||
| /** | ||
| * Convert kebab-case to PascalCase. | ||
| * Example: 'accessibility-2' -> 'Accessibility2' | ||
| */ | ||
| function kebabToPascal(str: string): string { | ||
| return str | ||
| .split('-') | ||
| .map((part) => part.charAt(0).toUpperCase() + part.slice(1)) | ||
| .join('') | ||
| } | ||
|
|
||
| const LEGACY_ICON_PATTERN = /^Icon(.+?)(Line|Solid)$/ | ||
|
|
||
| /** | ||
| * Returns the new icon export name for the given legacy icon name, | ||
| * or null if no mapping exists. | ||
| * Example: 'IconA11yLine' -> 'Accessibility2InstUIIcon' | ||
| * Warns about legacy icons that has no mappings and have to be migrated manually | ||
| */ | ||
| function getNewIconName(oldName: string): string | null { | ||
| const match = oldName.match(/^Icon(.+?)(Line|Solid)$/) | ||
| if (!match) return null | ||
| const mappedName = iconMapping[match[1]] | ||
| if (!mappedName) return null | ||
| return kebabToPascal(mappedName) + 'InstUIIcon' | ||
| function warnUnmappedIcons( | ||
| j: JSCodeshift, | ||
| root: Collection, | ||
| filePath: string | ||
| ): void { | ||
| // findEveryImport only accepts a single path, so flatMap collects results across all paths | ||
|
Comment on lines
+67
to
+72
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When can this happen? We have icons which have no mappings?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This may happen, when the mapping file changes so I believe it is worth preparing for this case (we get a new |
||
| IMPORT_PATHS.flatMap((p) => findEveryImport(j, root, p)).forEach( | ||
| (localName) => { | ||
| const match = localName.match(LEGACY_ICON_PATTERN) | ||
| if (!match || iconMappings[match[1]]) return | ||
|
|
||
| printWarning( | ||
| filePath, | ||
| undefined, | ||
| `No mapping found for "${localName}". Please migrate manually.` | ||
| ) | ||
| } | ||
| ) | ||
| } | ||
|
|
||
| /** | ||
| * Migrates from legacy InstUI icons to new lucide and custom icons | ||
| * Iterates every known legacy icon name (derived from mapping.json) and migrates it: | ||
| * 1. Renames the import specifier in-place | ||
| * 2. Renames JSX elements | ||
| * 3. Renames references (`renderIcon={OldIcon}`). | ||
| */ | ||
| const migrateToNewIcons: Transform = ( | ||
| file, | ||
| api, | ||
| options?: { fileName?: string; usePrettier?: boolean } | ||
| ) => { | ||
| return instUICodemodExecutor(migrateToNewIconsCodemod, file, api, options) | ||
| } | ||
| function migrateIcons(j: JSCodeshift, root: Collection): boolean { | ||
| let didChange = false | ||
|
|
||
| const migrateToNewIconsCodemod: InstUICodemod = (j, root, filePath) => { | ||
| let hasModifications = false | ||
| const newIconNames = new Set<string>() | ||
| // localIconName -> newIconName (handles aliased imports like `import { A as B }`) | ||
| const replacements = new Map<string, string>() | ||
|
|
||
| root.find(j.ImportDeclaration).forEach((path) => { | ||
| if (!ICON_IMPORT_PATHS.includes(path.node.source.value as string)) return | ||
| for (const [iconSuffix, newIconKey] of Object.entries(iconMappings)) { | ||
| const newName = kebabToPascal(newIconKey) + 'InstUIIcon' | ||
|
|
||
| const remainingSpecifiers: typeof path.node.specifiers = [] | ||
|
|
||
| path.node.specifiers?.forEach((spec) => { | ||
| if (!isImportSpecifier(spec)) { | ||
| remainingSpecifiers.push(spec) | ||
| return | ||
| } | ||
| for (const variant of ['Line', 'Solid']) { | ||
| const legacyName = `Icon${iconSuffix}${variant}` | ||
|
|
||
| const oldIconName = spec.imported.name as string | ||
| const localIconName = (spec.local?.name as string) ?? oldIconName | ||
| const newIconName = getNewIconName(oldIconName) | ||
|
|
||
| if (!newIconName) { | ||
| remainingSpecifiers.push(spec) | ||
| printWarning( | ||
| filePath, | ||
| spec.loc?.start.line, | ||
| `No mapping found for icon: ${oldIconName}. Please migrate manually.` | ||
| ) | ||
| return | ||
| if (renameImportAndUsages(j, root, legacyName, newName, IMPORT_PATHS)) { | ||
| didChange = true | ||
| } | ||
|
|
||
| replacements.set(localIconName, newIconName) | ||
| newIconNames.add(newIconName) | ||
| hasModifications = true | ||
| }) | ||
|
|
||
| if (remainingSpecifiers.length === 0) { | ||
| j(path).remove() | ||
| } else { | ||
| // eslint-disable-next-line no-param-reassign | ||
| path.node.specifiers = remainingSpecifiers | ||
| } | ||
| }) | ||
|
|
||
| if (!hasModifications) return false | ||
|
|
||
| // Rename JSX usages | ||
| root.find(j.JSXElement).forEach((path) => { | ||
| const opening = path.node.openingElement | ||
| const closing = path.node.closingElement | ||
| if (opening.name.type !== 'JSXIdentifier') return | ||
| } | ||
|
|
||
| const newName = replacements.get(opening.name.name) | ||
| if (!newName) return | ||
| return didChange | ||
| } | ||
|
|
||
| opening.name.name = newName | ||
| if (closing?.name.type === 'JSXIdentifier') { | ||
| closing.name.name = newName | ||
| } | ||
| }) | ||
|
|
||
| // Add new import at the top, sorted alphabetically | ||
| const specifiers = Array.from(newIconNames) | ||
| .sort() | ||
| .map((name) => j.importSpecifier(j.identifier(name), j.identifier(name))) | ||
| const newImport = j.importDeclaration( | ||
| specifiers, | ||
| j.literal(NEW_ICON_IMPORT_PATH) | ||
| ) | ||
| const firstImport = root.find(j.ImportDeclaration).at(0) | ||
| if (firstImport.length > 0) { | ||
| firstImport.insertBefore(newImport) | ||
| } else { | ||
| root.get().node.program.body.unshift(newImport) | ||
| } | ||
| /** | ||
| * Migrates legacy InstUI icons (`IconXxxLine` / `IconXxxSolid`) to new | ||
| * `XxxInstUIIcon` components. | ||
| * | ||
| * Renames specifiers in-place for imports from: | ||
| * - `@instructure/ui-icons` | ||
| * - `@instructure/ui-icons/es/svg` | ||
| * - `@instructure/ui` | ||
| */ | ||
| const migrateToNewIconsCodemod: InstUICodemod = (j, root, filePath) => { | ||
| warnUnmappedIcons(j, root, filePath) | ||
| return migrateIcons(j, root) | ||
| } | ||
|
|
||
| return true | ||
| const migrateToNewIcons: Transform = (file, api, options) => { | ||
| return instUICodemodExecutor(migrateToNewIconsCodemod, file, api, options) | ||
| } | ||
|
|
||
| export default migrateToNewIcons | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I refactored the codemod and used these helper functions.