Skip to content

fix(no-wildcard-imports): mix-and-match import types #249

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Oct 9, 2024
Merged

fix(no-wildcard-imports): mix-and-match import types #249

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0fcc217
fix(no-wildcard-imports): mix-and-match import types
joshblack Oct 8, 2024
f815572
chore: add changeset
joshblack Oct 8, 2024
8a432e8
refactor: update logic to reuse imports
joshblack Oct 8, 2024
45382c5
fix: add support for default
joshblack Oct 8, 2024
ec254e8
fix: reuse existing type import declarations if they exist
joshblack Oct 9, 2024
179c605
chore: fix eslint warnings
joshblack Oct 9, 2024
197bca9
fix: lookup for specifiers
joshblack Oct 9, 2024
61f66c5
fix: update overlap when removing and replacing node
joshblack Oct 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/grumpy-masks-lie.md
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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