Implement npm@4's prepublish behavior

[Jessidhia]

Fixes #1323, motivation is explained there.

Lifecycle calls were written in the same order as npm runs them. A deprecation warning for prepublish was also added. npm@5 intends to stop running prepublish, and a later npm version will rename prepublishOnly to prepublish.

One notable difference is that npm pack is also supposed to run prepare and (old) prepublish, but yarn pack does neither. Not sure where that should go.

I also tried looking into writing tests, but as the lifecycle scripts are run by a wrapper rather than by the Install class itself, it can't be tested with the helpers used by __tests__/commands/install.js (?).

Sample output for install:

$ cat package.json
{
  "scripts": {
    "preinstall": "echo preinstall",
    "install": "echo install",
    "postinstall": "echo postinstall",
    "prepare": "echo prepare",
    "prepublish": "echo prepublish",
    "prepublishOnly": "echo prepublishOnly"
  }
}

$ ~/src/yarn/bin/yarn
yarn install v0.16.2
info No lockfile found.
$ echo preinstall
preinstall
warning No license field
success Nothing to install.
warning `prepublish` on install is deprecated and will stop running in the future.
$ echo install
install
$ echo postinstall
postinstall
$ echo prepublish
prepublish
$ echo prepare
prepare
✨  Done in 0.08s.

I don't know how to test yarn publish as there is no way to do a dry-run with it AFAICT.

[Jessidhia]

The test failures look unrelated to the changes in this PR

[wyze]

If you rebase this onto master and push, it should fix the CI builds for this PR.

[SimenB]

Would love to see this merged

[hpurmann]

@Kovensky, still there? 😉

[Jessidhia]

Yep.

It is hard to keep the branch up-to-date, though, because of the change to the english strings file. Any new string added will conflict with this, so it's easier to only rebase once this is accepted and ready to merge.

[bestander]

I am not too sure that Yarn would want to follow breaking changes along with npm CLI.
npm CLI introduces a breaking change between version 4 and 5, should Yarn bump major versions as well then?
I would remove the warning because I am not too sure where this is going for Yarn now, this requires a wider RFC discussion.

Although it seems reasonable to add the new prepare lifecycle step to ensure more compatibility.

[vovacodes]

@bestander it is very important for yarn to follow the same contract as npm client, otherwise, we will divide the ecosystem

[bestander]

It is but we need to think better how this should align with semver of yarn.

[FezVrasta]

Are there any news on this change? Seems pretty important.

[bestander]

Can we do a minimum non breaking change to support npm3 and npm4 for now?

[FezVrasta]

As far I see, this PR doesn't break the compatibility with v3, it will just throw a warning in case prepublish is used, just like npm@4 does right now.

The warning deprecates prepublish but unlike npm, doesn't specifies in which version it will stop working, so seems vague enough to give Yarn liberty to proceed as you prefer in the future.

[bestander]

I am not sure that warning is needed at this point because we haven't discussed what legacy npm features Yarn will support and which ones and when it will drop.
You are welcome to start an RFC for that.

Considering that this is a change that affects installation behavior, would you add tests that describe the desired new behavior?

[Jessidhia]

@bestander any way to inspect the warnings output, in a test, as a result of an install command?

I am also not sure what would be a portable way of checking each given lifecycle script ran. On my local testing I just used echo prepublish (et al, which would actually be portable, but would need a way to capture the stdout instead).

[bestander]

Here is an example of install scripts tests https://github.com/yarnpkg/yarn/blob/master/__tests__/commands/install/integration.js#L372-L389.

You don't have to pack the dependencies for the test like there anymore.
You can have a local file dependency for tests e.g. https://github.com/yarnpkg/yarn/blob/master/__tests__/fixtures/install/artifacts-finds-and-saves/package.json

To test for a warning message, I guess you can access reporter.stdout and then search for a regex in it.

[bestander]

As this has been around for some time and is controversial I recommend going through RFC.
I'll close this PR for now.