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] lifecycle scripts are run for 'npm install --package-lock-only' #2787

Closed
DeeDeeG opened this issue Feb 26, 2021 · 9 comments
Closed

[BUG] lifecycle scripts are run for 'npm install --package-lock-only' #2787

DeeDeeG opened this issue Feb 26, 2021 · 9 comments
Labels
Bug thing that needs fixing Priority 1 high priority issue Release 7.x work is associated with a specific npm 7 release

Comments

@DeeDeeG
Copy link

DeeDeeG commented Feb 26, 2021

Current Behavior:

For example, prepare and postinstall scripts are run after doing npm install --package-lock-only.

Expected Behavior:

This is supposed to simply update package-lock.json, no more no less. (Like in npm v6, I expect lifecycle scripts not to run with --package-lock-only.)

Steps To Reproduce:

  1. In a directory with a package.json that has lifecycle scripts defined...
  2. Run npm install --package-lock-only
  3. Lifecycle scripts run unexpectedly

Environment:

ex.

  • OS: macOS 10.15.7
  • Node: v10.24.0, v12.21.0, v14.16.0, v15.10.0
  • npm: 7.6.0 (or any other npm v7 I have tested, such as v7.2)
@DeeDeeG DeeDeeG added Bug thing that needs fixing Needs Triage needs review for next steps Release 7.x work is associated with a specific npm 7 release labels Feb 26, 2021
@ljharb
Copy link
Contributor

ljharb commented Feb 26, 2021

Lifecycle scripts might be written to edit package.json, prepopulate things in node_modules, or edit the lockfile after the fact, all of which would apply to --package-lock-only. Can you elaborate on the lifecycle scripts you have where running them on package-lock-only is problematic?

@DeeDeeG
Copy link
Author

DeeDeeG commented Feb 26, 2021

  • Any script that relies on node_modules existing. (Including any/all devDependencies).

    • I most often use --package-lock-only to maintain or update package metadata without having to go through an install.
  • Any script that relies on the repository being in a tidy state

    • Similar to the above scenario, I frequently use this to be able to maintain or update the package metadata when I know the dependencies are built for a wrong target, or I've edited some things and the repo is out of sorts.

@DeeDeeG
Copy link
Author

DeeDeeG commented Feb 26, 2021

A workaround is --package-lock-only --ignore-scripts, but I feel --package-lock-only, by the name, fairly clearly implies it will just update package-lock.json. Hmm. I mean, at the end of the day I can admit this could be a design choice rather than a strict bug.

And --package-lock-only --ignore-scripts is pretty semantic (like in a good way -- it's clear what it means)... I just personally find it a bit verbose and counterintuitive to need the latter if using the former.

@DeeDeeG
Copy link
Author

DeeDeeG commented Mar 5, 2021

I was doing a rebase today and that reminded me:

Rebases where there's a merge conflict in package-lock.json are super common. I have a habit of running npm install --package-lock-only to resolve these, which is mentioned as one of the recommended ways of fixing these conflicts in the npm v6 docs. I'd love to not have the lifecycle scripts unexpectedly run while I'm trying to resolve a merge conflict in package-lock.json by doing npm install --package-lock-only . (Which happened to me today :/)

@darcyclarke darcyclarke added Priority 1 high priority issue and removed Needs Triage needs review for next steps labels Apr 16, 2021
@darcyclarke
Copy link
Contributor

Just a quick update/reference here, we should revert back to this behaviour as this seems to be an unintended change from v6 to v7 (change should be in lib/install.js, adding a check for --package-lock-only: https://github.com/npm/cli/blob/latest/lib/install.js#L139-L164)

@darcyclarke
Copy link
Contributor

darcyclarke commented Jul 12, 2021

Actions

  • pacote should not run prepare scripts when package-lock-only or ignore-scripts is set

@DeeDeeG
Copy link
Author

DeeDeeG commented Jul 12, 2021

Closed accidentally? (I couldn't find a commit/PR that updates the behavior relating to this issue.)

@DeeDeeG
Copy link
Author

DeeDeeG commented Jul 12, 2021

By the way: I am surprised to find npm@6 runs the preinstall lifecycle script even with the npm install --package-lock-only command + flag. So I suppose getting rid of that could be an unexpected change for some people who rely on that (if any). Also note the shrinkwrap scripts that run in npm@6, but not in npm@7.

Results of the npm install --package-lock-only command in npm 6 and 7:

Script name NPM 6 NPM 7
preinstall 🏃 🏃
preshrinkwrap 🏃
shrinkwrap 🏃
postshrinkwrap 🏃
install 🏃
postinstall 🏃
prepublish 🏃
preprepare 🏃
prepare 🏃
postprepare 🏃

I personally don't think that any of the lifecycle scripts should run with --package-lock-only, but that's just my opinion. I'm not sure if the preinstall script running would negatively impact me/my use-cases (I haven't noticed a negative impact from it so far), and in any case I wanted to relay that information.

@DeeDeeG
Copy link
Author

DeeDeeG commented Jul 15, 2021

Can this issue please be re-opened?

I think it was closed accidentally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Priority 1 high priority issue Release 7.x work is associated with a specific npm 7 release
Projects
None yet
Development

No branches or pull requests

3 participants