[New] no-duplicates: add a considerQueryString option to handle false positives when using some webpack loaders.

Fixes #1107.

Author: pcorpet

Diff for: 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 for: 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 for: 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 for: 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';",