diff --git a/CHANGELOG.md b/CHANGELOG.md index 9203739fd..70846eee1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel - [`no-unused-modules`]: add eslint v8 support ([#2194], thanks [@coderaiser]) - [`no-restricted-paths`]: add/restore glob pattern support ([#2219], thanks [@stropho]) - [`no-unused-modules`]: support dynamic imports ([#1660], [#2212], thanks [@maxkomarychev], [@aladdin-add], [@Hypnosphi]) +- [`no-duplicates`]: add `combineTypeImports` option to de-duplicate flow type imports ([#2229], thanks [@christianvuerings]) ## [2.24.2] - 2021-08-24 @@ -906,6 +907,7 @@ for info on changes for earlier releases. [`memo-parser`]: ./memo-parser/README.md +[#2229]: https://github.com/import-js/eslint-plugin-import/pull/2229 [#2219]: https://github.com/import-js/eslint-plugin-import/pull/2219 [#2212]: https://github.com/import-js/eslint-plugin-import/pull/2212 [#2196]: https://github.com/import-js/eslint-plugin-import/pull/2196 diff --git a/docs/rules/no-duplicates.md b/docs/rules/no-duplicates.md index c37510325..bc4f2dcca 100644 --- a/docs/rules/no-duplicates.md +++ b/docs/rules/no-duplicates.md @@ -14,12 +14,11 @@ is different in two key ways: Valid: ```js import SomeDefaultClass, * as names from './mod' -// Flow `type` import from same module is fine +// By default the flow `type` import from same module is fine (see combineTypeImports) import type SomeType from './mod' ``` -...whereas here, both `./mod` imports will be reported: - +Invalid: ```js import SomeDefaultClass from './mod' @@ -36,7 +35,8 @@ The motivation is that this is likely a result of two developers importing diffe names from the same module at different times (and potentially largely different locations in the file.) This rule brings both (or n-many) to attention. -### Query Strings +## Options +### considerQueryString By default, this rule ignores query strings (i.e. paths followed by a question mark), and thus imports from `./mod?a` and `./mod?b` will be considered as duplicates. However you can use the option `considerQueryString` to handle them as different (primarily because browsers will resolve those imports differently). @@ -61,6 +61,25 @@ import SomeDefaultClass from './mod?minify' import * from './mod.js?minify' ``` +### combineTypeImports + +By default, this rule ignores flow & typescript type imports. However you can use the option `combineTypeImports` to combine type imports with regular imports. + +Currently only `flow` is supported. + +Valid: +```js +/*eslint import/no-unresolved: [2, { combineTypeImports: "flow" }]*/ +import x, { type y } from './foo' +``` + +Invalid: +```js +/*eslint import/no-unresolved: [2, { combineTypeImports: "flow" }]*/ +import { type y } from './foo' // reported since './foo' is imported twice +import { x } from './foo' +``` + ## When Not To Use It If the core ESLint version is good enough (i.e. you're _not_ using Flow and you _are_ using [`import/extensions`](./extensions.md)), keep it and don't use this. diff --git a/src/rules/no-duplicates.js b/src/rules/no-duplicates.js index c887b39e7..b0c614e73 100644 --- a/src/rules/no-duplicates.js +++ b/src/rules/no-duplicates.js @@ -1,13 +1,13 @@ import resolve from 'eslint-module-utils/resolve'; import docsUrl from '../docsUrl'; -function checkImports(imported, context) { +function checkImports(imported, context, combineTypeImports) { for (const [module, nodes] of imported.entries()) { if (nodes.length > 1) { const message = `'${module}' imported multiple times.`; const [first, ...rest] = nodes; const sourceCode = context.getSourceCode(); - const fix = getFix(first, rest, sourceCode); + const fix = getFix(first, rest, sourceCode, combineTypeImports); context.report({ node: first.source, @@ -25,7 +25,7 @@ function checkImports(imported, context) { } } -function getFix(first, rest, sourceCode) { +function getFix(first, rest, sourceCode, combineTypeImports) { // Sorry ESLint <= 3 users, no autofix for you. Autofixing duplicate imports // requires multiple `fixer.whatever()` calls in the `fix`: We both need to // update the first one, and remove the rest. Support for multiple @@ -107,15 +107,15 @@ function getFix(first, rest, sourceCode) { const firstIsEmpty = !hasSpecifiers(first); const [specifiersText] = specifiers.reduce( - ([result, needsComma], specifier) => { - return [ - needsComma && !specifier.isEmpty - ? `${result},${specifier.text}` - : `${result}${specifier.text}`, - specifier.isEmpty ? needsComma : true, - ]; - }, - ['', !firstHasTrailingComma && !firstIsEmpty] + ([result, needsComma], specifier) => [ + `${result}${needsComma && !specifier.isEmpty ? ',' : ''}${ + combineTypeImports === 'flow' && specifier.importNode.importKind === 'type' + ? `type ${specifier.text.trim()}` + : specifier.text + }`, + specifier.isEmpty ? needsComma : true, + ], + ['', !firstHasTrailingComma && !firstIsEmpty], ); const fixes = []; @@ -255,6 +255,10 @@ module.exports = { considerQueryString: { type: 'boolean', }, + combineTypeImports: { + enum: ['flow', 'disabled'], + default: 'disabled', + }, }, additionalProperties: false, }, @@ -262,11 +266,13 @@ module.exports = { }, create(context) { + const { + considerQueryString = false, + combineTypeImports = 'disabled', + } = context.options[0] || {}; // Prepare the resolver from options. - const considerQueryStringOption = context.options[0] && - context.options[0]['considerQueryString']; const defaultResolver = sourcePath => resolve(sourcePath, context) || sourcePath; - const resolver = considerQueryStringOption ? (sourcePath => { + const resolver = considerQueryString ? (sourcePath => { const parts = sourcePath.match(/^([^?]*)\?(.*)$/); if (!parts) { return defaultResolver(sourcePath); @@ -280,7 +286,7 @@ module.exports = { const namedTypesImported = new Map(); function getImportMap(n) { - if (n.importKind === 'type') { + if (n.importKind === 'type' && combineTypeImports === 'disabled') { return n.specifiers.length > 0 && n.specifiers[0].type === 'ImportDefaultSpecifier' ? defaultTypesImported : namedTypesImported; } @@ -301,10 +307,10 @@ module.exports = { }, 'Program:exit': function () { - checkImports(imported, context); - checkImports(nsImported, context); - checkImports(defaultTypesImported, context); - checkImports(namedTypesImported, context); + checkImports(imported, context, combineTypeImports); + checkImports(nsImported, context, combineTypeImports); + checkImports(defaultTypesImported, context, combineTypeImports); + checkImports(namedTypesImported, context, combineTypeImports); }, }; }, diff --git a/tests/src/rules/no-duplicates.js b/tests/src/rules/no-duplicates.js index e550b63ce..251c1cb24 100644 --- a/tests/src/rules/no-duplicates.js +++ b/tests/src/rules/no-duplicates.js @@ -412,6 +412,51 @@ import {x,y} from './foo' output: "import Bar, { Foo } from './foo';\nexport const value = {}", errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'], }), + + // #2229 add option to de-duplicate flow type imports + test({ + code: "import { x } from './foo'; import type { y } from './foo'", + output: "import { x ,type y} from './foo'; ", + errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'], + options: [{ 'combineTypeImports': 'flow' }], + parser: require.resolve('babel-eslint'), + }), + + // #2229 add option to de-duplicate flow type imports + test({ + code: "import { x } from './foo'; import type {y} from './foo'", + output: "import { x ,type y} from './foo'; ", + errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'], + options: [{ 'combineTypeImports': 'flow' }], + parser: require.resolve('babel-eslint'), + }), + + // #2229 add option to de-duplicate flow type imports + test({ + code: "import { type x } from './foo'; import { y } from './foo'", + output: "import { type x , y } from './foo'; ", + errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'], + options: [{ 'combineTypeImports': 'flow' }], + parser: require.resolve('babel-eslint'), + }), + + // #2229 add option to de-duplicate flow type imports + test({ + code: "import { type x } from './foo'; import type { y } from './foo'", + output: "import { type x ,type y} from './foo'; ", + errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'], + options: [{ 'combineTypeImports': 'flow' }], + parser: require.resolve('babel-eslint'), + }), + + // #2229 add option to de-duplicate flow type imports + test({ + code: "import { type x } from './foo'; import type { y } from './foo'; import type { z } from './foo'", + output: "import { type x ,type y,type z} from './foo'; ", + errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'], + options: [{ 'combineTypeImports': 'flow' }], + parser: require.resolve('babel-eslint'), + }), ], }); @@ -429,7 +474,7 @@ context('TypeScript', function () { ruleTester.run('no-duplicates', rule, { valid: [ - // #1667: ignore duplicate if is a typescript type import + // #1667: ignore duplicate if is a typescript type import test({ code: "import type { x } from './foo'; import y from './foo'", ...parserConfig,