Skip to content

Commit 7aa4ce2

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 7aa4ce2

File tree

4 files changed

+83
-1
lines changed

4 files changed

+83
-1
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
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

Lines changed: 25 additions & 0 deletions
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

Lines changed: 24 additions & 1 deletion
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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,18 @@ 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+
}),
31+
test({
32+
code: "import x from './foo'; import y from './bar';",
33+
options: [{'considerQueryString': true}],
34+
settings: { 'import/resolver': 'webpack' },
35+
}),
2436
],
2537
invalid: [
2638
test({
@@ -43,6 +55,26 @@ ruleTester.run('no-duplicates', rule, {
4355
errors: 2, // path ends up hardcoded
4456
}),
4557

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

0 commit comments

Comments
 (0)