Skip to content

Conversation

jkrems
Copy link
Contributor

@jkrems jkrems commented Nov 16, 2015

By default eslint always resolves files in extends relative to the top-level project. Using require.resolve converts the references to absolute paths. Example:

node_modules/
  eslint-config-groupon/
    node_modules/
      eslint-config-airbnb/
        index.js # refers to 'eslint-config-airbnb/base'

By default eslint will look for eslint-config-airbnb/base in the top-level node_modules directory instead of one level down. By using require.resolve the "normal" npm resolution rules apply.

We use this approach in eslint-config-groupon. This change would allow us to get rid of the uglier parts in that repo.

@ljharb
Copy link
Collaborator

ljharb commented Nov 16, 2015

This seems like something eslint should be handling - require.resolve is something that generally shouldn't be done outside of bundling tools.

@jkrems
Copy link
Contributor Author

jkrems commented Nov 16, 2015

Reading the existing discussion doesn't make it sound like they want to address this anytime soon. But I understand why you'd be uncomfortable with this change.

@justjake
Copy link
Collaborator

This seems legit to me -- do linting & tests still pass after this change?

@jkrems
Copy link
Contributor Author

jkrems commented Nov 17, 2015

The tests of this repo itself pass (e.g. running npm test in packages/eslint-config-airbnb).

@joshhunt
Copy link

joshhunt commented Dec 6, 2015

Keen to get this in :) This issue is preventing our internal config from upgrading to latest config.

justjake added a commit that referenced this pull request Dec 15, 2015
Use require.resolve to allow nested extend
@justjake justjake merged commit a7541c9 into airbnb:master Dec 15, 2015
@jkrems jkrems deleted the jk-require-resolve branch December 15, 2015 16:46
jgkim referenced this pull request in tipplrio/eslint-config-tipplr Dec 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants