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

Call version lifecycle scripts during publish #1029

Merged
merged 2 commits into from Sep 27, 2017
Merged

Call version lifecycle scripts during publish #1029

merged 2 commits into from Sep 27, 2017

Conversation

cwmoo740
Copy link
Contributor

@cwmoo740 cwmoo740 commented Sep 27, 2017

Description

Following the guidelines here: https://docs.npmjs.com/cli/version

Lerna will now call preversion, version, and postversion scripts at the appropriate times in each package when running lerna publish.

Motivation and Context

This allows more hooks into the lerna publish command. For example, I want to run a custom changelog generation script after package.json version is updated, but before the git commit.

Closes: #774

How Has This Been Tested?

Added some tests to verify that the three lifecycle scripts are called.

I wanted to mock Package.js but found no easy way to do that - the changes I was working on ended up affecting many of the tests in Publish.js, since it doesn't import or have access to the Package class directly.

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)

This may be a breaking change because it now calls version scripts on publish, whereas before they were ignored.

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.

@@ -595,7 +609,7 @@ export default class PublishCommand extends Command {
}

gitCommitAndTagVersionForUpdates() {
const tags = this.updates.map(({ package: { name } }) =>
const tags = this.updates.map(({ "package": { name } }) =>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can back out this change, but package is a reserved keyword and shouldn't be referenced without quotes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, good call!

@cwmoo740
Copy link
Contributor Author

I corrected the tests to work with windows path separators - it should rerun the build.

Copy link
Member

@evocateur evocateur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good so far, a couple nits and we can merge.

@@ -2,7 +2,6 @@ import chalk from "chalk";
import log from "npmlog";
import normalizeNewline from "normalize-newline";
import path from "path";

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These newlines are significant (and mostly consistent across the other test files), please keep them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do

script,
[],
path.resolve(testDir, "packages", "package-2"),
jasmine.any(Function)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty sure expect.any(Function) should work here? Or is this Jest 19 (not upgraded yet for reasons) biting us again?

@cwmoo740
Copy link
Contributor Author

The appveyor test failed on node 4, but it seems to be unrelated - can those tests be re-run?

@evocateur
Copy link
Member

Nah, those appveyor tests have been flapping due to some unrelated reason. It's quite likely a re-run would just fail randomly in the node 8 matrix. We're fine here.

@evocateur
Copy link
Member

Looks great, thanks @cwmoo740!

@evocateur evocateur changed the title Add calls to version lifecycle scripts during lerna publish (#774) Call version lifecycle scripts during publish Sep 27, 2017
@evocateur
Copy link
Member

Oh, and I don't see this as breaking, since they were never run previously, and moreover shouldn't have been run by a manual npm version inside the leaf package's directory.

@evocateur evocateur merged commit 86db2fc into lerna:master Sep 27, 2017
@@ -536,12 +536,26 @@ export default class PublishCommand extends Command {
this.updatePackageDepsObject(pkg, "devDependencies", exact);
this.updatePackageDepsObject(pkg, "peerDependencies", exact);

// exec preversion script
pkg.runScript("preversion", (err) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will run script asynchronously, wouldn't it? Same goes for other hooks that were added.
Are we fine with that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That’s true. I forgot that almost everything in publish is synchronous, unlike bootstrap where parallelism is important.

@alex-pex
Copy link

If I read correctly, hooks are called in handled package, not on the root package.

Can we also call hooks on the rootpackage? I'd like to call a hook on postversion, when everything is done (I'd like to create a merge commit between two branches).

If we can't, I'm stuck with a wrapper command

  "scripts": {
    "publish-all": "lerna publish && ./hooks/postversion.js"
  }

@lock
Copy link

lock bot commented Dec 27, 2018

This thread has been automatically locked because there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Dec 27, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Execute scripts preversion, version and postversion during lerna publish.
4 participants