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

fix: utils requires for resolver-node to be dependent #828 #1492

Open
wants to merge 1 commit into
base: master
from

Conversation

@anion155
Copy link

commented Oct 1, 2019

fix #828.
Tested with pnpm by linking resolver-node to module-utils's node_modules folder as if it was declared in peerDependencies, it worked. So I'm assuming just putting it in peerDependencies would work too.

@anion155 anion155 changed the title fix: utils requires for resolver-node to be dependent #828 fix: utils requires for resolver-node to be dependent https://github.com/benmosher/eslint-plugin-import/issues/828 Oct 1, 2019
@anion155 anion155 changed the title fix: utils requires for resolver-node to be dependent https://github.com/benmosher/eslint-plugin-import/issues/828 fix: utils requires for resolver-node to be dependent #828 Oct 1, 2019
@coveralls

This comment has been minimized.

Copy link

commented Oct 1, 2019

Coverage Status

Coverage increased (+0.8%) to 96.183% when pulling 747b0f2 on anion155:patch-1 into 5e143b2 on benmosher:master.

@coveralls

This comment has been minimized.

Copy link

commented Oct 1, 2019

Coverage Status

Coverage increased (+0.8%) to 96.116% when pulling 747b0f2 on anion155:patch-1 into 5e143b2 on benmosher:master.

6 similar comments
@coveralls

This comment has been minimized.

Copy link

commented Oct 1, 2019

Coverage Status

Coverage increased (+0.8%) to 96.116% when pulling 747b0f2 on anion155:patch-1 into 5e143b2 on benmosher:master.

@coveralls

This comment has been minimized.

Copy link

commented Oct 1, 2019

Coverage Status

Coverage increased (+0.8%) to 96.116% when pulling 747b0f2 on anion155:patch-1 into 5e143b2 on benmosher:master.

@coveralls

This comment has been minimized.

Copy link

commented Oct 1, 2019

Coverage Status

Coverage increased (+0.8%) to 96.116% when pulling 747b0f2 on anion155:patch-1 into 5e143b2 on benmosher:master.

@coveralls

This comment has been minimized.

Copy link

commented Oct 1, 2019

Coverage Status

Coverage increased (+0.8%) to 96.116% when pulling 747b0f2 on anion155:patch-1 into 5e143b2 on benmosher:master.

@coveralls

This comment has been minimized.

Copy link

commented Oct 1, 2019

Coverage Status

Coverage increased (+0.8%) to 96.116% when pulling 747b0f2 on anion155:patch-1 into 5e143b2 on benmosher:master.

@coveralls

This comment has been minimized.

Copy link

commented Oct 1, 2019

Coverage Status

Coverage increased (+0.8%) to 96.116% when pulling 747b0f2 on anion155:patch-1 into 5e143b2 on benmosher:master.

Copy link
Collaborator

left a comment

See #828 (comment) for the proper solution to #828.

@@ -27,5 +27,8 @@
"dependencies": {
"debug": "^2.6.8",
"pkg-dir": "^2.0.0"
},
"peerDependencies": {
"eslint-import-resolver-node": "^0.3.2"

This comment has been minimized.

Copy link
@ljharb

ljharb Oct 1, 2019

Collaborator

this definitely isn't the right solution; eslint-module-utils does not require the node resolver.

@anion155

This comment has been minimized.

Copy link
Author

commented Oct 15, 2019

@ljharb Sorry for long reaction. So then, add resolve package and wrap it to match utils/resolve's exports, would be a good approach?

@ljharb

This comment has been minimized.

Copy link
Collaborator

commented Oct 16, 2019

I'm still not sure what the underlying issue is; but anything that works with npm but doesn't work with an alternative client, is broken in the alternative client by definition.

#828 (comment) is the expected solution here. Want to update the PR to do only that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.