Skip to content

Commit

Permalink
[Fix] no-duplicates: Removing duplicates breaks in TypeScript
Browse files Browse the repository at this point in the history
Fixes #3016. Fixes #2792.
  • Loading branch information
yesl-kim authored and ljharb committed Aug 11, 2024
1 parent 98a0991 commit 4bdf61a
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 2 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
### Fixed
- [`no-extraneous-dependencies`]: allow wrong path ([#3012], thanks [@chabb])
- [`no-cycle`]: use scc algorithm to optimize ([#2998], thanks [@soryy708])
- [`no-duplicates`]: Removing duplicates breaks in TypeScript ([#3033], thanks [@yesl-kim])

### Changed
- [Docs] `no-extraneous-dependencies`: Make glob pattern description more explicit ([#2944], thanks [@mulztob])
Expand Down Expand Up @@ -1121,6 +1122,7 @@ for info on changes for earlier releases.

[`memo-parser`]: ./memo-parser/README.md

[#3033]: https://github.com/import-js/eslint-plugin-import/pull/3033
[#3012]: https://github.com/import-js/eslint-plugin-import/pull/3012
[#3011]: https://github.com/import-js/eslint-plugin-import/pull/3011
[#3004]: https://github.com/import-js/eslint-plugin-import/pull/3004
Expand Down Expand Up @@ -1962,6 +1964,7 @@ for info on changes for earlier releases.
[@wtgtybhertgeghgtwtg]: https://github.com/wtgtybhertgeghgtwtg
[@xM8WVqaG]: https://github.com/xM8WVqaG
[@xpl]: https://github.com/xpl
[@yesl-kim]: https://github.com/yesl-kim
[@yndajas]: https://github.com/yndajas
[@yordis]: https://github.com/yordis
[@Zamiell]: https://github.com/Zamiell
Expand Down
16 changes: 14 additions & 2 deletions src/rules/no-duplicates.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ function getFix(first, rest, sourceCode, context) {
const shouldAddDefault = getDefaultImportName(first) == null && defaultImportNames.size === 1;
const shouldAddSpecifiers = specifiers.length > 0;
const shouldRemoveUnnecessary = unnecessaryImports.length > 0;
const preferInline = context.options[0] && context.options[0]['prefer-inline'];

Check warning on line 135 in src/rules/no-duplicates.js

View check run for this annotation

Codecov / codecov/patch

src/rules/no-duplicates.js#L135

Added line #L135 was not covered by tests

if (!(shouldAddDefault || shouldAddSpecifiers || shouldRemoveUnnecessary)) {
return undefined;
Expand All @@ -157,8 +158,7 @@ function getFix(first, rest, sourceCode, context) {
([result, needsComma, existingIdentifiers], specifier) => {
const isTypeSpecifier = specifier.importNode.importKind === 'type';

const preferInline = context.options[0] && context.options[0]['prefer-inline'];
// a user might set prefer-inline but not have a supporting TypeScript version. Flow does not support inline types so this should fail in that case as well.
// a user might set prefer-inline but not have a supporting TypeScript version. Flow does not support inline types so this should fail in that case as well.
if (preferInline && (!typescriptPkg || !semver.satisfies(typescriptPkg.version, '>= 4.5'))) {
throw new Error('Your version of TypeScript does not support inline type imports.');
}
Expand Down Expand Up @@ -186,6 +186,18 @@ function getFix(first, rest, sourceCode, context) {

const fixes = [];

if (shouldAddSpecifiers && preferInline && first.importKind === 'type') {

Check warning on line 189 in src/rules/no-duplicates.js

View check run for this annotation

Codecov / codecov/patch

src/rules/no-duplicates.js#L189

Added line #L189 was not covered by tests
// `import type {a} from './foo'` → `import {type a} from './foo'`
const typeIdentifierToken = tokens.find((token) => token.type === 'Identifier' && token.value === 'type');
fixes.push(fixer.removeRange([typeIdentifierToken.range[0], typeIdentifierToken.range[1] + 1]));

Check warning on line 192 in src/rules/no-duplicates.js

View check run for this annotation

Codecov / codecov/patch

src/rules/no-duplicates.js#L191-L192

Added lines #L191 - L192 were not covered by tests

tokens
.filter((token) => firstExistingIdentifiers.has(token.value))
.forEach((identifier) => {
fixes.push(fixer.replaceTextRange([identifier.range[0], identifier.range[1]], `type ${identifier.value}`));

Check warning on line 197 in src/rules/no-duplicates.js

View check run for this annotation

Codecov / codecov/patch

src/rules/no-duplicates.js#L194-L197

Added lines #L194 - L197 were not covered by tests
});
}

if (shouldAddDefault && openBrace == null && shouldAddSpecifiers) {
// `import './foo'` → `import def, {...} from './foo'`
fixes.push(
Expand Down
18 changes: 18 additions & 0 deletions tests/src/rules/no-duplicates.js
Original file line number Diff line number Diff line change
Expand Up @@ -704,6 +704,24 @@ context('TypeScript', function () {
},
],
}),
test({
code: "import type {x} from 'foo'; import {type y} from 'foo'",
...parserConfig,
options: [{ 'prefer-inline': true }],
output: `import {type x,type y} from 'foo'; `,
errors: [
{
line: 1,
column: 22,
message: "'foo' imported multiple times.",
},
{
line: 1,
column: 50,
message: "'foo' imported multiple times.",
},
],
}),
test({
code: "import {type x} from 'foo'; import type {y} from 'foo'",
...parserConfig,
Expand Down

0 comments on commit 4bdf61a

Please sign in to comment.