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: module resolution of linked plugins #352

Merged
merged 1 commit into from
Jan 28, 2022

Conversation

mdonnalley
Copy link
Contributor

  • Fixes module resolution for linked plugins
  • Fixes issue with command_not_found hook always throwing an error - even if the correct command was run by plugin-not-found

@W-10501665@

@mdonnalley mdonnalley self-assigned this Jan 28, 2022
@@ -83,7 +83,7 @@ async function findRoot(name: string | undefined, root: string) {
if (name) {
let pkgPath
try {
pkgPath = resolvePackage(name, {paths: [__dirname, root]})
pkgPath = resolvePackage(name, {paths: [root]})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mdonnalley why is __dirname being removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@peternhale two reasons: it's redundant and it was causing linked plugins to resolve to the version in node_modules - not your local version.

In the case of an unlinked plugin, we have
__dirname => /Users/mdonnalley/.nvm/versions/node/v17.1.0/lib/node_modules/@salesforce/cli/node_modules/@oclif/core/lib/config
root => /Users/mdonnalley/.nvm/versions/node/v17.1.0/lib/node_modules/@salesforce/cli

In this case, __dirname is redundant since it's already included by the root path

In the case of a linked plugin, we have

__dirname => /Users/mdonnalley/.nvm/versions/node/v17.1.0/lib/node_modules/@salesforce/cli/node_modules/@oclif/core/lib/config
root => /Users/mdonnalley/repos/oclif/plugin-version

In this case, __dirname was causing the resolution to return the node_modules version instead of the local version

In both cases, root is sufficient for resolving the plugin's location

@mdonnalley mdonnalley requested a review from peternhale January 28, 2022 21:20
@mdonnalley mdonnalley merged commit c7f5d34 into main Jan 28, 2022
@mdonnalley mdonnalley deleted the mdonnalley/module-resolution branch January 28, 2022 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants