Skip to content

Commit

Permalink
fix(no-wildcard-imports): mix-and-match import types (#249)
Browse files Browse the repository at this point in the history 
* fix(no-wildcard-imports): mix-and-match import types

* chore: add changeset

* refactor: update logic to reuse imports

* fix: add support for default

* fix: reuse existing type import declarations if they exist

* chore: fix eslint warnings

* fix: lookup for specifiers

* fix: update overlap when removing and replacing node
  • Loading branch information
@joshblack
joshblack authored Oct 9, 2024
1 parent 2c5ea48 commit f8c8a9d
Show file tree
Hide file tree
Showing 3 changed files with 152 additions and 58 deletions.
5 changes: 5 additions & 0 deletions .changeset/grumpy-masks-lie.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'eslint-plugin-primer-react': patch
---

Update no-wildcard-imports rule to not create separate imports for type only imports. This prevents an issue downstream with autofixers
49 changes: 32 additions & 17 deletions src/rules/__tests__/no-wildcard-imports.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ ruleTester.run('no-wildcard-imports', rule, {
// Test type import
{
code: `import type {SxProp} from '@primer/react/lib-esm/sx'`,
output: `import type {SxProp} from '@primer/react'`,
output: `import {type SxProp} from '@primer/react'`,
errors: [
{
messageId: 'wildcardMigration',
Expand All @@ -44,7 +44,7 @@ ruleTester.run('no-wildcard-imports', rule, {
// Test multiple type imports
{
code: `import type {BetterSystemStyleObject, SxProp, BetterCssProperties} from '@primer/react/lib-esm/sx'`,
output: `import type {BetterSystemStyleObject, SxProp, BetterCssProperties} from '@primer/react'`,
output: `import {type BetterSystemStyleObject, type SxProp, type BetterCssProperties} from '@primer/react'`,
errors: [
{
messageId: 'wildcardMigration',
Expand All @@ -58,7 +58,7 @@ ruleTester.run('no-wildcard-imports', rule, {
// Test import alias
{
code: `import type {SxProp as RenamedSxProp} from '@primer/react/lib-esm/sx'`,
output: `import type {SxProp as RenamedSxProp} from '@primer/react'`,
output: `import {type SxProp as RenamedSxProp} from '@primer/react'`,
errors: [
{
messageId: 'wildcardMigration',
Expand Down Expand Up @@ -108,7 +108,7 @@ ruleTester.run('no-wildcard-imports', rule, {
// Test renamed wildcard imports
{
code: `import type {ItemProps} from '@primer/react/lib-esm/deprecated/ActionList/Item'`,
output: `import type {ActionListItemProps as ItemProps} from '@primer/react/deprecated'`,
output: `import {type ActionListItemProps as ItemProps} from '@primer/react/deprecated'`,
errors: [
{
messageId: 'wildcardMigration',
Expand All @@ -122,8 +122,7 @@ ruleTester.run('no-wildcard-imports', rule, {
// Test mixed imports
{
code: `import {Dialog, type DialogProps} from '@primer/react/lib-esm/Dialog/Dialog'`,
output: `import {Dialog} from '@primer/react/experimental'
import type {DialogProps} from '@primer/react/experimental'`,
output: `import {Dialog, type DialogProps} from '@primer/react/experimental'`,
errors: [
{
messageId: 'wildcardMigration',
Expand All @@ -134,6 +133,22 @@ import type {DialogProps} from '@primer/react/experimental'`,
],
},

// Use existing imports
{
code: `import {Box, type BoxProps} from '@primer/react'
import type {BetterSystemStyleObject} from '@primer/react/lib-esm/sx'`,
output: `import {Box, type BoxProps, type BetterSystemStyleObject} from '@primer/react'
`,
errors: [
{
messageId: 'wildcardMigration',
data: {
wildcardEntrypoint: '@primer/react/lib-esm/sx',
},
},
],
},

// Test migrations

// Test helpers ------------------------------------------------------------
Expand All @@ -155,7 +170,7 @@ import type {DialogProps} from '@primer/react/experimental'`,
code: `import {ButtonBase} from '@primer/react/lib-esm/Button/ButtonBase';
import type {ButtonBaseProps} from '@primer/react/lib-esm/Button/ButtonBase'`,
output: `import {ButtonBase} from '@primer/react'
import type {ButtonBaseProps} from '@primer/react'`,
import {type ButtonBaseProps} from '@primer/react'`,
errors: [
{
messageId: 'wildcardMigration',
Expand All @@ -173,7 +188,7 @@ import type {ButtonBaseProps} from '@primer/react'`,
},
{
code: `import type {ButtonBaseProps} from '@primer/react/lib-esm/Button/types'`,
output: `import type {ButtonBaseProps} from '@primer/react'`,
output: `import {type ButtonBaseProps} from '@primer/react'`,
errors: [
{
messageId: 'wildcardMigration',
Expand Down Expand Up @@ -209,7 +224,7 @@ import type {ButtonBaseProps} from '@primer/react'`,
},
{
code: `import type {SelectPanelProps} from '@primer/react/lib-esm/SelectPanel/SelectPanel'`,
output: `import type {SelectPanelProps} from '@primer/react'`,
output: `import {type SelectPanelProps} from '@primer/react'`,
errors: [
{
messageId: 'wildcardMigration',
Expand All @@ -221,7 +236,7 @@ import type {ButtonBaseProps} from '@primer/react'`,
},
{
code: `import type {LabelColorOptions} from '@primer/react/lib-esm/Label/Label'`,
output: `import type {LabelColorOptions} from '@primer/react'`,
output: `import {type LabelColorOptions} from '@primer/react'`,
errors: [
{
messageId: 'wildcardMigration',
Expand All @@ -245,7 +260,7 @@ import type {ButtonBaseProps} from '@primer/react'`,
},
{
code: `import type {IssueLabelTokenProps} from '@primer/react/lib-esm/Token/IssueLabelToken'`,
output: `import type {IssueLabelTokenProps} from '@primer/react'`,
output: `import {type IssueLabelTokenProps} from '@primer/react'`,
errors: [
{
messageId: 'wildcardMigration',
Expand All @@ -257,7 +272,7 @@ import type {ButtonBaseProps} from '@primer/react'`,
},
{
code: `import type {TokenSizeKeys} from '@primer/react/lib-esm/Token/TokenBase'`,
output: `import type {TokenSizeKeys} from '@primer/react'`,
output: `import {type TokenSizeKeys} from '@primer/react'`,
errors: [
{
messageId: 'wildcardMigration',
Expand All @@ -269,7 +284,7 @@ import type {ButtonBaseProps} from '@primer/react'`,
},
{
code: `import type {ItemProps} from '@primer/react/lib-esm/deprecated/ActionList'`,
output: `import type {ActionListItemProps as ItemProps} from '@primer/react/deprecated'`,
output: `import {type ActionListItemProps as ItemProps} from '@primer/react/deprecated'`,
errors: [
{
messageId: 'wildcardMigration',
Expand All @@ -281,7 +296,7 @@ import type {ButtonBaseProps} from '@primer/react'`,
},
{
code: `import type {GroupedListProps} from '@primer/react/lib-esm/deprecated/ActionList/List'`,
output: `import type {ActionListGroupedListProps as GroupedListProps} from '@primer/react/deprecated'`,
output: `import {type ActionListGroupedListProps as GroupedListProps} from '@primer/react/deprecated'`,
errors: [
{
messageId: 'wildcardMigration',
Expand All @@ -305,7 +320,7 @@ import type {ButtonBaseProps} from '@primer/react'`,
},
{
code: `import type {ItemProps} from '@primer/react/lib-esm/deprecated/ActionList/Item'`,
output: `import type {ActionListItemProps as ItemProps} from '@primer/react/deprecated'`,
output: `import {type ActionListItemProps as ItemProps} from '@primer/react/deprecated'`,
errors: [
{
messageId: 'wildcardMigration',
Expand Down Expand Up @@ -375,7 +390,7 @@ import type {ButtonBaseProps} from '@primer/react'`,
},
{
code: `import type {ResponsiveValue} from '@primer/react/lib-esm/hooks/useResponsiveValue'`,
output: `import type {ResponsiveValue} from '@primer/react'`,
output: `import {type ResponsiveValue} from '@primer/react'`,
errors: [
{
messageId: 'wildcardMigration',
Expand All @@ -391,7 +406,7 @@ import type {ButtonBaseProps} from '@primer/react'`,
// @primer/react/lib-esm/sx
{
code: `import type {BetterSystemStyleObject, SxProp, BetterCssProperties} from '@primer/react/lib-esm/sx'`,
output: `import type {BetterSystemStyleObject, SxProp, BetterCssProperties} from '@primer/react'`,
output: `import {type BetterSystemStyleObject, type SxProp, type BetterCssProperties} from '@primer/react'`,
errors: [
{
messageId: 'wildcardMigration',
Expand Down
156 changes: 115 additions & 41 deletions src/rules/no-wildcard-imports.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,20 @@ module.exports = {
create(context) {
return {
ImportDeclaration(node) {
if (node.source.value === '@primer/react/lib-esm/utils/test-helpers') {
context.report({
node,
messageId: 'wildcardMigration',
data: {
wildcardEntrypoint: node.source.value,
},
fix(fixer) {
return fixer.replaceText(node.source, `'@primer/react/test-helpers'`)
},
})
return
}

if (!node.source.value.startsWith('@primer/react/lib-esm')) {
return
}
Expand Down Expand Up @@ -340,64 +354,124 @@ module.exports = {
},
*fix(fixer) {
for (const [entrypoint, importSpecifiers] of changes) {
const typeSpecifiers = importSpecifiers.filter(([, , type]) => {
return type === 'type'
const importDeclaration = node.parent.body.find(node => {
return (
node.type === 'ImportDeclaration' && node.source.value === entrypoint && node.importKind !== 'type'
)
})
const typeImportDeclaration = node.parent.body.find(node => {
return (
node.type === 'ImportDeclaration' && node.source.value === entrypoint && node.importKind === 'type'
)
})
let originalImportReplaced = false
const namedSpecifiers = importSpecifiers.filter(([imported, , type]) => {
return imported !== 'default' && type !== 'type'
})
const namedTypeSpecifiers = importSpecifiers.filter(([imported, , type]) => {
return imported !== 'default' && type === 'type'
})
let defaultSpecifier = importSpecifiers.find(([imported, , type]) => {
return imported === 'default' && type !== 'type'
})
if (defaultSpecifier) {
defaultSpecifier = defaultSpecifier[1]
}
let defaultTypeSpecifier = importSpecifiers.find(([imported, , type]) => {
return imported === 'default' && type === 'type'
})
if (defaultTypeSpecifier) {
defaultTypeSpecifier = `type ${defaultTypeSpecifier[1]}`
}

// If all imports are type imports, emit emit as `import type {specifier} from '...'`
if (typeSpecifiers.length === importSpecifiers.length) {
const namedSpecifiers = typeSpecifiers.filter(([imported]) => {
return imported !== 'default'
})
const defaultSpecifier = typeSpecifiers.find(([imported]) => {
return imported === 'default'
})
// Reuse a type import if it exists
if (typeImportDeclaration) {
const firstSpecifier = typeImportDeclaration.specifiers[0]
const lastSpecifier = typeImportDeclaration.specifiers[typeImportDeclaration.specifiers.length - 1]

if (namedSpecifiers.length > 0 && !defaultSpecifier) {
const specifiers = namedSpecifiers.map(([imported, local]) => {
if (defaultTypeSpecifier) {
const postfix =
namedTypeSpecifiers.length > 0 || typeImportDeclaration.specifiers.length > 0 ? ', ' : ' '
yield fixer.insertTextBeforeRange(
[firstSpecifier.range[0] - 2, firstSpecifier.range[1]],
`${defaultTypeSpecifier}${postfix}`,
)
}

if (namedTypeSpecifiers.length > 0) {
const specifiers = namedTypeSpecifiers.map(([imported, local]) => {
if (imported !== local) {
return `${imported} as ${local}`
}
return imported
})
yield fixer.replaceText(node, `import type {${specifiers.join(', ')}} from '${entrypoint}'`)
} else if (namedSpecifiers.length > 0 && defaultSpecifier) {
yield fixer.replaceText(
node,
`import type ${defaultSpecifier[1]}, ${specifiers.join(', ')} from '${entrypoint}'`,
)
} else if (defaultSpecifier && namedSpecifiers.length === 0) {
yield fixer.replaceText(node, `import type ${defaultSpecifier[1]} from '${entrypoint}'`)
yield fixer.insertTextAfter(lastSpecifier, `, ${specifiers.join(', ')}`)
}

return
}

// Otherwise, we have a mix of type and value imports to emit
const valueSpecifiers = importSpecifiers.filter(([, , type]) => {
return type !== 'type'
})

if (valueSpecifiers.length === 0) {
return
}
// Reuse an import declaration if one exists
if (importDeclaration) {
const firstSpecifier = importDeclaration.specifiers[0]
const lastSpecifier = importDeclaration.specifiers[importDeclaration.specifiers.length - 1]

const specifiers = valueSpecifiers.map(([imported, local]) => {
if (imported !== local) {
return `${imported} as ${local}`
if (defaultSpecifier) {
const postfix = namedSpecifiers.length > 0 || importDeclaration.specifiers.length > 0 ? ', ' : ' '
yield fixer.insertTextBeforeRange(
[firstSpecifier.range[0] - 2, firstSpecifier.range[1]],
`${defaultSpecifier}${postfix}`,
)
}
return imported
})
yield fixer.replaceText(node, `import {${specifiers.join(', ')}} from '${entrypoint}'`)

if (typeSpecifiers.length > 0) {
const specifiers = typeSpecifiers.map(([imported, local]) => {
if (namedSpecifiers.length > 0 || (!typeImportDeclaration && namedTypeSpecifiers.length > 0)) {
let specifiers = [...namedSpecifiers]
if (!typeImportDeclaration) {
specifiers.push(...namedTypeSpecifiers)
}
specifiers = specifiers.map(([imported, local, type]) => {
const prefix = type === 'type' ? 'type ' : ''
if (imported !== local) {
return `${prefix}${imported} as ${local}`
}
return `${prefix}${imported}`
})
yield fixer.insertTextAfter(lastSpecifier, `, ${specifiers.join(', ')}`)
}
} else {
let specifiers = [...namedSpecifiers]
if (!typeImportDeclaration) {
specifiers.push(...namedTypeSpecifiers)
}
specifiers = specifiers.map(([imported, local, type]) => {
const prefix = type === 'type' ? 'type ' : ''
if (imported !== local) {
return `${imported} as ${local}`
return `${prefix}${imported} as ${local}`
}
return imported
return `${prefix}${imported}`
})
yield fixer.insertTextAfter(node, `\nimport type {${specifiers.join(', ')}} from '${entrypoint}'`)
let declaration = 'import '

if (defaultSpecifier) {
declaration += defaultSpecifier
}

if (defaultTypeSpecifier && !typeImportDeclaration) {
declaration += defaultTypeSpecifier
}

if (specifiers.length > 0) {
if (defaultSpecifier || defaultTypeSpecifier) {
declaration += ', '
}
declaration += `{${specifiers.join(', ')}}`
}

declaration += ` from '${entrypoint}'`
yield fixer.replaceText(node, declaration)
originalImportReplaced = true
}

if (!originalImportReplaced) {
yield fixer.remove(node)
}
}
},
Expand Down

0 comments on commit f8c8a9d

Please sign in to comment.