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

fix(version): Ignore npm lifecycle scripts on package lock update #3295

Merged
merged 2 commits into from Aug 16, 2022

Conversation

fahslaj
Copy link
Contributor

@fahslaj fahslaj commented Aug 16, 2022

Fixes an issue where npm install lifecycle scripts were being run during the version command.

Description

This change passes the --ignore-scripts option to the npm install command used to update the lockfile during the version command.

Motivation and Context

This prevents npm lifecycle scripts from running, which can cause CI workflows to fail.

How Has This Been Tested?

An end to end test has been created to cover the case of having failing lifecycle scripts during a version command. I also manually tested the change against a local environment with install scripts and git commit hooks (with husky).

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (change that has absolutely no effect on users)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@fahslaj fahslaj marked this pull request as ready for review August 16, 2022 17:51
@JamesHenry JamesHenry merged commit 1ba2b8a into lerna:main Aug 16, 2022
@fahslaj fahslaj deleted the ignore-scripts-on-package-lock-update branch August 16, 2022 18:24
@sluramod
Copy link

sluramod commented Aug 30, 2022

@fahslaj This actually creates an issue for us. Is there a way to use lerna.json configuration to specify whether or not use --ignore-scripts ?

@fahslaj
Copy link
Contributor Author

fahslaj commented Aug 31, 2022

@sluramod Running npm install --package-lock-only to update the root package-lock.json file is a very recent addition to the implementation details of the version command. It was never meant to run normal install lifecycle scripts, so this PR removed an unintended side-effect. If you need to run additional scripts during or after lerna version, the proper way to do so is via version lifecycle scripts.

If version lifecycle scripts do not solve your problem, then could you please share more information about your use case? (Specifically, the scripts that need to be run and why they must be done as part of the implementation details of the lerna version command)

Another note - this PR only affects the version command. Scripts will still be run during a normal npm install and during lerna bootstrap.

@sluramod
Copy link

sluramod commented Aug 31, 2022

@fahslaj I use package firebase-tools from google. when I run npm install I get a bunch of dependencies with "extraneous": true from that package. When I run lerna version from 5.4.3 all of those extraneous dependencies get removed from my package-lock.json. To fix this I need to run npm install after I run lerna version, amend lerna's commit with updated package-lock.json and move the tag that lerna created for the version to the amended commit.
As you can see it's not very convenient and I can't force the change in third-party package.

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.

None yet

3 participants