From 0fcc217d3dbd0df385fcbdeb63a6146f7a52106c Mon Sep 17 00:00:00 2001 From: Josh Black Date: Tue, 8 Oct 2024 14:28:33 -0500 Subject: [PATCH 1/8] fix(no-wildcard-imports): mix-and-match import types --- .../__tests__/no-wildcard-imports.test.js | 22 +++++- src/rules/no-wildcard-imports.js | 71 +++++-------------- 2 files changed, 38 insertions(+), 55 deletions(-) diff --git a/src/rules/__tests__/no-wildcard-imports.test.js b/src/rules/__tests__/no-wildcard-imports.test.js index db11f077..58c75440 100644 --- a/src/rules/__tests__/no-wildcard-imports.test.js +++ b/src/rules/__tests__/no-wildcard-imports.test.js @@ -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', @@ -134,6 +133,21 @@ 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 ------------------------------------------------------------ @@ -414,5 +428,7 @@ import type {ButtonBaseProps} from '@primer/react'`, }, ], }, - ], + ].filter((item, index) => { + return index === 7 + }), }) diff --git a/src/rules/no-wildcard-imports.js b/src/rules/no-wildcard-imports.js index 82e3c164..d4a6d199 100644 --- a/src/rules/no-wildcard-imports.js +++ b/src/rules/no-wildcard-imports.js @@ -340,64 +340,31 @@ module.exports = { }, *fix(fixer) { for (const [entrypoint, importSpecifiers] of changes) { - const typeSpecifiers = importSpecifiers.filter(([, , type]) => { - return type === 'type' + const namedSpecifiers = importSpecifiers.filter(([imported]) => { + return imported !== 'default' }) - - // 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' - }) - - if (namedSpecifiers.length > 0 && !defaultSpecifier) { - const specifiers = namedSpecifiers.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}'`) - } - - return - } - - // Otherwise, we have a mix of type and value imports to emit - const valueSpecifiers = importSpecifiers.filter(([, , type]) => { - return type !== 'type' + const defaultSpecifier = importSpecifiers.find(([imported]) => { + return imported === 'default' }) - - if (valueSpecifiers.length === 0) { - return - } - - const specifiers = valueSpecifiers.map(([imported, local]) => { + const specifiers = namedSpecifiers.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.replaceText(node, `import {${specifiers.join(', ')}} from '${entrypoint}'`) - if (typeSpecifiers.length > 0) { - const specifiers = typeSpecifiers.map(([imported, local]) => { - if (imported !== local) { - return `${imported} as ${local}` - } - return imported - }) - yield fixer.insertTextAfter(node, `\nimport type {${specifiers.join(', ')}} from '${entrypoint}'`) + if (namedSpecifiers.length > 0 && !defaultSpecifier) { + yield fixer.replaceText(node, `import {${specifiers.join(', ')}} from '${entrypoint}'`) + } else if (namedSpecifiers.length > 0 && defaultSpecifier) { + const prefix = defaultSpecifier[2].type === 'type' ? 'type ' : '' + yield fixer.replaceText( + node, + `import ${prefix}${defaultSpecifier[1]}, {${specifiers.join(', ')}} from '${entrypoint}'`, + ) + } else if (defaultSpecifier && namedSpecifiers.length === 0) { + const prefix = defaultSpecifier[2].type === 'type' ? 'type ' : '' + yield fixer.replaceText(node, `import ${prefix}${defaultSpecifier[1]} from '${entrypoint}'`) } } }, From f8155728de11dfc3f8736f6801077680dd223900 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Tue, 8 Oct 2024 14:29:07 -0500 Subject: [PATCH 2/8] chore: add changeset --- .changeset/grumpy-masks-lie.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/grumpy-masks-lie.md diff --git a/.changeset/grumpy-masks-lie.md b/.changeset/grumpy-masks-lie.md new file mode 100644 index 00000000..0d181e40 --- /dev/null +++ b/.changeset/grumpy-masks-lie.md @@ -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 From 8a432e813dc06b50026ef5233d20b96cf2470911 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Tue, 8 Oct 2024 15:00:55 -0500 Subject: [PATCH 3/8] refactor: update logic to reuse imports --- .../__tests__/no-wildcard-imports.test.js | 37 ++++---- src/rules/no-wildcard-imports.js | 86 ++++++++++++++----- 2 files changed, 84 insertions(+), 39 deletions(-) diff --git a/src/rules/__tests__/no-wildcard-imports.test.js b/src/rules/__tests__/no-wildcard-imports.test.js index 58c75440..a6de13a3 100644 --- a/src/rules/__tests__/no-wildcard-imports.test.js +++ b/src/rules/__tests__/no-wildcard-imports.test.js @@ -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', @@ -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', @@ -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', @@ -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', @@ -137,7 +137,8 @@ ruleTester.run('no-wildcard-imports', rule, { { 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'`, + output: `import {Box, type BoxProps, type BetterSystemStyleObject} from '@primer/react' +`, errors: [ { messageId: 'wildcardMigration', @@ -169,7 +170,7 @@ import type {BetterSystemStyleObject} from '@primer/react/lib-esm/sx'`, 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', @@ -187,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', @@ -223,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', @@ -235,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', @@ -259,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', @@ -271,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', @@ -283,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', @@ -295,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', @@ -319,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', @@ -389,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', @@ -405,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', @@ -428,7 +429,5 @@ import type {ButtonBaseProps} from '@primer/react'`, }, ], }, - ].filter((item, index) => { - return index === 7 - }), + ], }) diff --git a/src/rules/no-wildcard-imports.js b/src/rules/no-wildcard-imports.js index d4a6d199..0c578230 100644 --- a/src/rules/no-wildcard-imports.js +++ b/src/rules/no-wildcard-imports.js @@ -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 } @@ -340,31 +354,63 @@ module.exports = { }, *fix(fixer) { for (const [entrypoint, importSpecifiers] of changes) { - const namedSpecifiers = importSpecifiers.filter(([imported]) => { - return imported !== 'default' + const importDeclaration = node.parent.body.find(node => { + return node.type === 'ImportDeclaration' && node.source.value === entrypoint }) - const defaultSpecifier = importSpecifiers.find(([imported]) => { + const namedSpecifiers = importSpecifiers + .filter(([imported]) => { + return imported !== 'default' + }) + .map(([imported, local, type]) => { + const prefix = type === 'type' ? 'type ' : '' + if (imported !== local) { + return `${prefix}${imported} as ${local}` + } + return `${prefix}${imported}` + }) + let defaultSpecifier = importSpecifiers.find(([imported]) => { return imported === 'default' }) - const specifiers = namedSpecifiers.map(([imported, local, type]) => { - const prefix = type === 'type' ? 'type ' : '' - if (imported !== local) { - return `${prefix}${imported} as ${local}` + if (defaultSpecifier) { + const prefix = defaultSpecifier[2] === 'type' ? 'type ' : '' + defaultSpecifier = `${prefix}${defaultSpecifier[1]}` + } + + const hasNamedSpecifiers = namedSpecifiers.length > 0 + const hasDefaultSpecifier = !!defaultSpecifier + + if (importDeclaration) { + yield fixer.remove(node) + + if (importDeclaration.specifiers.length === 0) { + throw new Error('Import Declaration has no specifiers') + } + + let lastSpecifier = importDeclaration.specifiers[importDeclaration.specifiers.length - 1] + + if (hasDefaultSpecifier) { + console.log('wat') + } + + if (hasNamedSpecifiers) { + yield fixer.insertTextAfter(lastSpecifier, `, ${namedSpecifiers.join(', ')}`) + } + } else { + let declaration = 'import ' + + if (hasDefaultSpecifier) { + declaration += defaultSpecifier + } + + if (hasNamedSpecifiers) { + if (hasDefaultSpecifier) { + declaration += ', ' + } + declaration += `{${namedSpecifiers.join(', ')}}` } - return `${prefix}${imported}` - }) - if (namedSpecifiers.length > 0 && !defaultSpecifier) { - yield fixer.replaceText(node, `import {${specifiers.join(', ')}} from '${entrypoint}'`) - } else if (namedSpecifiers.length > 0 && defaultSpecifier) { - const prefix = defaultSpecifier[2].type === 'type' ? 'type ' : '' - yield fixer.replaceText( - node, - `import ${prefix}${defaultSpecifier[1]}, {${specifiers.join(', ')}} from '${entrypoint}'`, - ) - } else if (defaultSpecifier && namedSpecifiers.length === 0) { - const prefix = defaultSpecifier[2].type === 'type' ? 'type ' : '' - yield fixer.replaceText(node, `import ${prefix}${defaultSpecifier[1]} from '${entrypoint}'`) + declaration += ` from '${entrypoint}'` + yield fixer.replaceText(node, declaration) } } }, From 45382c574b9c37fe35132690fc248dc26012c9cb Mon Sep 17 00:00:00 2001 From: Josh Black Date: Tue, 8 Oct 2024 15:11:55 -0500 Subject: [PATCH 4/8] fix: add support for default --- src/rules/no-wildcard-imports.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/rules/no-wildcard-imports.js b/src/rules/no-wildcard-imports.js index 0c578230..81bf3fc8 100644 --- a/src/rules/no-wildcard-imports.js +++ b/src/rules/no-wildcard-imports.js @@ -386,10 +386,11 @@ module.exports = { throw new Error('Import Declaration has no specifiers') } - let lastSpecifier = importDeclaration.specifiers[importDeclaration.specifiers.length - 1] + const firstSpecifier = importDeclaration.specifiers[0] + const lastSpecifier = importDeclaration.specifiers[importDeclaration.specifiers.length - 1] if (hasDefaultSpecifier) { - console.log('wat') + yield fixer.insertTextBefore(firstSpecifier, `${defaultSpecifier}, `) } if (hasNamedSpecifiers) { From ec254e8e2cfe0579d143a5b74e327e8586570d1d Mon Sep 17 00:00:00 2001 From: Josh Black Date: Wed, 9 Oct 2024 11:27:53 -0500 Subject: [PATCH 5/8] fix: reuse existing type import declarations if they exist --- src/rules/no-wildcard-imports.js | 96 +++++++++++++++++++++++++------- 1 file changed, 75 insertions(+), 21 deletions(-) diff --git a/src/rules/no-wildcard-imports.js b/src/rules/no-wildcard-imports.js index 81bf3fc8..ac0d210a 100644 --- a/src/rules/no-wildcard-imports.js +++ b/src/rules/no-wildcard-imports.js @@ -355,11 +355,29 @@ module.exports = { *fix(fixer) { for (const [entrypoint, importSpecifiers] of changes) { const importDeclaration = node.parent.body.find(node => { - return node.type === 'ImportDeclaration' && node.source.value === entrypoint + 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' + ) }) const namedSpecifiers = importSpecifiers - .filter(([imported]) => { - return imported !== 'default' + .filter(([imported, _local, type]) => { + return imported !== 'default' && type !== 'type' + }) + .map(([imported, local, type]) => { + const prefix = type === 'type' ? 'type ' : '' + if (imported !== local) { + return `${prefix}${imported} as ${local}` + } + return `${prefix}${imported}` + }) + const namedTypeSpecifiers = importSpecifiers + .filter(([imported, _local, type]) => { + return imported !== 'default' && type === 'type' }) .map(([imported, local, type]) => { const prefix = type === 'type' ? 'type ' : '' @@ -368,46 +386,82 @@ module.exports = { } return `${prefix}${imported}` }) - let defaultSpecifier = importSpecifiers.find(([imported]) => { - return imported === 'default' + let defaultSpecifier = importSpecifiers.find(([imported, _local, type]) => { + return imported === 'default' && type !== 'type' }) if (defaultSpecifier) { - const prefix = defaultSpecifier[2] === 'type' ? 'type ' : '' - defaultSpecifier = `${prefix}${defaultSpecifier[1]}` + defaultSpecifier = defaultSpecifier[1] + } + let defaultTypeSpecifier = importSpecifiers.find(([imported, _local, type]) => { + return imported === 'default' && type === 'type' + }) + if (defaultTypeSpecifier) { + defaultTypeSpecifier = `type ${defaultTypeSpecifier[1]}` } - const hasNamedSpecifiers = namedSpecifiers.length > 0 - const hasDefaultSpecifier = !!defaultSpecifier - - if (importDeclaration) { + if (typeImportDeclaration || importDeclaration) { yield fixer.remove(node) + } + + // Reuse a type import if it exists + if (typeImportDeclaration) { + const firstSpecifier = typeImportDeclaration.specifiers[0] + const lastSpecifier = typeImportDeclaration.specifiers[importDeclaration.specifiers.length - 1] - if (importDeclaration.specifiers.length === 0) { - throw new Error('Import Declaration has no specifiers') + if (defaultTypeSpecifier) { + const postfix = + namedTypeSpecifiers.length > 0 || typeImportDeclaration.specifiers.length > 0 ? ', ' : ' ' + yield fix.insertTextBeforeRange( + [firstSpecifier.range[0] - 1, firstSpecifier.range[1]], + `${defaultTypeSpecifier}${postfix}`, + ) } + if (namedTypeSpecifiers.length > 0) { + yield fixer.insertTextAfter(lastSpecifier, `, ${namedTypeSpecifiers.join(', ')}`) + } + } + + // Reuse an import declaration if one exists + if (importDeclaration) { const firstSpecifier = importDeclaration.specifiers[0] const lastSpecifier = importDeclaration.specifiers[importDeclaration.specifiers.length - 1] - if (hasDefaultSpecifier) { - yield fixer.insertTextBefore(firstSpecifier, `${defaultSpecifier}, `) + if (defaultSpecifier) { + const postfix = namedSpecifiers.length > 0 || importDeclaration.specifiers.length > 0 ? ', ' : ' ' + yield fix.insertTextBeforeRange( + [firstSpecifier.range[0] - 1, firstSpecifier.range[1]], + `${defaultSpecifier}${postfix}`, + ) } - if (hasNamedSpecifiers) { - yield fixer.insertTextAfter(lastSpecifier, `, ${namedSpecifiers.join(', ')}`) + if (namedSpecifiers.length > 0 || (!typeImportDeclaration && namedTypeSpecifiers.length > 0)) { + const specifiers = [...namedSpecifiers] + if (!typeImportDeclaration) { + specifiers.push(...namedTypeSpecifiers) + } + yield fixer.insertTextAfter(lastSpecifier, `, ${specifiers.join(', ')}`) } } else { + const specifiers = [...namedSpecifiers] + if (!typeImportDeclaration) { + specifiers.push(...namedTypeSpecifiers) + } let declaration = 'import ' - if (hasDefaultSpecifier) { + if (defaultSpecifier) { declaration += defaultSpecifier } - if (hasNamedSpecifiers) { - if (hasDefaultSpecifier) { + if (defaultTypeSpecifier && !typeImportDeclaration) { + declaration += defaultTypeSpecifier + } + + if (specifiers.length > 0) { + if (defaultSpecifier || defaultTypeSpecifier) { declaration += ', ' } - declaration += `{${namedSpecifiers.join(', ')}}` + declaration += `{${specifiers.join(', ')}}` } declaration += ` from '${entrypoint}'` From 179c605d0cb3c5f758c48fb91440eebf2fe7f47c Mon Sep 17 00:00:00 2001 From: Josh Black Date: Wed, 9 Oct 2024 11:30:14 -0500 Subject: [PATCH 6/8] chore: fix eslint warnings --- src/rules/no-wildcard-imports.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/rules/no-wildcard-imports.js b/src/rules/no-wildcard-imports.js index ac0d210a..08ab2ccc 100644 --- a/src/rules/no-wildcard-imports.js +++ b/src/rules/no-wildcard-imports.js @@ -365,7 +365,7 @@ module.exports = { ) }) const namedSpecifiers = importSpecifiers - .filter(([imported, _local, type]) => { + .filter(([imported, , type]) => { return imported !== 'default' && type !== 'type' }) .map(([imported, local, type]) => { @@ -376,7 +376,7 @@ module.exports = { return `${prefix}${imported}` }) const namedTypeSpecifiers = importSpecifiers - .filter(([imported, _local, type]) => { + .filter(([imported, , type]) => { return imported !== 'default' && type === 'type' }) .map(([imported, local, type]) => { @@ -386,13 +386,13 @@ module.exports = { } return `${prefix}${imported}` }) - let defaultSpecifier = importSpecifiers.find(([imported, _local, type]) => { + let defaultSpecifier = importSpecifiers.find(([imported, , type]) => { return imported === 'default' && type !== 'type' }) if (defaultSpecifier) { defaultSpecifier = defaultSpecifier[1] } - let defaultTypeSpecifier = importSpecifiers.find(([imported, _local, type]) => { + let defaultTypeSpecifier = importSpecifiers.find(([imported, , type]) => { return imported === 'default' && type === 'type' }) if (defaultTypeSpecifier) { @@ -411,7 +411,7 @@ module.exports = { if (defaultTypeSpecifier) { const postfix = namedTypeSpecifiers.length > 0 || typeImportDeclaration.specifiers.length > 0 ? ', ' : ' ' - yield fix.insertTextBeforeRange( + yield fixer.insertTextBeforeRange( [firstSpecifier.range[0] - 1, firstSpecifier.range[1]], `${defaultTypeSpecifier}${postfix}`, ) @@ -429,7 +429,7 @@ module.exports = { if (defaultSpecifier) { const postfix = namedSpecifiers.length > 0 || importDeclaration.specifiers.length > 0 ? ', ' : ' ' - yield fix.insertTextBeforeRange( + yield fixer.insertTextBeforeRange( [firstSpecifier.range[0] - 1, firstSpecifier.range[1]], `${defaultSpecifier}${postfix}`, ) From 197bca91173b46ec505cabf198713f846e9117c8 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Wed, 9 Oct 2024 11:48:57 -0500 Subject: [PATCH 7/8] fix: lookup for specifiers --- src/rules/no-wildcard-imports.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/no-wildcard-imports.js b/src/rules/no-wildcard-imports.js index 08ab2ccc..d171159f 100644 --- a/src/rules/no-wildcard-imports.js +++ b/src/rules/no-wildcard-imports.js @@ -406,7 +406,7 @@ module.exports = { // Reuse a type import if it exists if (typeImportDeclaration) { const firstSpecifier = typeImportDeclaration.specifiers[0] - const lastSpecifier = typeImportDeclaration.specifiers[importDeclaration.specifiers.length - 1] + const lastSpecifier = typeImportDeclaration.specifiers[typeImportDeclaration.specifiers.length - 1] if (defaultTypeSpecifier) { const postfix = From 61f66c54cb31764c423cd76ef5641688748a0582 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Wed, 9 Oct 2024 14:01:08 -0500 Subject: [PATCH 8/8] fix: update overlap when removing and replacing node --- src/rules/no-wildcard-imports.js | 68 +++++++++++++++++--------------- 1 file changed, 37 insertions(+), 31 deletions(-) diff --git a/src/rules/no-wildcard-imports.js b/src/rules/no-wildcard-imports.js index d171159f..36425594 100644 --- a/src/rules/no-wildcard-imports.js +++ b/src/rules/no-wildcard-imports.js @@ -364,28 +364,13 @@ module.exports = { node.type === 'ImportDeclaration' && node.source.value === entrypoint && node.importKind === 'type' ) }) - const namedSpecifiers = importSpecifiers - .filter(([imported, , type]) => { - return imported !== 'default' && type !== 'type' - }) - .map(([imported, local, type]) => { - const prefix = type === 'type' ? 'type ' : '' - if (imported !== local) { - return `${prefix}${imported} as ${local}` - } - return `${prefix}${imported}` - }) - const namedTypeSpecifiers = importSpecifiers - .filter(([imported, , type]) => { - return imported !== 'default' && type === 'type' - }) - .map(([imported, local, type]) => { - const prefix = type === 'type' ? 'type ' : '' - if (imported !== local) { - return `${prefix}${imported} as ${local}` - } - return `${prefix}${imported}` - }) + 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' }) @@ -399,10 +384,6 @@ module.exports = { defaultTypeSpecifier = `type ${defaultTypeSpecifier[1]}` } - if (typeImportDeclaration || importDeclaration) { - yield fixer.remove(node) - } - // Reuse a type import if it exists if (typeImportDeclaration) { const firstSpecifier = typeImportDeclaration.specifiers[0] @@ -412,13 +393,19 @@ module.exports = { const postfix = namedTypeSpecifiers.length > 0 || typeImportDeclaration.specifiers.length > 0 ? ', ' : ' ' yield fixer.insertTextBeforeRange( - [firstSpecifier.range[0] - 1, firstSpecifier.range[1]], + [firstSpecifier.range[0] - 2, firstSpecifier.range[1]], `${defaultTypeSpecifier}${postfix}`, ) } if (namedTypeSpecifiers.length > 0) { - yield fixer.insertTextAfter(lastSpecifier, `, ${namedTypeSpecifiers.join(', ')}`) + const specifiers = namedTypeSpecifiers.map(([imported, local]) => { + if (imported !== local) { + return `${imported} as ${local}` + } + return imported + }) + yield fixer.insertTextAfter(lastSpecifier, `, ${specifiers.join(', ')}`) } } @@ -430,23 +417,37 @@ module.exports = { if (defaultSpecifier) { const postfix = namedSpecifiers.length > 0 || importDeclaration.specifiers.length > 0 ? ', ' : ' ' yield fixer.insertTextBeforeRange( - [firstSpecifier.range[0] - 1, firstSpecifier.range[1]], + [firstSpecifier.range[0] - 2, firstSpecifier.range[1]], `${defaultSpecifier}${postfix}`, ) } if (namedSpecifiers.length > 0 || (!typeImportDeclaration && namedTypeSpecifiers.length > 0)) { - const specifiers = [...namedSpecifiers] + 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 { - const specifiers = [...namedSpecifiers] + 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}` + }) let declaration = 'import ' if (defaultSpecifier) { @@ -466,6 +467,11 @@ module.exports = { declaration += ` from '${entrypoint}'` yield fixer.replaceText(node, declaration) + originalImportReplaced = true + } + + if (!originalImportReplaced) { + yield fixer.remove(node) } } },