-
Notifications
You must be signed in to change notification settings - Fork 2
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: correctly require plugins with yarn+pnp (for all versions) #14
Conversation
@@ -2,7 +2,9 @@ | |||
|
|||
// helper for convenient testing | |||
exports.require = (name) => { | |||
const importedModule = require(require.resolve(name, {paths: [process.cwd()]})); // Require module using the absolute path | |||
const defaultPathsToResolve = require.resolve.paths(name); | |||
const absoluteModulePath = require.resolve(name, {paths: [process.cwd(), ...defaultPathsToResolve]}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found a bug which was reproduced for user with [email protected], plugin is not found in process.cwd
, but found in default folders from require.resolve.paths
. So I fixed it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Jfyi: after this one we would not need to create symlinks for testing-project/node_modules 🔥
@@ -2,7 +2,9 @@ | |||
|
|||
// helper for convenient testing | |||
exports.require = (name) => { | |||
const importedModule = require(require.resolve(name, {paths: [process.cwd()]})); // Require module using the absolute path | |||
const defaultPathsToResolve = require.resolve.paths(name); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It just returns default path the plugin can be resolved to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, and I can actually pass just an empty string there. But I decided to pass the package name just in case if something changed in next nodejs versions.
No description provided.