Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #1297 from echenley/ech/fix-isBuiltIn-local-aliases
fix: aliased internal modules that look like core modules
  • Loading branch information
ljharb committed Apr 11, 2019
2 parents 0ff1c83 + ba0aed9 commit 2d21c4c
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 10 deletions.
12 changes: 6 additions & 6 deletions resolvers/webpack/index.js
Expand Up @@ -38,10 +38,6 @@ exports.resolve = function (source, file, settings) {
source = source.slice(0, finalQuestionMark)
}

if (source in coreLibs) {
return { found: true, path: coreLibs[source] }
}

var webpackConfig

var configPath = get(settings, 'config')
Expand Down Expand Up @@ -73,7 +69,7 @@ exports.resolve = function (source, file, settings) {
throw e
}
} else {
log("No config path found relative to", file, "; using {}")
log('No config path found relative to', file, '; using {}')
webpackConfig = {}
}

Expand Down Expand Up @@ -123,6 +119,10 @@ exports.resolve = function (source, file, settings) {
try {
return { found: true, path: resolveSync(path.dirname(file), source) }
} catch (err) {
if (source in coreLibs) {
return { found: true, path: coreLibs[source] }
}

log('Error during module resolution:', err)
return { found: false }
}
Expand All @@ -136,7 +136,7 @@ function getResolveSync(configPath, webpackConfig) {
if (!cached) {
cached = {
key: cacheKey,
value: createResolveSync(configPath, webpackConfig)
value: createResolveSync(configPath, webpackConfig),
}
// put in front and pop last item
if (_cache.unshift(cached) > MAX_CACHE) {
Expand Down
4 changes: 3 additions & 1 deletion src/core/importType.js
Expand Up @@ -21,7 +21,9 @@ export function isAbsolute(name) {
return name.indexOf('/') === 0
}

export function isBuiltIn(name, settings) {
// path is defined only when a resolver resolves to a non-standard path
export function isBuiltIn(name, settings, path) {
if (path) return false
const base = baseModule(name)
const extras = (settings && settings['import/core-modules']) || []
return coreModules[base] || extras.indexOf(base) > -1
Expand Down
1 change: 1 addition & 0 deletions tests/files/constants/index.js
@@ -0,0 +1 @@
export const FOO = 'FOO'
23 changes: 20 additions & 3 deletions tests/src/core/importType.js
Expand Up @@ -7,6 +7,7 @@ import { testContext } from '../utils'

describe('importType(name)', function () {
const context = testContext()
const pathToTestFiles = path.join(__dirname, '..', '..', 'files')

it("should return 'absolute' for paths starting with a /", function() {
expect(importType('/', context)).to.equal('absolute')
Expand Down Expand Up @@ -42,20 +43,36 @@ describe('importType(name)', function () {
})

it("should return 'internal' for non-builtins resolved outside of node_modules", function () {
const pathContext = testContext({ "import/resolver": { node: { paths: [ path.join(__dirname, '..', '..', 'files') ] } } })
const pathContext = testContext({ "import/resolver": { node: { paths: [pathToTestFiles] } } })
expect(importType('importType', pathContext)).to.equal('internal')
})

it.skip("should return 'internal' for scoped packages resolved outside of node_modules", function () {
const pathContext = testContext({ "import/resolver": { node: { paths: [ path.join(__dirname, '..', '..', 'files') ] } } })
const pathContext = testContext({ "import/resolver": { node: { paths: [pathToTestFiles] } } })
expect(importType('@importType/index', pathContext)).to.equal('internal')
})

it("should return 'internal' for internal modules that are referenced by aliases", function () {
const pathContext = testContext({ 'import/resolver': { node: { paths: [path.join(__dirname, '..', '..', 'files')] } } })
const pathContext = testContext({ 'import/resolver': { node: { paths: [pathToTestFiles] } } })
expect(importType('@my-alias/fn', pathContext)).to.equal('internal')
})

it("should return 'internal' for aliased internal modules that look like core modules (node resolver)", function () {
const pathContext = testContext({ 'import/resolver': { node: { paths: [pathToTestFiles] } } })
expect(importType('constants/index', pathContext)).to.equal('internal')
expect(importType('constants/', pathContext)).to.equal('internal')
// resolves exact core modules over internal modules
expect(importType('constants', pathContext)).to.equal('builtin')
})

it("should return 'internal' for aliased internal modules that look like core modules (webpack resolver)", function () {
const webpackConfig = { resolve: { modules: [pathToTestFiles, 'node_modules'] } }
const pathContext = testContext({ 'import/resolver': { webpack: { config: webpackConfig } } })
expect(importType('constants/index', pathContext)).to.equal('internal')
expect(importType('constants/', pathContext)).to.equal('internal')
expect(importType('constants', pathContext)).to.equal('internal')
})

it("should return 'parent' for internal modules that go through the parent", function() {
expect(importType('../foo', context)).to.equal('parent')
expect(importType('../../foo', context)).to.equal('parent')
Expand Down

0 comments on commit 2d21c4c

Please sign in to comment.