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

Why isn't "eslint-import-resolver-node" listed in eslint-module-utils's peerDependencies? #828

Closed
onlywei opened this issue May 11, 2017 · 13 comments

Comments

@onlywei
Copy link

onlywei commented May 11, 2017

This plugin does not work with pnpm because it relies on NPM3's flat node_modules structure. Please see: pnpm/pnpm#739, where there is a reproducible repo.

@ljharb
Copy link
Member

ljharb commented May 11, 2017

First off, if something works with npm but not an npm clone, then the npm clone is broken.

It's listed as a dependency (https://github.com/benmosher/eslint-plugin-import/blob/master/package.json#L83), and this project works in npm 2 (which doesn't flatten like npm 3+ does), so I'm not sure why that would be an issue.

@ljharb
Copy link
Member

ljharb commented May 11, 2017

After rereading the linked issue, pnpm/pnpm#739 (comment) is about eslint-module-utils, not eslint-plugin-import - so you're asking about https://github.com/benmosher/eslint-plugin-import/blob/master/utils/package.json - which yes, should probably specify everything it requires as a dep or a peer+dev dep.

@benmosher
Copy link
Member

see: pnpm/pnpm#739 (comment)

@ljharb: after reading the linked comment, I'm curious what ideas you have to resolve this. I wouldn't be against just spec'ing npm install eslint-plugin-import eslint-import-resolver-node in the README if you think that wouldn't be too much drag on the user install experience.

I like that it works out of the box for most people, thus the current setup.

@ljharb
Copy link
Member

ljharb commented May 12, 2017

pnpm/pnpm#739 (comment) seems reasonable.

@stereokai
Copy link

So what's the verdict on this? :)

@ljharb
Copy link
Member

ljharb commented Jun 21, 2017

From pnpm/pnpm#739 (comment) via #828 (comment):

eslint-plugin-import should pass its __dirname to eslint-module-utils, and it should resolve from that dir. The resolve-from package might help.

PRs are welcome.

@zkochan
Copy link

zkochan commented Aug 1, 2017

An update on this problem. I did a pull request to resolve to add a preserveSymlinks option to it. Could this package use resolve with preserveSymlinks: false instead of the custom utils/resolve module?

@ljharb
Copy link
Member

ljharb commented Aug 1, 2017

Since I maintain both resolve and collaborate on eslint-plugin-import, I'm naturally in favor of that.

@stereokai
Copy link

@ljharb Sounds great!

@dandv
Copy link

dandv commented Dec 29, 2018

Any progress on this?

@benmosher benmosher changed the title Why isn't "eslint-import-resolver-node" listed in peerDependencies? Why isn't "eslint-import-resolver-node" listed in eslint-module-utils's peerDependencies? Jan 14, 2019
cdfa added a commit to cdfa/react-turn-reveal that referenced this issue Jan 22, 2019
@ljharb
Copy link
Member

ljharb commented Jan 4, 2020

This seems to have been fixed by #1591.

@ljharb ljharb closed this as completed Jan 4, 2020
@ljharb ljharb removed the resolver label Jan 6, 2020
@dsnoeck
Copy link

dsnoeck commented Feb 21, 2023

This issue is not resolved. Using pnpm: 7.27.1 & eslint-plugin-import: 2.26.0

@d00rsfan
Copy link

The same issue happens with npm i --install-strategy=linked that uses symbol links instead of copying packages to node_modules.
Fixed by npm i eslint-import-resolver-node --save-dev --install-strategy=linked

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