Skip to content

Conversation

@charlespierce
Copy link
Contributor

Closes #1166

Info

  • Our interception of npm install --global (and other global npm commands) relies on us setting the prefix configuration via an environment variable.
  • However, if a user (or script) explicitly provides a prefix at the command line via --prefix, that will override our environment variable.
  • In those cases, our intercepted install will fail since the install will go to a different location than we expect.
  • At the same time, explicitly providing a prefix is an advanced option that likely means the user is doing something intentional and not trying to do a "standard" global install.
  • We shouldn't try to be smarter than the user, in these cases we should skip interception entirely to prevent unexpected errors.

Changes

  • Updated the has_global_flag helper to has_global_without_prefix to check for both the presence of --global and the absence of --prefix

Tested

  • Added a unit test that covers each of the global commands and confirms that the parser disables global interception when an explicit prefix is added.

Copy link
Contributor

@chriskrycho chriskrycho left a comment

Choose a reason for hiding this comment

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

One thought here: should we give (at least in "verbose" mode) some feedback about this? It seems like it'd be handy to let users know that they are doing something which circumvents our normal handling, and what (if any) consequences there might be.

@charlespierce
Copy link
Contributor Author

Good call! Added a verbose-mode message when we detect a global that we aren't going to intercept. I think for non-verbose mode it makes sense to omit the message, since we actually provide messages on the other side, when we meaningfully change the install process, so a message that we aren't changing things seems overly intrusive. But for verbose mode it could be useful for debugging.

@charlespierce charlespierce merged commit 6e26543 into volta-cli:main Feb 28, 2022
@charlespierce charlespierce deleted the npm_global_prefix branch February 28, 2022 19:25
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.

Bot Framework Composer unable to detect installed NodeJS

3 participants