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: use internal npm for pkg checks #595

Merged
merged 4 commits into from
Apr 27, 2023
Merged

fix: use internal npm for pkg checks #595

merged 4 commits into from
Apr 27, 2023

Conversation

cristiand391
Copy link
Member

When installing a plugin using an "unfriendly" plugin name, plugins install needs to call npm to verify that the pkg exists in the registry. Until now it's been doing that by executing npm show <pkg-name> dist-tags, which works fine unless you don't have npm installed globally on your system.

This PR updates this check to use the npm version pinned in and shipped with plugin-plugins, this way the user doesn't need to install npm and also helps avoid weird issues if a user has an old npm version or a new breaking change is released.

Testing notes:

  1. Checkout this PR locally
  2. Link plugin (plugins link .)
  3. Try installing a plugin using an "unfriendly" name and verify the debug output to see which node/npm is being used.

Suggestion
set DEBUG='@oclif/plugins' when testing to see additional debug output.
Screenshot 2023-04-27 at 11 22 37

Possible scenarios

  1. Install CLI via mac/win installer or tarballs
    Installers/tarballs include a node executable and plugins install <unfriendly-name will perform the call to the pinned npm version in plugin-plugins/package.json using the include node binary.

  2. Install CLI via npm (so user has node installed)
    plugins install <unfriendly-name> will use the global node binary to call the pinned npm version in plugin-plugins/package.json

It doesn't matter if the user has npm installed globally, plugin-plugins should always use the pinned version included by the plugin.

What's an "unfriendly" pkg name?

If you define an npm pkg scope in your CLI (pjson.oclif.scope), you can use a shorter name when installing plugins published under that scope.
https://github.com/oclif/plugin-plugins/blob/main/src/plugins.ts#L258

Example:
The Salesforce CLI defines salesforce as an npm scope here:
https://github.com/salesforcecli/sfdx-cli/blob/main/package.json#L27

To install @salesforce/plugin-data you can run:
sfdx plugins install data and plugin-plugins resolves the plugin name.

@W-13018874@

.do(output => expect(output.stdout).to.equal('No plugins installed.\n'))
.it('installs via an alias')
if (platform() !== 'win32') {
test
Copy link
Member Author

Choose a reason for hiding this comment

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

these fail on windows with the err "CannotFindNodeExecutable: Cannot locate node executable" due to process.env.PATH being set to an empty string.

shelljs.which looks for dirs in process.env.PATH but (I think) fancy-test is modifying.
https://github.com/shelljs/shelljs/blob/master/src/which.js#L58

tried passing ENV as process.env.PATH but the tests are executed through yarn which adds a few paths at the start of the array and end up messing the test.
https://github.com/oclif/fancy-test#environment-variables

@WillieRuemmele
Copy link

###QA Notes

✅ : suggested QA above

 ➜  DEBUG='@oclif/plugins' sfdx plugins install 'trustad'
  @oclif/plugins checking registry for expanded package name @salesforce/plugin-trustad +0ms
  @oclif/plugins Using node executable located at: /Users/william.ruemmele/.nvm/versions/node/v18.7.0/bin/node +3ms
  @oclif/plugins Using npm executable located at: /Users/william.ruemmele/projects/oclif/plugin-plugins/node_modules/npm/bin/npm-cli.js +0ms

✅ : uninstall node, install sfdx via installers, get bundled node

 ➜  node -v
zsh: command not found: node
➜  plugin-plugins git:(cd/global-npm) 
 ➜  DEBUG='@oclif/plugins' sfdx plugins install 'trustad'
  @oclif/plugins checking registry for expanded package name @salesforce/plugin-trustad +0ms
  @oclif/plugins Using node executable located at: /usr/local/lib/sfdx/bin/node +2ms
  @oclif/plugins Using npm executable located at: /Users/william.ruemmele/projects/oclif/plugin-plugins/node_modules/npm/bin/npm-cli.js +1ms

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