Skip to content

Commit

Permalink
[Fix] extensions: Ignore query strings when checking for extensions.
Browse files Browse the repository at this point in the history
Fixes #1567.
  • Loading branch information
pcorpet authored and ljharb committed Dec 15, 2019
1 parent b791ba5 commit 982493d
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 4 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -14,6 +14,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
- [`import/extensions`]: ignore non-main modules ([#1563], thanks [@saschanaz])
- TypeScript config: lookup for external modules in @types folder ([#1526], thanks [@joaovieira])
- [`no-extraneous-dependencies`]: ensure `node.source` is truthy ([#1589], thanks [@ljharb])
- [`extensions`]: Ignore query strings when checking for extensions ([#1572], thanks [@pcorpet])

## [2.19.1] - 2019-12-08
### Fixed
Expand Down Expand Up @@ -631,6 +632,7 @@ for info on changes for earlier releases.

[#1589]: https://github.com/benmosher/eslint-plugin-import/issues/1589
[#1586]: https://github.com/benmosher/eslint-plugin-import/pull/1586
[#1572]: https://github.com/benmosher/eslint-plugin-import/pull/1572
[#1563]: https://github.com/benmosher/eslint-plugin-import/pull/1563
[#1560]: https://github.com/benmosher/eslint-plugin-import/pull/1560
[#1551]: https://github.com/benmosher/eslint-plugin-import/pull/1551
Expand Down
10 changes: 6 additions & 4 deletions src/rules/extensions.js
Expand Up @@ -132,10 +132,12 @@ module.exports = {
// bail if the declaration doesn't have a source, e.g. "export { foo };"
if (!source) return

const importPath = source.value
const importPathWithQueryString = source.value

// don't enforce anything on builtins
if (isBuiltIn(importPath, context.settings)) return
if (isBuiltIn(importPathWithQueryString, context.settings)) return

const importPath = importPathWithQueryString.replace(/\?(.*)$/, '')

const resolvedPath = resolve(importPath, context)

Expand All @@ -154,14 +156,14 @@ module.exports = {
context.report({
node: source,
message:
`Missing file extension ${extension ? `"${extension}" ` : ''}for "${importPath}"`,
`Missing file extension ${extension ? `"${extension}" ` : ''}for "${importPathWithQueryString}"`,
})
}
} else if (extension) {
if (isUseOfExtensionForbidden(extension) && isResolvableWithoutExtension(importPath)) {
context.report({
node: source,
message: `Unexpected use of file extension "${extension}" for "${importPath}"`,
message: `Unexpected use of file extension "${extension}" for "${importPathWithQueryString}"`,
})
}
}
Expand Down
34 changes: 34 additions & 0 deletions tests/src/rules/extensions.js
Expand Up @@ -116,6 +116,16 @@ ruleTester.run('extensions', rule, {
].join('\n'),
options: [ 'never' ],
}),

// Query strings.
test({
code: 'import bare from "./foo?a=True.ext"',
options: [ 'never' ],
}),
test({
code: 'import bare from "./foo.js?a=True"',
options: [ 'always' ],
}),
],

invalid: [
Expand Down Expand Up @@ -375,5 +385,29 @@ ruleTester.run('extensions', rule, {
},
],
}),

// Query strings.
test({
code: 'import withExtension from "./foo.js?a=True"',
options: [ 'never' ],
errors: [
{
message: 'Unexpected use of file extension "js" for "./foo.js?a=True"',
line: 1,
column: 27,
},
],
}),
test({
code: 'import withoutExtension from "./foo?a=True.ext"',
options: [ 'always' ],
errors: [
{
message: 'Missing file extension for "./foo?a=True.ext"',
line: 1,
column: 30,
},
],
}),
],
})

0 comments on commit 982493d

Please sign in to comment.