Skip to content

Commit

Permalink
no-duplicates: add a query-string option to handle false positives wh…
Browse files Browse the repository at this point in the history
…en using some webpack loaders. Fixes import-js#1107.
  • Loading branch information
pcorpet committed May 28, 2018
1 parent ebafcbf commit 335977a
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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
Expand Down
25 changes: 25 additions & 0 deletions docs/rules/no-duplicates.md
Expand Up @@ -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.
Expand Down
13 changes: 12 additions & 1 deletion src/rules/no-duplicates.js
Expand Up @@ -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)) {
Expand Down
27 changes: 27 additions & 0 deletions tests/src/rules/no-duplicates.js
Expand Up @@ -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({
Expand All @@ -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';",
Expand Down

0 comments on commit 335977a

Please sign in to comment.