Skip to content

Commit 0328e99

Browse files
committed
no-duplicates: add a considerQueryString option to handle false positives when using some webpack loaders. Fixes import-js#1107.
1 parent 5480240 commit 0328e99

File tree

4 files changed

+78
-1
lines changed

4 files changed

+78
-1
lines changed

CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
44
This change log adheres to standards from [Keep a CHANGELOG](http://keepachangelog.com).
55

66
## [Unreleased]
7+
### Added
8+
- Add `considerQueryString` option to [`no-duplicates`] rule: allow duplicate imports with different query strings ([#1107], thanks [@pcorpet]).
79

810
## [2.13.0] - 2018-06-24
911
### Added

docs/rules/no-duplicates.md

+25
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,31 @@ The motivation is that this is likely a result of two developers importing diffe
3535
names from the same module at different times (and potentially largely different
3636
locations in the file.) This rule brings both (or n-many) to attention.
3737

38+
### Query Strings
39+
40+
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).
41+
42+
Config:
43+
44+
```json
45+
"import/no-duplicates": ["error", {"considerQueryString": true}]
46+
```
47+
48+
And then the following code becomes valid:
49+
```js
50+
import minifiedMod from './mod?minify'
51+
import noCommentsMod from './mod?comments=0'
52+
import originalMod from './mod'
53+
```
54+
55+
It will still catch duplicates when using the same module and the exact same query string:
56+
```js
57+
import SomeDefaultClass from './mod?minify'
58+
59+
// This is invalid, assuming `./mod` and `./mod.js` are the same target:
60+
import * from './mod.js?minify'
61+
```
62+
3863
## When Not To Use It
3964

4065
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.

src/rules/no-duplicates.js

+24-1
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,38 @@ module.exports = {
1616
docs: {
1717
url: docsUrl('no-duplicates'),
1818
},
19+
schema: [
20+
{
21+
type: 'object',
22+
properties: {
23+
considerQueryString: {
24+
type: 'boolean',
25+
},
26+
},
27+
additionalProperties: false,
28+
},
29+
],
1930
},
2031

2132
create: function (context) {
33+
// Prepare the resolver from options.
34+
const considerQueryStringOption = context.options[0] &&
35+
context.options[0]['considerQueryString']
36+
const defaultResolver = sourcePath => resolve(sourcePath, context) || sourcePath
37+
const resolver = considerQueryStringOption ? (sourcePath => {
38+
const parts = sourcePath.match(/^([^?]*)\?(.*)$/)
39+
if (!parts) {
40+
return defaultResolver(sourcePath)
41+
}
42+
return defaultResolver(parts[1]) + '?' + parts[2]
43+
}) : defaultResolver
44+
2245
const imported = new Map()
2346
const typesImported = new Map()
2447
return {
2548
'ImportDeclaration': function (n) {
2649
// resolved path will cover aliased duplicates
27-
const resolvedPath = resolve(n.source.value, context) || n.source.value
50+
const resolvedPath = resolver(n.source.value)
2851
const importMap = n.importKind === 'type' ? typesImported : imported
2952

3053
if (importMap.has(resolvedPath)) {

tests/src/rules/no-duplicates.js

+27
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,13 @@ ruleTester.run('no-duplicates', rule, {
2121
code: "import { x } from './foo'; import type { y } from './foo'",
2222
parser: 'babel-eslint',
2323
}),
24+
25+
// #1107: Using different query strings that trigger different webpack loaders.
26+
test({
27+
code: "import x from './bar?optionX'; import y from './bar?optionY';",
28+
options: [{'considerQueryString': true}],
29+
settings: { 'import/resolver': 'webpack' },
30+
}),
2431
],
2532
invalid: [
2633
test({
@@ -43,6 +50,26 @@ ruleTester.run('no-duplicates', rule, {
4350
errors: 2, // path ends up hardcoded
4451
}),
4552

53+
// #1107: Using different query strings that trigger different webpack loaders.
54+
test({
55+
code: "import x from './bar.js?optionX'; import y from './bar?optionX';",
56+
settings: { 'import/resolver': 'webpack' },
57+
errors: 2, // path ends up hardcoded
58+
}),
59+
test({
60+
code: "import x from './bar?optionX'; import y from './bar?optionY';",
61+
settings: { 'import/resolver': 'webpack' },
62+
errors: 2, // path ends up hardcoded
63+
}),
64+
65+
// #1107: Using same query strings that trigger the same loader.
66+
test({
67+
code: "import x from './bar?optionX'; import y from './bar.js?optionX';",
68+
options: [{'considerQueryString': true}],
69+
settings: { 'import/resolver': 'webpack' },
70+
errors: 2, // path ends up hardcoded
71+
}),
72+
4673
// #86: duplicate unresolved modules should be flagged
4774
test({
4875
code: "import foo from 'non-existent'; import bar from 'non-existent';",

0 commit comments

Comments
 (0)