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

Wrong ESLint plugin loaded (works w/ NPM) #3332

Closed
cmoesel opened this issue May 5, 2017 · 11 comments
Closed

Wrong ESLint plugin loaded (works w/ NPM) #3332

cmoesel opened this issue May 5, 2017 · 11 comments
Labels

Comments

@cmoesel
Copy link

cmoesel commented May 5, 2017

Do you want to request a feature or report a bug?
bug

What is the current behavior?
Our project uses react-scripts and eslint-config-airbnb devDependencies. react-scripts declares eslint-plugin-import "2.0.1" as a dependency. eslint-config-airbnb declares eslint-plugin-import "^2.2.0" as a peer dependency. We've added eslint-plugin-import "^2.2.0" as our own devDependency to satisfy the peer dependency.

When we run eslint, we get many of these errors: error Definition for rule 'import/no-named-default' was not found. Since this rule was added in eslint-plugin-import v2.2.0, my best guess is that eslint is looking for the rule in the eslint-plugin-import v2.0.1 transitive dependency.

I'm suspecting the issue may be with yarn because if we use npm install instead of yarn install, then it works as expected and we do not get the previous errors when running eslint. That said, I'm not confident -- so feel free to disagree.

If the current behavior is a bug, please provide the steps to reproduce.
See above.

What is the expected behavior?
ESLint should find the rule in eslint_plugin_import v2.2.0 rather than v2.0.1"

Please mention your node.js, yarn and operating system version.
Node 6.6.0, Yarn 0.23.4, Mac OS X 10.12.4

@cmoesel
Copy link
Author

cmoesel commented May 5, 2017

As noted above, I filed a similar issue against eslint in case the problem is on their end. Hoping someone from one of the teams might have an idea what is going on.

@mbifulco
Copy link

mbifulco commented May 10, 2017

Ditto - I'm in the same boat as @cmoesel on my project.

@cmoesel
Copy link
Author

cmoesel commented May 10, 2017

If anyone wants to see this in action, I've created a simple project that reproduces the problem:

https://github.com/cmoesel/eslint-8547

In fact, it's even worse than my real project since there are also some react rules that can't be found (but again, it all works fine if you install with npm instead of yarn).

@bestander
Copy link
Member

Thanks for a report.
Some more help with investigating what versions are installed in the tree and what is the expected structure will be welcome

@cmoesel
Copy link
Author

cmoesel commented May 12, 2017

Thanks @bestander. I've updated my example project with the data for your (and others') convenience.

NPM

You will find the results of npm ls and the resulting node_modules here:
https://github.com/cmoesel/eslint-8547/tree/master/npm_data

In the npm ls output, the eslint-plugin-import package shows up at the root w/ v2.2.0:

├─┬ eslint-plugin-import@2.2.0
│ ├── builtin-modules@1.1.1
│ ├── contains-path@0.1.0
│ ├── doctrine@1.5.0
│ ├─┬ eslint-import-resolver-node@0.2.3
│ │ └─┬ resolve@1.3.3
│ │   └── path-parse@1.0.5
│ ├─┬ eslint-module-utils@2.0.0
│ │ ├─┬ debug@2.2.0
│ │ │ └── ms@0.7.1
│ │ └── pkg-dir@1.0.0
│ ├─┬ has@1.0.1
│ │ └── function-bind@1.1.0
│ ├── lodash.cond@4.5.2
│ ├─┬ minimatch@3.0.4
│ │ └─┬ brace-expansion@1.1.7
│ │   ├── balanced-match@0.4.2
│ │   └── concat-map@0.0.1
│ └─┬ pkg-up@1.0.0
│   └─┬ find-up@1.1.2
│     ├── path-exists@2.1.0
│     └─┬ pinkie-promise@2.0.1
│       └── pinkie@2.0.4

and under react-scripts@0.9.5 w/ v 2.0.1:

└─┬ react-scripts@0.9.5
  ├── (bunch of other packages are here...)
  ├─┬ eslint-plugin-import@2.0.1
  │ ├── doctrine@1.3.0
  │ └─┬ eslint-module-utils@1.0.0
  │   └─┬ debug@2.2.0
  │     └── ms@0.7.1

Yarn

You will find the results of yarn list and the resulting node_modules here:
https://github.com/cmoesel/eslint-8547/tree/master/yarn_data

In the yarn list output, the eslint-plugin-import package shows up at the root w/ v2.2.0:

├─ eslint-plugin-import@2.2.0
│  ├─ builtin-modules@^1.1.1
│  ├─ contains-path@^0.1.0
│  ├─ debug@^2.2.0
│  ├─ doctrine@1.5.0
│  ├─ eslint-import-resolver-node@^0.2.0
│  ├─ eslint-module-utils@^2.0.0
│  ├─ has@^1.0.1
│  ├─ lodash.cond@^4.3.0
│  ├─ minimatch@^3.0.3
│  └─ pkg-up@^1.0.0

and twice (?) under react-scripts@0.9.5 w/ v2.0.1:

├─ react-scripts@0.9.5
│  ├─ (bunch of other packages are here...)
│  ├─ eslint-plugin-import@2.0.1
│  ├─ eslint-plugin-import@2.0.1
│  │  ├─ builtin-modules@^1.1.1
│  │  ├─ contains-path@^0.1.0
│  │  ├─ debug@^2.2.0
│  │  ├─ doctrine@1.3.x
│  │  ├─ eslint-import-resolver-node@^0.2.0
│  │  ├─ eslint-module-utils@^1.0.0
│  │  ├─ has@^1.0.1
│  │  ├─ lodash.cond@^4.3.0
│  │  ├─ minimatch@^3.0.3
│  │  └─ pkg-up@^1.0.0

@cmoesel
Copy link
Author

cmoesel commented May 12, 2017

Forgot to mention the versions of tools I'm working with:

  • Operating System: macOS Sierra 10.12.4
  • Node: v6.6.0
  • NPM: 3.10.3
  • Yarn: 0.23.4

@bestander
Copy link
Member

So far looks like Yarn is behaving correctly.

The double output of eslint-plugin-import@2.0.1 is weird but that is ok.

I ran yarn check --verify-tree to see if install created a correct folder structure from Node.js module resolution point of view.
It passes fine.

And according to your findings both npm and yarn give you correct

├─ eslint-plugin-import@2.2.0
├─ react-scripts@0.9.5
│  ├─ eslint-plugin-import@2.0.1

I checked the package.json files of all 3 of them and it is all right.

I don't think the issue is here.
Any other ideas?

@cmoesel
Copy link
Author

cmoesel commented May 15, 2017

Thank you for looking into this, @bestander. It sounds like both yarn and npm are technically doing the right things from a dependency installation/resolution point of view -- so any difference in eslint behavior is likely due to some variance that is not technically illegal but still somehow affects eslint plugin resolution.

There have been a couple of "me too"s over on the eslint side (see eslint#8547), so I'm hoping that maybe someone over there will be able to take a look and shed some light on this.

Thanks again for taking a look. I appreciate it.

@bestander
Copy link
Member

bestander commented May 15, 2017 via email

@cmoesel
Copy link
Author

cmoesel commented May 30, 2017

See #3535 for what looks like it may be a smoking gun.

@bestander
Copy link
Member

This is fixed via #3545

cmoesel added a commit to AHRQ-CDS/AHRQ-CDS-Connect-Authoring-Tool that referenced this issue Feb 21, 2018
When using yarn, eslint is loading an older version of eslint-plugin-import, which causes problems in the current configuration.  This commit fixes the problems by disabling the problem rule and adjusting the problem glob.

See: yarnpkg/yarn#3332

And: import-js/eslint-plugin-import#602
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants