Sitelet https://github.com/import-js/eslint-plugin-import/pull/1297
Skip to content

fix: aliased internal modules that look like core modules - #1297

Merged
ljharb merged 2 commits into
import-js:masterfrom
echenley:ech/fix-isBuiltIn-local-aliases
Apr 11, 2019
Merged

ljharb merged 2 commits into
import-js:masterfrom
echenley:ech/fix-isBuiltIn-local-aliases

Conversation

@echenley

@echenley echenley commented Mar 6, 2019 •

Copy link
Copy Markdown
Contributor

What

Update isBuiltIn to return false if a path is returned from a resolver.
Previously an alias like 'constants/foo' would be classified as a builtin rather than internal since 'constants' is a core module of node. Fixes one of the issues mentioned in #1034.

Update webpack resolver to attempt to resolve modules internally before classifying them as builtin.

Add several tests to cover these cases.

Example

project
└── src
    └── constants
        └── index.js
{
  "settings": {
    "import/resolver": {
      "node": {
        "paths": ["/project/src"]
      }
    }
  }
}

Result:

// BEFORE
import { FOO } from 'constants/index'
import { isObject } from 'lodash'

// AFTER
import { isObject } from 'lodash'
import { FOO } from 'constants/index'

Comment thread tests/src/core/importType.js Outdated
Comment thread tests/src/core/importType.js Outdated
@coveralls

coveralls commented Mar 6, 2019 •

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.001%) to 97.863% when pulling ba0aed9 on echenley:ech/fix-isBuiltIn-local-aliases into 0ff1c83 on benmosher:master.

Comment thread src/core/importType.js Outdated
Comment thread tests/src/core/importType.js Outdated
Comment thread tests/src/core/importType.js Outdated
@ljharb
ljharb requested a review from benmosher March 16, 2019 06:31
@karuana

karuana commented Mar 22, 2019

Copy link
Copy Markdown

When will this PR be merged?
I am waiting for this PR to be merged. 😭😭😭

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants