From 335977a063b94c8ca9160eb2e5d3a2f258f58ee9 Mon Sep 17 00:00:00 2001
From: Pascal Corpet <pascal@bayesimpact.org>
Date: Mon, 28 May 2018 21:51:36 +0200
Subject: [PATCH] no-duplicates: add a query-string option to handle false
 positives when using some webpack loaders. Fixes #1107.

---
 CHANGELOG.md                     |  1 +
 docs/rules/no-duplicates.md      | 25 +++++++++++++++++++++++++
 src/rules/no-duplicates.js       | 13 ++++++++++++-
 tests/src/rules/no-duplicates.js | 27 +++++++++++++++++++++++++++
 4 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 628a9a9448..320644a103 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
 
 ## [Unreleased]
 - Add [`no-relative-parent-imports`] rule: disallow relative imports from parent directories.
+- Add `query-string` option to [`no-duplicates`] rule: allow duplicate imports with different query strings ([#1107], thanks [@pcorpet]).
 
 ## [2.12.0] - 2018-05-17
 ### Added
diff --git a/docs/rules/no-duplicates.md b/docs/rules/no-duplicates.md
index 580f360119..8677afb211 100644
--- a/docs/rules/no-duplicates.md
+++ b/docs/rules/no-duplicates.md
@@ -35,6 +35,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 `query-string` to handle them as different (usually because a webpack loader would actually resolve those imports differently).
+
+Config:
+
+```json
+"import/no-duplicates": ["error", {"query-string": "different"}]
+```
+
+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 72b305e677..6c73b96fb0 100644
--- a/src/rules/no-duplicates.js
+++ b/src/rules/no-duplicates.js
@@ -19,12 +19,23 @@ module.exports = {
   },
 
   create: function (context) {
+    const queryStringOption = context.options[0] && context.options[0]['query-string']
     const imported = new Map()
     const typesImported = new Map()
+
+    const defaultResolver = sourcePath => resolve(sourcePath, context) || sourcePath
+    const resolver = queryStringOption === 'different' ? (sourcePath => {
+      const parts = sourcePath.match(/^([^?]*)\?(.*)$/)
+      if (!parts) {
+        return defaultResolver(sourcePath)
+      }
+      return defaultResolver(parts[1]) + '?' + parts[2]
+    }) : defaultResolver
+
     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 82bccdee05..19a182e35e 100644
--- a/tests/src/rules/no-duplicates.js
+++ b/tests/src/rules/no-duplicates.js
@@ -21,6 +21,13 @@ ruleTester.run('no-duplicates', rule, {
       code: "import { x } from './foo'; import type { y } from './foo'",
       parser: '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: [{'query-string': 'different'}],
+      settings: { 'import/resolver': 'webpack' },
+     }),
   ],
   invalid: [
     test({
@@ -43,6 +50,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: [{'query-string': 'different'}],
+      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';",