Skip to content

Commit

Permalink
no-duplicates: add a considerQueryString option to handle false posit…
Browse files Browse the repository at this point in the history
…ives when using some webpack loaders. Fixes import-js#1107.
  • Loading branch information
pcorpet committed Jun 13, 2018
1 parent ebafcbf commit 33925b6
Show file tree
Hide file tree
Showing 4 changed files with 77 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 `considerQueryString` 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 `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.
Expand Down
25 changes: 24 additions & 1 deletion src/rules/no-duplicates.js
Expand Up @@ -16,15 +16,38 @@ module.exports = {
docs: {
url: docsUrl('no-duplicates'),
},
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)) {
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: [{'considerQueryString': true}],
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: [{'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';",
Expand Down

0 comments on commit 33925b6

Please sign in to comment.