cli icon indicating copy to clipboard operation
cli copied to clipboard

[BUG] Preinstall script runs after installing dependencies

Open t1m0thyj opened this issue 3 years ago • 31 comments

Current Behavior:

In NPM v7 the preinstall script runs after dependencies are installed, which breaks backwards compatibility with NPM v6.

This may have been on purpose, but I do not see the behavior change listed under breaking changes. https://blog.npmjs.org/post/626173315965468672/npm-v7-series-beta-release-and-semver-major

Expected Behavior:

The preinstall script should run before dependencies are installed.

  • I'm aware that a similar issue (#2253) was closed with an explanation of "works as intended". I'm requesting for this behavior of NPM v7 to please be reconsidered.
  • This same issue appeared in npm@3 and was treated as a bug and fixed: https://github.com/npm/npm/issues/10379
  • The ability to hook into NPM before dependencies are installed may not be best practice but is sometimes required. This is the primary reason why I use preinstall scripts.
  • If preinstall scripts run after dependencies are installed, they seem identical to postinstall scripts and there is no longer a reason for me to use them.

Steps To Reproduce:

Clone this minimal sample repo which contains 2 packages:

  • package1 - This package is designed to fail if installed standalone. It expects the file "temp.txt" to exist that has been generated by a parent package before install.
  • package2 - This package installs package1 as a dependency, and has a preinstall script that creates the file "temp.txt".

Run cd package2 && npm install.

With NPM v6, the install succeeds because the preinstall script creates "temp.txt" before package1 is installed. With NPM v7, the install fails because the preinstall script creates "temp.txt" after package1 is installed.

Environment:

  • Ubuntu 18.04
  • Node: 15.8.0
  • npm: 7.5.3

t1m0thyj avatar Feb 09 '21 15:02 t1m0thyj

@t1m0thyj thanks for filing this. We have a consolidated issue tracking work/investigating this: https://github.com/npm/statusboard/issues/267

@wraithgar is looking into it.

darcyclarke avatar Feb 12 '21 19:02 darcyclarke

@darcyclarke @wraithgar Thanks for investigating this and updating the documentation. Can this issue please be reopened? I see the linked issue was closed, but the preinstall script is still implemented improperly in [email protected].

t1m0thyj avatar Mar 09 '21 18:03 t1m0thyj

today in npm 7 the preinstall script is working as intended for the majority of use cases. what you're asking for isn't unreasonable, but would create a new class of problem wherein a preinstall script cannot leverage any dependencies. resolving for that problem is why preinstall behaves the way it does.

i would suggest opening an rfc where we can start a discussion about potentially adding a new lifecycle script that runs before anything else. it would have to be a net new hook, though, maybe preunpack or something similar.

nlf avatar Mar 09 '21 19:03 nlf

By its name, "preinstall" should not have any dependencies available. imo that it runs after they're installed is just as much of a naming blunder as "prepublish" running on "not just before publish".

ljharb avatar Mar 09 '21 19:03 ljharb

you're not wrong, to be honest a lot of our lifecycle scripts are poorly named and don't run exactly when you expect. that's why there's both a prepublish and a prepublishOnly. we have a lot of technical debt here, but changing the expectations about how lifecycle scripts work is a glacier of a breaking change.

the best we can do right now is clearly document when scripts run and what's available to them

nlf avatar Mar 09 '21 19:03 nlf

a lot of the confusion here is that install is an event as well as a script, and in npm terms today pre and post mean before and after the script not the event. since we run the install script after we've already installed the package, that means that preinstall also runs after the package has been installed, immediately before the install script.

i agree it's not very intuitive, and we'd love to fix it so that the pre and post refer to the actual events and not the accompanying scripts, but it's a long road to get there.

adding these more intuitive hooks could potentially be done as a net new feature though, using a separate top level field in the package.json like "events", that would allow us to keep the current behavior of "scripts" while providing a better implementation elsewhere. eventually we deprecate the usage of "scripts" for anything other than npm run-script and "events" becomes the canonical place you tell us what to run during the various processes. that combined with a clear chart of the lifecycle that details what takes place before and after each event and where event scripts will be run would be a huge win

nlf avatar Mar 09 '21 20:03 nlf

I'm also noticing that for npm v6, I know how to run a script prior to installation of dependencies, and for npm v7, I don't. I'm not too attached to the specific methodology of using a preinstall hook, as long as I have some way of running a script before dependencies are installed. Can someone please let me know how to accomplish this for npm v7?

I've already looked here but couldn't find anything that seemed to help.

jfrancos avatar May 25 '21 23:05 jfrancos

I also think this should be treated as a bug as previous versions of npm and all versions of yarn support this.

For example we use preinstall script for generating workspaces from gRPC .proto files before workspaces and dependencies get evaluated. This works perfectly with yarn workspaces but doesn't with npm v7

davidgoitia avatar Jun 17 '21 11:06 davidgoitia

Agreed on treating this as a bug. Another example is users who use the preinstall hook to authenticate with private registries on their cloud platforms and/or any other registries with short lived tokens.

Azure: https://github.com/gsoft-inc/azure-devops-npm-auth AWS: https://aws.amazon.com/blogs/devops/publishing-private-npm-packages-aws-codeartifact/

ryansonshine avatar Jun 25 '21 14:06 ryansonshine

We were also using preinstall lifecycle script to set the registry and ensure we were logged in:

{
  "name": "private-npm",
  "version": "1.0.0",
  "description": "Private npm demo",
  "main": "index.js",
  "scripts": {
    "login": "npm config set registry https://registry.your-registry.npme.io/ && npm login --registry=https://registry.company-name.npme.io",
    "preinstall": "npm run login",
    "prepare": "npm run login"
  },
  "keywords": [],
  "author": "",
  "license": "ISC",
  "dependencies": {}
}

The change to npm v7 broke this functionality.

kmturley avatar Jun 25 '21 16:06 kmturley

Agreed on treating this as a bug. Another example is users who use the preinstall hook to authenticate with private registries on their cloud platforms and/or any other registries with short lived tokens.

Azure: https://github.com/gsoft-inc/azure-devops-npm-auth AWS: https://aws.amazon.com/blogs/devops/publishing-private-npm-packages-aws-codeartifact/

Facing the same issue right now with CodeArtifact

GregVes avatar Aug 25 '21 10:08 GregVes

Re-opening as per: https://github.com/npm/rfcs/blob/da73d3fd50a40e742f2d3755238ecdb8a6758d7f/meetings/2021-07-21.md#pr-403-rfc-add-preunpack-life-cycle-script---ryansonshine

darcyclarke avatar Aug 25 '21 15:08 darcyclarke

Hi, again not fixed on v7.x.x and v8.x.x ? Actually only v6.14.15 works well...

preinstall script is very needed before doing something why ? We have to prepare to install command and need packet from other source (apt for exemple)

IF we don't have the preinstall command, the result is ... install failed with errors

That's again to your works guys! I am not worried, you will fix the problem

@bugsounet

bugsounet avatar Nov 20 '21 21:11 bugsounet

Another use case: I added a preinstall script to our repo that prints a message informing people how to authenticate with our private npm registry by generating a github token and adding it to an .npmrc file. I was quite surprised when I found out the message never prints because install runs before preinstall, and all the user is left with is a generic "Incorrect or missing password" error.

bdcarr avatar Jan 04 '22 23:01 bdcarr

@bdcarr Unrelated to this issue but even once the order of preinstall is fixed, I'm not sure you'll be able to make the preinstall script work the way you're describing. Since npm 7, the default behavior now is to run lifecycle scripts as a background process, so their output won't be printed to the console (see doc for --foreground-scripts).

t1m0thyj avatar Jan 08 '22 19:01 t1m0thyj

@t1m0thyj thanks for letting me know 😭 , weirdly I'm still seeing the output with npm 8.3.0. I do note the docs say "build scripts for installed packages", so maybe it doesn't apply to the current project's scripts?

bdcarr avatar Jan 08 '22 23:01 bdcarr

related issue https://github.com/npm/cli/issues/4067

martin770 avatar Feb 17 '22 14:02 martin770

Hello,

Something new in order to solve preinstall script execution BEFORE any download?

Thanks

alliresan avatar Mar 14 '22 14:03 alliresan

It's 2022. Hasn't this basic problem been solved yet?

microkof avatar Mar 29 '22 09:03 microkof

which is more likely - that it's basic and nobody on the entire internet in all this time has been able to make a PR and/or convince the maintainers to fix it? or that it's not basic at all and there's very good reasons why this remains open?

ljharb avatar Mar 29 '22 14:03 ljharb

@yorickshan if there were, they’d be posted on the issue. Asking if there’s updates is spam.

ljharb avatar Apr 07 '22 03:04 ljharb

if there were, they’d be posted on the issue. Asking if there’s updates is spam.

Or it's a reminder/signal to the team that this type of update is important to users. As the old adage goes, "the squeaky wheel gets the oil."

dPowNextdoor avatar Apr 07 '22 03:04 dPowNextdoor

@dPowNextdoor that's what emoji reactions on the OP are for. Speaking as a package maintainer (not of npm, ofc), creating notification noise for everyone subscribed to this issue will literally never accelerate the speed of a fix.

ljharb avatar Apr 07 '22 04:04 ljharb

@dPowNextdoor so what can we do? Nothing is happening and this bug is pretty bad. I guess moving to yarn is the fastest solution.

sciutand avatar Apr 14 '22 02:04 sciutand

Any plans on this?

haskelcurry avatar Jun 08 '22 15:06 haskelcurry

:/

dec-land avatar Jun 16 '22 12:06 dec-land

Agreed on treating this as a bug. Another example is users who use the preinstall hook to authenticate with private registries on their cloud platforms and/or any other registries with short lived tokens.

Azure: https://github.com/gsoft-inc/azure-devops-npm-auth AWS: https://aws.amazon.com/blogs/devops/publishing-private-npm-packages-aws-codeartifact/

hi, this is exactly my situation. for those encountering this issue, i have a workaround, which hopefully alleviates some of the pressure on this issue!

credit goes to @bluwy for his find here: https://community.cloudflare.com/t/add-pnpm-to-pre-installed-cloudflare-pages-tools/288514/5 which references @yyx990803 's commit: https://github.com/vitejs/vite/blob/155fd11b783eddd33f0cd7e411eea40a3585217a/netlify.toml

i will explain cloudflare but surely azure / aws / netlify will work similarly

step 1: set an NPM_FLAGS: --version environment variable step 2: set your READ_ONLY_PAT / NPM_TOKEN environment variable step 3: set the build command (on cloudflare) to npm run cloudflare:ci step 4: package.json change below

package.json

"scripts": {
  ...
  "cloudflare:ci": "if [ -n \"$CF_PAGES\" ]; then echo \"//npm.pkg.github.com/:_authToken=${READ_ONLY_PAT}\" >> ./.npmrc && npm install; fi && npm run build",
  ...
}

ty, @bluwy & @yyx990803 for the nice find!

skilbjo avatar Aug 01 '22 06:08 skilbjo

@skilbjo I'm glad this solution worked for you, but it is not a proper workaround. It's just npm install with extra steps. You could put those extra steps in the scripts section of package.json, or a separate bash shell script, or whatever, but it doesn't address the original problem.

If you had packaged your library such that another program was using it as an external dependency. That external program would only ever call "npm install" from the top level and fail to install this dependency.

martin770 avatar Aug 01 '22 17:08 martin770

correct, it's not a proper workaround for this github issue, but it is a proper workaround for these JAMstack hosting providers (netlify, cloudflare pages, aws amplify, etc), where you have little/news control of the CI lifecycle, and due to a private registry, would otherwise fail to do a build in one of these systems, which i imagine a large lot of folks are here for

the key in the workaround i identified above is to set "NPM_OPTIONS=--version" as an env var, which overrides the default install behavior and allows you to defer the actual install behavior toward later, where you can set the npmrc file with the context needed beforehand

skilbjo avatar Aug 01 '22 17:08 skilbjo

This seems like a gigantic breaking change that is not even mentioned as a breaking change (or a change at all!) in v7, with no way to even work around it. I'm having a hard time understanding how a solution to this in some form is not an absolute top priority. At this point, having not been listed as a breaking change, this should absolutely be considered a bug.

I understand what's happening, but just take a step back and say this out loud: "We've changed the preinstall script to run after dependencies are installed and not listed it as a change."

That is mind-boggling.

bearfriend avatar Aug 03 '22 18:08 bearfriend