Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

npm 5 link / lerna resolve issue #9746

Closed
chrisblossom opened this issue Dec 20, 2017 · 11 comments
Closed

npm 5 link / lerna resolve issue #9746

chrisblossom opened this issue Dec 20, 2017 · 11 comments
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion question This issue asks a question about ESLint works as intended The behavior described in this issue is working correctly

Comments

@chrisblossom
Copy link

chrisblossom commented Dec 20, 2017

Tell us about your environment

  • ESLint Version: 4.13.1
  • Node Version: 8.9.3
  • npm Version: 5.6.0

What parser (default, Babel-ESLint, etc.) are you using?
default

Issue:
When using Lerna with npm v5, eslint is unable to correctly resolve linked packages.

Example Repo: chrisblossom/eslint-issue-9746

Updated repo to include both scoped and non-scoped example.

What actually happened? Please include the actual, raw output from ESLint.

example git:(master) eslint index.js
Cannot find module '@chrisblossom/eslint-config'
Referenced from: /Users/chris/github/eslint-issue-9746/packages/example/.eslintrc.js
Error: Cannot find module '@chrisblossom/eslint-config'
Referenced from: /Users/chris/github/eslint-issue-9746/packages/example/.eslintrc.js
    at ModuleResolver.resolve (/Users/chris/github/eslint-issue-9746/node_modules/eslint/lib/util/module-resolver.js:74:19)
    at resolve (/Users/chris/github/eslint-issue-9746/node_modules/eslint/lib/config/config-file.js:471:25)
    at load (/Users/chris/github/eslint-issue-9746/node_modules/eslint/lib/config/config-file.js:542:26)
    at configExtends.reduceRight (/Users/chris/github/eslint-issue-9746/node_modules/eslint/lib/config/config-file.js:421:36)
    at Array.reduceRight (<anonymous>)
    at applyExtends (/Users/chris/github/eslint-issue-9746/node_modules/eslint/lib/config/config-file.js:403:28)
    at loadFromDisk (/Users/chris/github/eslint-issue-9746/node_modules/eslint/lib/config/config-file.js:514:22)
    at Object.load (/Users/chris/github/eslint-issue-9746/node_modules/eslint/lib/config/config-file.js:550:20)
    at Config.getLocalConfigHierarchy (/Users/chris/github/eslint-issue-9746/node_modules/eslint/lib/config.js:228:44)
    at Config.getConfigHierarchy (/Users/chris/github/eslint-issue-9746/node_modules/eslint/lib/config.js:180:43)
@not-an-aardvark not-an-aardvark added the triage An ESLint team member will look at this issue soon label Dec 24, 2017
@platinumazure
Copy link
Member

I'm afraid I don't know much about lerna. Does this issue happen without lerna as well? I'm honestly not sure whether we're supposed to support lerna...

@eslint/eslint-team Can anyone weigh in here?

@not-an-aardvark
Copy link
Member

This is happening because lerna is installing eslint and your scoped config into two different locations. ESLint needs to be able to resolve your shareable config from the location of ESLint itself.

@davidpelayo
Copy link

davidpelayo commented Jan 10, 2018

@not-an-aardvark how this dependency installation should look like? I'm using lerna and happens to me the same, but if I bypass lerna and install dependencies normally, with npm, at the subpackage level, still getting the same error.

@gugiserman
Copy link

gugiserman commented Jan 10, 2018

I'm having the same problem and already tried to install locally at package level as well, got the same error output.

Edit: nevermind, @not-an-aardvark comment made sense to me after a sec of thought.
@davidpelayo I had eslint installed as a devDependency on the root package.json, the one that is on the same level as lerna.json. Then, I tried installing my shareable eslint-config as a devDependency on that same package.json (also had to install babel-eslint there) and it worked.

root package.json that works now:

{
  "babel-eslint": "^8.2.1",
  "eslint": "^4.15.0",
  "@pismo/eslint-config-bolt": "^0.0.1-5",
}

@davidpelayo
Copy link

davidpelayo commented Jan 10, 2018

@gugiserman the solution I came up with a few minutes ago was to avoid hoisting the eslint package by installing the dependencies as follow:

lerna bootstrap --hoist --npm-client=npm --nohoist=eslint

This resolved correctly the shared eslint config, although it takes more time at installing.

EDIT: I like more your solution, btw. At least from the performance point of view.

@ilyavolodin
Copy link
Member

Closing this issue as it seems like it's resolved.

@chrisblossom
Copy link
Author

chrisblossom commented Jan 10, 2018

I do not think this issue should be closed. I still think this is a bug and eslint should work as expected with npm link/lerna without additional configuration. I've updated the example repo with a babel example that works as expected.

@davidpelayo
Copy link

The example @chrisblossom is quite self-explanatory.

As specified here, it'd probably has to do with the module resolution, that doesn't follow the module resolution spec closely.

@ilyavolodin ilyavolodin reopened this Jan 11, 2018
@platinumazure platinumazure added question This issue asks a question about ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Jan 11, 2018
@not-an-aardvark
Copy link
Member

I think this is a general property of libraries that have peerDependencies, and isn't specific to ESLint. The current behavior is working as designed.

One might reasonably argue that the design should be changed, and I'd be open to considering that, provided that a clear proposal is presented which takes compatibility issues into account. (Keep in mind that config/plugin resolution has been discussed at length in issues like #3458 -- it might be worth reading some of that discussion if you're interested in learning more about the tradeoffs of ESLint's config resolution.) In any case, this behavior is not a bug.

@kaicataldo kaicataldo added the works as intended The behavior described in this issue is working correctly label Jan 23, 2018
@kaicataldo
Copy link
Member

It sounds like we should close this. If someone would like to make a new issue with a concrete proposal, please feel free!

@kaicataldo kaicataldo assigned kaicataldo and unassigned kaicataldo Jan 23, 2018
ngotchac added a commit to ngotchac/eslint that referenced this issue Jan 26, 2018
Fixes module resolution by splitting extraPath into all possible
paths, and by fixing the project path
@SamStonehouse
Copy link

SamStonehouse commented May 17, 2018

@gugiserman regarding your way of fixing this issue

I have a local package containing my eslint configuration, which is scoped, however when I try and install it in the root directory I get package not found on:

npm install --save-dev @example/eslint-config-example

Which isn't surprising as it's not published.

I've also just added it as a dependency in the package.json and attempted lerna bootstrap and lerna bootstrap --hoist which also don't seem to be able to find the module.

And I've tried simply using npm link to add it and that doesn't seem to work either.

In some instances it seems to work without the scoped namespace however I'd like to keep it and you have it in your example, are there any different steps you followed?

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Jul 24, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Jul 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion question This issue asks a question about ESLint works as intended The behavior described in this issue is working correctly
Projects
None yet
Development

No branches or pull requests

8 participants