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

[BUG] npm install --production runs scripts #4027

Open
2 tasks done
mxschmitt opened this issue Nov 10, 2021 · 12 comments
Open
2 tasks done

[BUG] npm install --production runs scripts #4027

mxschmitt opened this issue Nov 10, 2021 · 12 comments
Labels
Documentation documentation related issue Priority 2 secondary priority issue Release 8.x work is associated with a specific npm 8 release

Comments

@mxschmitt
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

This issue exists in the latest npm version

  • I am using the latest npm

Current Behavior

When using npm install --production it executes the prepare scripts in version 7+. In version 6 it didn't execute them.

Workaround: npm install --production --ignore-scripts

(Didn't see that the behaviour has changed in the changelog or in the docs, thats why I'm filing it. The workaround is working for us)

Expected Behavior

The prepare script does not get executed.

Steps To Reproduce

  1. Create this package.json
{
  "name": "npm-install-production-prepare",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "prepare": "echo \"PREPARE GETS EXECUTED\""
  },
  "keywords": [],
  "author": "",
  "license": "ISC"
}
  1. npm install --production
  2. Observe that the prepare script gets executed

Environment

  • npm: 7.0.0+
  • Node: 16
  • OS: macOS
  • platform: Macbook Pro
@mxschmitt mxschmitt added Bug thing that needs fixing Needs Triage needs review for next steps Release 8.x work is associated with a specific npm 8 release labels Nov 10, 2021
@mbtts
Copy link

mbtts commented Nov 11, 2021

I think it might also be running build (or trying to) as well (see #4003).

The behaviour is certainly different to version 6 and (as far as I can see) not clearly documented.

@lucaswerkmeister
Copy link
Contributor

lucaswerkmeister commented Dec 3, 2021

I agree that this behavior doesn’t match what the documentation says:

prepare (since [email protected])

  • Runs on local npm install without any arguments
  • As of npm@7 these scripts run in the background. To see the output, run with: --foreground-scripts.

It’s also unfortunate, because under the previous behavior, it made sense to use the prepare script for steps that only make sense in a plain npm install, and put its dependencies in devDependencies; now that npm runs it on npm install --production too, it’ll crash because those dependencies can’t be found.

wmfgerrit pushed a commit to wikimedia/wikibase-termbox that referenced this issue Dec 6, 2021
Node 10 support in the ecosystem is fading, and npm apparently ignores
the engines package.json field, making it increasingly tedious to pick
Node 10-compatible versions when upgrading packages.

This also makes us use npm v7, which introduces an undocumented behavior
change (reported as a bug at [1]): the prepare script now runs during
`npm install --only=production` as well, where it will fail because its
dependencies are only devDependencies. We don’t actually want this
script to run during the production-only install in the release
pipeline, so wrap the imports in try/catch and exit with a warning if
they fail.

Also, update service-runner to a version that uses wikimedia-kad-fork
instead of kad; the previous version was causing problems in some cases
(“cannot find module 'merge'”, see P18027).

[1]: npm/cli#4027

Bug: T297006
Change-Id: I845199975455b6426efacbcf33f1ac0c8563a107
@damianprzygodzki
Copy link

Agree. After update to npm 8 (from 6) we've observed unexpected run of prepare lifeCycle script before npm install --production. It can even brake building an image with app. Documentation doesn't mention that fact.

@jeongmincha
Copy link

Is there any progress on this issue?

nxhack added a commit to nxhack/packages that referenced this issue Apr 6, 2022
With the upgrade of node.js to version 16, the npm version will also change to version 8.
This fix is to support npm@8. npm@6 can also build without problems.

The modification method is different from other node modules.
The reason is due to the npm@8 issue.
npm/cli#4027

Signed-off-by: Hirokazu MORIKAWA <[email protected]>
neheb pushed a commit to openwrt/packages that referenced this issue Apr 16, 2022
With the upgrade of node.js to version 16, the npm version will also change to version 8.
This fix is to support npm@8. npm@6 can also build without problems.

The modification method is different from other node modules.
The reason is due to the npm@8 issue.
npm/cli#4027

Signed-off-by: Hirokazu MORIKAWA <[email protected]>
Beginner-Go pushed a commit to coolsnowwolf/packages that referenced this issue Apr 22, 2022
With the upgrade of node.js to version 16, the npm version will also change to version 8.
This fix is to support npm@8. npm@6 can also build without problems.

The modification method is different from other node modules.
The reason is due to the npm@8 issue.
npm/cli#4027

Signed-off-by: Hirokazu MORIKAWA <[email protected]>
@wraithgar wraithgar added Documentation documentation related issue Priority 2 secondary priority issue and removed Bug thing that needs fixing Needs Triage needs review for next steps labels May 4, 2022
nxhack added a commit to nxhack/packages that referenced this issue May 22, 2022
With the upgrade of node.js to version 16, the npm version will also change to version 8.
This fix is to support npm@8. npm@6 can also build without problems.

The modification method is different from other node modules.
The reason is due to the npm@8 issue.
npm/cli#4027

Signed-off-by: Hirokazu MORIKAWA <[email protected]>
(cherry picked from commit eee26db)
BKPepe pushed a commit to openwrt/packages that referenced this issue May 22, 2022
With the upgrade of node.js to version 16, the npm version will also change to version 8.
This fix is to support npm@8. npm@6 can also build without problems.

The modification method is different from other node modules.
The reason is due to the npm@8 issue.
npm/cli#4027

Signed-off-by: Hirokazu MORIKAWA <[email protected]>
(cherry picked from commit eee26db)
Beginner-Go added a commit to coolsnowwolf/packages that referenced this issue May 23, 2022
* node-homebridge: Support for npm@8

With the upgrade of node.js to version 16, the npm version will also change to version 8.
This fix is to support npm@8. npm@6 can also build without problems.

Signed-off-by: Hirokazu MORIKAWA <[email protected]>

* Revert "node-yarn: bump to v1.22.17"

This reverts commit 38b18a5.

* node-yarn: Support for npm@8

With the upgrade of node.js to version 16, the npm version will also change to version 8.
This fix is to support npm@8. npm@6 can also build without problems.

Signed-off-by: Hirokazu MORIKAWA <[email protected]>

* node-hid: Support for npm@8

With the upgrade of node.js to version 16, the npm version will also change to version 8.
This fix is to support npm@8. npm@6 can also build without problems.

Signed-off-by: Hirokazu MORIKAWA <[email protected]>

* javascript-obfuscator: bump to 2.19.0 and switch to autorelease

New version of package node-javascript-obfuscator.

Signed-off-by: Zbyněk Kocur <[email protected]>

* node-javascript-obfuscator: Support for npm@8

With the upgrade of node.js to version 16, the npm version will also change to version 8.
This fix is to support npm@8. npm@6 can also build without problems.

The modification method is different from other node modules.
The reason is due to the npm@8 issue.
npm/cli#4027

Signed-off-by: Hirokazu MORIKAWA <[email protected]>

* node-serialport: Support for npm@8

With the upgrade of node.js to version 16, the npm version will also change to version 8.
This fix is to support npm@8. npm@6 can also build without problems.

Signed-off-by: Hirokazu MORIKAWA <[email protected]>

* node-cylon: Support for npm@8

With the upgrade of node.js to version 16, the npm version will also change to version 8.
This fix is to support npm@8. npm@6 can also build without problems.

Signed-off-by: Hirokazu MORIKAWA <[email protected]>

* node-arduino-firmata: Support for npm@8

With the upgrade of node.js to version 16, the npm version will also change to version 8.
This fix is to support npm@8. npm@6 can also build without problems.

Signed-off-by: Hirokazu MORIKAWA <[email protected]>

* node-serialport-bindings: Support for npm@8

With the upgrade of node.js to version 16, the npm version will also change to version 8.
This fix is to support npm@8. npm@6 can also build without problems.

Signed-off-by: Hirokazu MORIKAWA <[email protected]>

Co-authored-by: Hirokazu MORIKAWA <[email protected]>
Co-authored-by: Zbyněk Kocur <[email protected]>
@pmoleri
Copy link

pmoleri commented Aug 15, 2022

Given that the documentation states:

NOTE: If a package being installed through git contains a prepare script, its dependencies and devDependencies will be installed, and the prepare script will be run, before the package is packaged and installed.

Looks like npm acknowledges that prepare requires the devDependencies.
I think when running --production and there's a prepare script, it should:

  • Not run prepare (older behavior), and state it in the docs.
  • or ignore the --production flag
  • or bail out

In any of the cases, it would make sense to print a warning.

@pkellz
Copy link

pkellz commented Jan 20, 2023

Any update on this issue?

SibrenVasse pushed a commit to SibrenVasse/packages that referenced this issue Feb 26, 2023
With the upgrade of node.js to version 16, the npm version will also change to version 8.
This fix is to support npm@8. npm@6 can also build without problems.

The modification method is different from other node modules.
The reason is due to the npm@8 issue.
npm/cli#4027

Signed-off-by: Hirokazu MORIKAWA <[email protected]>
(cherry picked from commit eee26db)
@timakhalaya
Copy link

any update?

as a workaround set prepare script to "" in RUN stage of Docker ???

@jorenbroekema
Copy link

jorenbroekema commented Jan 26, 2024

For what it's worth, I've just started using postinstall/prepare with a NodeJS script that checks process.env.NODE_ENV before running dev-only postinstall scripts:

// only run these scripts if we install dev deps & npm install was ran inside style-dictionary package itself
// since our consumers don't ever need to run these themselves.
if (process.env.NODE_ENV !== 'production' && process.env.PWD === process.env.INIT_CWD) {
  const husky = require('husky');
  husky.install();
  // we're only patching dev deps
  require('patch-package');
}

I didn't confirm this but it seems the 2nd check in the if condition may be redundant if you use prepare over postinstall, although prepare seems to run in different lifecycle hooks as well so for me this wasn't really the right hook to use.

Note: On Windows, NODE_ENV and PWD are both undefined...... :(

@robross0606
Copy link

Why are we now on npm 10 and this bug still exists?

ineedfat pushed a commit to ineedfat/rockchip_rk3568_openwrt_packages that referenced this issue Jun 7, 2024
* node-homebridge: Support for npm@8

With the upgrade of node.js to version 16, the npm version will also change to version 8.
This fix is to support npm@8. npm@6 can also build without problems.

Signed-off-by: Hirokazu MORIKAWA <[email protected]>

* Revert "node-yarn: bump to v1.22.17"

This reverts commit 38b18a5.

* node-yarn: Support for npm@8

With the upgrade of node.js to version 16, the npm version will also change to version 8.
This fix is to support npm@8. npm@6 can also build without problems.

Signed-off-by: Hirokazu MORIKAWA <[email protected]>

* node-hid: Support for npm@8

With the upgrade of node.js to version 16, the npm version will also change to version 8.
This fix is to support npm@8. npm@6 can also build without problems.

Signed-off-by: Hirokazu MORIKAWA <[email protected]>

* javascript-obfuscator: bump to 2.19.0 and switch to autorelease

New version of package node-javascript-obfuscator.

Signed-off-by: Zbyněk Kocur <[email protected]>

* node-javascript-obfuscator: Support for npm@8

With the upgrade of node.js to version 16, the npm version will also change to version 8.
This fix is to support npm@8. npm@6 can also build without problems.

The modification method is different from other node modules.
The reason is due to the npm@8 issue.
npm/cli#4027

Signed-off-by: Hirokazu MORIKAWA <[email protected]>

* node-serialport: Support for npm@8

With the upgrade of node.js to version 16, the npm version will also change to version 8.
This fix is to support npm@8. npm@6 can also build without problems.

Signed-off-by: Hirokazu MORIKAWA <[email protected]>

* node-cylon: Support for npm@8

With the upgrade of node.js to version 16, the npm version will also change to version 8.
This fix is to support npm@8. npm@6 can also build without problems.

Signed-off-by: Hirokazu MORIKAWA <[email protected]>

* node-arduino-firmata: Support for npm@8

With the upgrade of node.js to version 16, the npm version will also change to version 8.
This fix is to support npm@8. npm@6 can also build without problems.

Signed-off-by: Hirokazu MORIKAWA <[email protected]>

* node-serialport-bindings: Support for npm@8

With the upgrade of node.js to version 16, the npm version will also change to version 8.
This fix is to support npm@8. npm@6 can also build without problems.

Signed-off-by: Hirokazu MORIKAWA <[email protected]>

Co-authored-by: Hirokazu MORIKAWA <[email protected]>
Co-authored-by: Zbyněk Kocur <[email protected]>
@tannerstern
Copy link

tannerstern commented Aug 15, 2024

I lost a few hours on this issue several weeks ago. I can't speak to what the behavior should be (at the moment I'm using a tiny shell script that checks for NODE_ENV and runs accordingly). I do agree that the documentation is misleading: I took it on face value from the v10 scripts documentation that

  • Runs on local npm install without any arguments

meant that including the --production flag would not trigger my prepare script.

In my use case, npm install --production is being called from within a Google Cloud Functions deployment, over which I have no control or ability to add --ignore-scripts.

Maybe that line ought simply to be removed from the docs. I don't think I would have spent so much time scratching my head without its apparent contradiction of what I was seeing.

@ljharb
Copy link
Contributor

ljharb commented Aug 15, 2024

@tannerstern you can add ignore-scripts: true to .npmrc, or you can set the NPM_CONFIG_IGNORE_SCRIPTS env var.

@MohammedSiddiqui
Copy link

For cases where dev dependencies don't exist when running npm install on your CI, this little snippet did the trick for me:

"prepare": "husky || true",

However, this would only work for Unix-based systems where we are ignoring any errors from the first command.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation documentation related issue Priority 2 secondary priority issue Release 8.x work is associated with a specific npm 8 release
Projects
None yet
Development

No branches or pull requests