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

Always use globally installed npm binary #410

Closed
wants to merge 3 commits into from
Closed

Always use globally installed npm binary #410

wants to merge 3 commits into from

Conversation

Kerumen
Copy link

@Kerumen Kerumen commented May 21, 2019

I just had an issue because of 2 different npm binaries installed on my machine. One was up-to-date and the other was very old. I figured out that the node@10 I've installed with brew has a "local" npm binary attached to it.

For execa, preferLocal is set to true by default hence it will try to load local binaries first.

I think it's - for npm at least - a better idea to always load the global binary instead of any local (outdated) ones.

@Kerumen Kerumen changed the title Configure preferLocal to false for npm bin Always prefer global npm binary May 21, 2019
@Kerumen Kerumen changed the title Always prefer global npm binary Always use globally installed npm binary May 21, 2019
@saulimus
Copy link

saulimus commented Jun 5, 2019

I applied this commit manually to my installation.
This made the version check to pass, but during publish it np still used the local npm (5.8.0):

$ np 1.0.0

  ✔ Prerequisite check
  ✔ Git
  ↓ Cleanup [skipped]
  ✖ Installing dependencies using npm
    → npm ERR!     <redacted>/.npm/_logs/2019-06-05T12_32_19_831Z-debug.log
    Running tests using npm
    Bumping version using npm
    Publishing package using npm
    Enabling two-factor authentication
    Pushing tags

✖ Command failed: npm ci
npm WARN npm npm does not support Node.js v10.16.0
npm WARN npm You should probably upgrade to a newer version of node as we
npm WARN npm can't make any promises that npm will work with this version.
npm WARN npm Supported releases of Node.js are the latest release of 4, 6, 7, 8, 9.
npm WARN npm You can find the latest version at https://nodejs.org/
npm WARN prepare removing existing node_modules/ before installation
npm ERR! code MODULE_NOT_FOUND
npm ERR! Cannot find module 'validate-npm-package-name'

npm ERR! A complete log of this run can be found in:
npm ERR!     <redacted>/.npm/_logs/2019-06-05T12_32_19_831Z-debug.log

Debug log:

0 info it worked if it ends with ok
1 warn npm npm does not support Node.js v10.16.0
2 warn npm You should probably upgrade to a newer version of node as we
3 warn npm can't make any promises that npm will work with this version.
4 warn npm Supported releases of Node.js are the latest release of 4, 6, 7, 8, 9.
5 warn npm You can find the latest version at https://nodejs.org/
6 verbose cli [ '<redacted>/.nvm/versions/node/v10.16.0/bin/node',
6 verbose cli   '<redacted>/node_modules/.bin/npm',
6 verbose cli   'ci' ]
7 info using npm@5.8.0
8 info using node@v10.16.0
9 verbose npm-session 26c3f61fdf972719
10 info prepare initializing installer
11 verbose prepare starting workers
12 verbose prepare installation prefix: <redacted>
13 verbose prepare using package-lock.json
14 warn prepare removing existing node_modules/ before installation
15 verbose checkLock verifying package-lock data
16 verbose teardown shutting down workers.
17 info teardown Done in 0.018s
18 verbose stack Error: Cannot find module 'validate-npm-package-name'
18 verbose stack     at Function.Module._resolveFilename (internal/modules/cjs/loader.js:636:15)
18 verbose stack     at Function.Module._load (internal/modules/cjs/loader.js:562:25)
18 verbose stack     at Module.require (internal/modules/cjs/loader.js:690:17)
18 verbose stack     at require (internal/modules/cjs/helpers.js:25:18)
18 verbose stack     at Result.setName (/<redacted>/node_modules/npm/node_modules/libcipm/node_modules/lock-verify/node_modules/npm-package-arg/npa.js:104:51)
18 verbose stack     at new Result (/<redacted>/node_modules/npm/node_modules/libcipm/node_modules/lock-verify/node_modules/npm-package-arg/npa.js:96:23)
18 verbose stack     at Function.resolve (/<redacted>/node_modules/npm/node_modules/libcipm/node_modules/lock-verify/node_modules/npm-package-arg/npa.js:45:15)
18 verbose stack     at Object.keys.forEach.name (/<redacted>/node_modules/npm/node_modules/libcipm/node_modules/lock-verify/index.js:26:26)
18 verbose stack     at Array.forEach (<anonymous>)
18 verbose stack     at Promise.all.then.result (/<redacted>/node_modules/npm/node_modules/libcipm/node_modules/lock-verify/index.js:25:25)
(<redacted>/node_modules/npm/node_modules/libcipm/node_modules/lock-verify/i$
19 verbose cwd <redacted>
20 verbose Linux 4.4.0-17134-Microsoft
21 verbose argv "<redacted>/.nvm/versions/node/v10.16.0/bin/node" "<redacted>/node_modules/.bin/npm" "ci"
22 verbose node v10.16.0
23 verbose npm  v5.8.0
24 error code MODULE_NOT_FOUND
25 error Cannot find module 'validate-npm-package-name'
26 verbose exit [ 1, true ]

@saulimus
Copy link

saulimus commented Jun 5, 2019

The previous attempt (#410 (comment)) nuked node_modules/ and the local npm binary as well.
Thus retrying the "np" command it succeeded until it tried enabling 2fa.
This would indicate that at some point np switches to use the local node_modules/.bin/npm installed during the "Installing dependencies using npm" phase.

$ np 1.0.0

  ✔ Prerequisite check
  ✔ Git
  ↓ Cleanup [skipped]
  ✔ Installing dependencies using npm
  ✔ Running tests using npm
  ✔ Bumping version using npm
  ✔ Publishing package using npm
  ✖ Enabling two-factor authentication
    → npm ERR! npm access edit [<package>]
    Pushing tags

✖ Command failed: npm access 2fa-required @<redacted>/<redacted>
npm WARN npm npm does not support Node.js v10.16.0
npm WARN npm You should probably upgrade to a newer version of node as we
npm WARN npm can't make any promises that npm will work with this version.
npm WARN npm Supported releases of Node.js are the latest release of 4, 6, 7, 8, 9.
npm WARN npm You can find the latest version at https://nodejs.org/
npm ERR! access subcommand must be one of public, restricted, grant, revoke, ls-packages, ls-collaborators, edit
npm ERR!
npm ERR! Usage:
npm ERR! npm access public [<package>]
npm ERR! npm access restricted [<package>]
npm ERR! npm access grant <read-only|read-write> <scope:team> [<package>]
npm ERR! npm access revoke <scope:team> [<package>]
npm ERR! npm access ls-packages [<user>|<scope>|<scope:team>]
npm ERR! npm access ls-collaborators [<package> [<user>]]
npm ERR! npm access edit [<package>]

It seems 2fa-required isn't available in npm 5.x.x

@itaisteinherz
Copy link
Collaborator

Another option: when np is used locally, the local binaries of npm and yarn will be used, and when used, the global binaries will be used.

@sindresorhus
Copy link
Owner

Another option: when np is used locally, the local binaries of npm and yarn will be used, and when used, the global binaries will be used.

That would be weird. Better to always use the global ones.

@itaisteinherz
Copy link
Collaborator

@Kerumen Are you interested in finishing this? The only issue that's currently blocking this PR is #410 (comment).

@Kerumen
Copy link
Author

Kerumen commented Jun 24, 2019

@itaisteinherz I rebased and added preferLocal everywhere yarn/npm was used. Tell me if it's alright, sorry for the delay 🙏🏻

@sindresorhus
Copy link
Owner

@itaisteinherz Don't merge this yet. I'm working on something.

@itaisteinherz
Copy link
Collaborator

itaisteinherz commented Jun 24, 2019

@Kerumen No worries 👌🏻
There are still places where the local binaries of npm and yarn are being used though (e.g. here and here).

@Kerumen
Copy link
Author

Kerumen commented Jun 24, 2019

@itaisteinherz I fixed all the missing spots. 😅

@sindresorhus
Copy link
Owner

This is no longer needed as preferLocal is false by default in execa v2. I just upgraded the version.

@itaisteinherz
Copy link
Collaborator

itaisteinherz commented Jun 28, 2019

In any case, @Kerumen thanks for your contribution :)

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.

4 participants