From c188a7edff3c92e54153bab86ac128f2b9fd388a Mon Sep 17 00:00:00 2001 From: Pascal Corpet Date: Mon, 28 May 2018 21:51:36 +0200 Subject: [PATCH] [New] `no-duplicates`: add a considerQueryString option to handle false positives when using some webpack loaders. Fixes #1107. --- CHANGELOG.md | 5 ++++- docs/rules/no-duplicates.md | 25 +++++++++++++++++++++++++ src/rules/no-duplicates.js | 25 ++++++++++++++++++++++++- tests/src/rules/no-duplicates.js | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 85 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2769c5544..0ea8a264b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,6 @@ This project adheres to [Semantic Versioning](http://semver.org/). This change log adheres to standards from [Keep a CHANGELOG](http://keepachangelog.com). ## [Unreleased] -- [`no-extraneous-dependencies`]: Check `export from` ([#1049], thanks [@marcusdarmstrong]) ### Added - [`internal-regex`]: regex pattern for marking packages "internal" ([#1491], thanks [@Librazy]) @@ -14,6 +13,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel - [`no-extraneous-dependencies`]: Implement support for [bundledDependencies](https://npm.github.io/using-pkgs-docs/package-json/types/bundleddependencies.html) ([#1436], thanks [@schmidsi])) - [`no-unused-modules`]: add flow type support ([#1542], thanks [@rfermann]) - [`order`]: Adds support for pathGroups to allow ordering by defined patterns ([#795], [#1386], thanks [@Mairu]) +- [`no-duplicates`]: Add `considerQueryString` option : allow duplicate imports with different query strings ([#1107], thanks [@pcorpet]). ### Fixed - [`default`]: make error message less confusing ([#1470], thanks [@golopot]) @@ -25,6 +25,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel - [`no-unused-modules`]: fix crash due to `export *` ([#1496], thanks [@Taranys]) - [`no-cycle`]: should not warn for Flow imports ([#1494], thanks [@maxmalov]) - [`order`]: fix `@someModule` considered as `unknown` instead of `internal` ([#1493], thanks [@aamulumi]) +- [`no-extraneous-dependencies`]: Check `export from` ([#1049], thanks [@marcusdarmstrong]) ### Docs - [`no-useless-path-segments`]: add docs for option `commonjs` ([#1507], thanks [@golopot]) @@ -692,6 +693,7 @@ for info on changes for earlier releases. [#1126]: https://github.com/benmosher/eslint-plugin-import/pull/1126 [#1122]: https://github.com/benmosher/eslint-plugin-import/pull/1122 [#1112]: https://github.com/benmosher/eslint-plugin-import/pull/1112 +[#1107]: https://github.com/benmosher/eslint-plugin-import/pull/1107 [#1106]: https://github.com/benmosher/eslint-plugin-import/pull/1106 [#1093]: https://github.com/benmosher/eslint-plugin-import/pull/1093 [#1085]: https://github.com/benmosher/eslint-plugin-import/pull/1085 @@ -1032,3 +1034,4 @@ for info on changes for earlier releases. [@marcusdarmstrong]: https://github.com/marcusdarmstrong [@Mairu]: https://github.com/Mairu [@aamulumi]: https://github.com/aamulumi +[@pcorpet]: https://github.com/pcorpet diff --git a/docs/rules/no-duplicates.md b/docs/rules/no-duplicates.md index 0641e4418..f59b14d9c 100644 --- a/docs/rules/no-duplicates.md +++ b/docs/rules/no-duplicates.md @@ -36,6 +36,31 @@ 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 + +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). + +Config: + +```json +"import/no-duplicates": ["error", {"considerQueryString": true}] +``` + +And then the following code becomes valid: +```js +import minifiedMod from './mod?minify' +import noCommentsMod from './mod?comments=0' +import originalMod from './mod' +``` + +It will still catch duplicates when using the same module and the exact same query string: +```js +import SomeDefaultClass from './mod?minify' + +// This is invalid, assuming `./mod` and `./mod.js` are the same target: +import * from './mod.js?minify' +``` + ## 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 33e335748..1334a1258 100644 --- a/src/rules/no-duplicates.js +++ b/src/rules/no-duplicates.js @@ -230,15 +230,38 @@ module.exports = { url: docsUrl('no-duplicates'), }, fixable: 'code', + schema: [ + { + type: 'object', + properties: { + considerQueryString: { + type: 'boolean', + }, + }, + additionalProperties: false, + }, + ], }, create: function (context) { + // 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 parts = sourcePath.match(/^([^?]*)\?(.*)$/) + if (!parts) { + return defaultResolver(sourcePath) + } + return defaultResolver(parts[1]) + '?' + parts[2] + }) : defaultResolver + const imported = new Map() const typesImported = new Map() return { 'ImportDeclaration': function (n) { // resolved path will cover aliased duplicates - const resolvedPath = resolve(n.source.value, context) || n.source.value + const resolvedPath = resolver(n.source.value) const importMap = n.importKind === 'type' ? typesImported : imported if (importMap.has(resolvedPath)) { diff --git a/tests/src/rules/no-duplicates.js b/tests/src/rules/no-duplicates.js index a93fdfa92..a4c41f677 100644 --- a/tests/src/rules/no-duplicates.js +++ b/tests/src/rules/no-duplicates.js @@ -25,6 +25,18 @@ ruleTester.run('no-duplicates', rule, { code: "import { x } from './foo'; import type { y } from './foo'", parser: require.resolve('babel-eslint'), }), + + // #1107: Using different query strings that trigger different webpack loaders. + test({ + code: "import x from './bar?optionX'; import y from './bar?optionY';", + options: [{'considerQueryString': true}], + settings: { 'import/resolver': 'webpack' }, + }), + test({ + code: "import x from './foo'; import y from './bar';", + options: [{'considerQueryString': true}], + settings: { 'import/resolver': 'webpack' }, + }), ], invalid: [ test({ @@ -50,6 +62,26 @@ ruleTester.run('no-duplicates', rule, { errors: 2, // path ends up hardcoded }), + // #1107: Using different query strings that trigger different webpack loaders. + test({ + code: "import x from './bar.js?optionX'; import y from './bar?optionX';", + settings: { 'import/resolver': 'webpack' }, + errors: 2, // path ends up hardcoded + }), + test({ + code: "import x from './bar?optionX'; import y from './bar?optionY';", + settings: { 'import/resolver': 'webpack' }, + errors: 2, // path ends up hardcoded + }), + + // #1107: Using same query strings that trigger the same loader. + test({ + code: "import x from './bar?optionX'; import y from './bar.js?optionX';", + options: [{'considerQueryString': true}], + settings: { 'import/resolver': 'webpack' }, + errors: 2, // path ends up hardcoded + }), + // #86: duplicate unresolved modules should be flagged test({ code: "import foo from 'non-existent'; import bar from 'non-existent';",