Skip to content

Commit

Permalink
[Fix] extensions: Ignore root external modules when checking for ex…
Browse files Browse the repository at this point in the history
…tensions
  • Loading branch information
adrian-seijo authored and ljharb committed Nov 11, 2016
1 parent aed7a73 commit cd25d9d
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 12 deletions.
4 changes: 4 additions & 0 deletions src/core/importType.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ function typeTest(name, settings, path) {
return 'unknown'
}

export function isScopedModule(name) {
return name.indexOf('@') === 0
}

export default function resolveImportType(name, context) {
return typeTest(name, context.settings, resolve(name, context))
}
14 changes: 13 additions & 1 deletion src/rules/extensions.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import path from 'path'

import resolve from 'eslint-module-utils/resolve'
import { isBuiltIn, isExternalModule, isScoped } from '../core/importType'
import { isBuiltIn, isExternalModule, isScoped, isScopedModule } from '../core/importType'
import docsUrl from '../docsUrl'

const enumValues = { enum: [ 'always', 'ignorePackages', 'never' ] }
Expand Down Expand Up @@ -126,6 +126,14 @@ module.exports = {
return resolvedFileWithoutExtension === resolve(file, context)
}

function isExternalRootModule(file) {
const slashCount = file.split('/').length - 1

if (isScopedModule(file) && slashCount <= 1) return true
if (isExternalModule(file, context, resolve(file, context)) && !slashCount) return true
return false
}

function checkFileExtension(node) {
const { source } = node

Expand All @@ -139,6 +147,10 @@ module.exports = {

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

// don't enforce in root external packages as they may have names with `.js`.
// Like `import Decimal from decimal.js`)
if (isExternalRootModule(importPath)) return

const resolvedPath = resolve(importPath, context)

// get extension from resolved path, if possible.
Expand Down
56 changes: 45 additions & 11 deletions tests/src/rules/extensions.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,16 @@ ruleTester.run('extensions', rule, {
options: [ 'never' ],
}),

// Root packages should be ignored and they are names not files
test({
code: [
'import lib from "pkg.js"',
'import lib2 from "pgk/package"',
'import lib3 from "@name/pkg.js"',
].join('\n'),
options: [ 'never' ],
}),

// Query strings.
test({
code: 'import bare from "./foo?a=True.ext"',
Expand All @@ -126,6 +136,15 @@ ruleTester.run('extensions', rule, {
code: 'import bare from "./foo.js?a=True"',
options: [ 'always' ],
}),

test({
code: [
'import lib from "pkg"',
'import lib2 from "pgk/package.js"',
'import lib3 from "@name/pkg"',
].join('\n'),
options: [ 'always' ],
}),
],

invalid: [
Expand All @@ -137,15 +156,6 @@ ruleTester.run('extensions', rule, {
column: 15,
} ],
}),
test({
code: 'import a from "a"',
options: [ 'always' ],
errors: [ {
message: 'Missing file extension "js" for "a"',
line: 1,
column: 15,
} ],
}),
test({
code: 'import dot from "./file.with.dot"',
options: [ 'always' ],
Expand Down Expand Up @@ -285,11 +295,35 @@ ruleTester.run('extensions', rule, {
],
}),
test({
code: 'import thing from "non-package"',
code: 'import thing from "non-package/test"',
options: [ 'always' ],
errors: [
{
message: 'Missing file extension for "non-package"',
message: 'Missing file extension for "non-package/test"',
line: 1,
column: 19,
},
],
}),

test({
code: 'import thing from "@name/pkg/test"',
options: [ 'always' ],
errors: [
{
message: 'Missing file extension for "@name/pkg/test"',
line: 1,
column: 19,
},
],
}),

test({
code: 'import thing from "@name/pkg/test.js"',
options: [ 'never' ],
errors: [
{
message: 'Unexpected use of file extension "js" for "@name/pkg/test.js"',
line: 1,
column: 19,
},
Expand Down

0 comments on commit cd25d9d

Please sign in to comment.