Publishing order should take peerDependencies into account

[loganfsmyth]

Currently the package graph ordering is defined in

lerna/core/package-graph/index.js

Line 68 in bd79949

? Object.assign({}, currentNode.pkg.optionalDependencies, currentNode.pkg.dependencies)

and only takes dependencies into account, and not peerDependencies. This means it is possible to publish a module with a peerDependency on a specific version that you are publishing at the same time, and the version with the peerDependency may be published before the version that it depends on.

Expected Behavior

Ideally the version ranges in the peerDependencies should be validated, and if the range is only satisfied by the version you are just about to publish, it should publish it later, after the dependency is publishes.

Context

Just noticed on Babel that @babel/cli was being published before @babel/core which means there is a period (which on this publish was about 4.5 minutes, since we publish a LOT of packages), where the CLI depends on a non-existent version of @babel/core because during our beta phase we're using specific non-range peerDependencies. That gets extra dangerous once you take caching registry mirrors into account.

Your Environment

Executable	Version
lerna --version	2.0.0
npm --version	VERSION
yarn --version	1.6
node --version	6.12.3

[loganfsmyth]

This is also exacerbated by #1018 since Lerna actually explicitly changes the peerDependency to a non-range version.

[evocateur]

It's weird that publish even cares to distinguish between "allDependencies" vs "dependencies" when constructing the graph. Batching "dependencies" merely omits devDependencies, which is the traditional location for pairing with peerDependencies (and would "solve" your issue). There's a comment explanation, but it's still weird. (I'd rather just throw on all circularity, tbh)

We would need to improve the peerDependencies parsing to recognize local siblings, and I doubt that work would be easy to back-port into the 2.x branch. 3.x will be latest soon enough, however.

Notably, since v2.7.0 lerna does not mutate peerDependencies during publish (#1187).

[loganfsmyth]

It's weird that publish even cares to distinguish between "allDependencies" vs "dependencies" when constructing the graph.

I see it as important because it's possible to have dependency cycles that are present when devDependencies are taken into account, which don't actually matter to users of your package. If devDependencies were walked too, it makes it much more likely that you'd publish in a bad order due to cycles.

We would need to improve the peerDependencies parsing to recognize local siblings, and I doubt that work would be easy to back-port into the 2.x branch. 3.x will be latest soon enough, however.

No problem. Thanks for your work on Lerna :)

Notably, since v2.7.0 lerna does not mutate peerDependencies during publish (#1187).

Oh cool, great to know. I saw that the issue I linked was still open and assumed it hadn't been tackled yet.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[sparebytes]

It appears lerna 3.13.1 doesn't honor peerDependencies for toposort. Is the solution to use peerDependencies in conjunction with devDependencies?

[padraig-meaney]

Running in to this issue as well, using a mono repo with 3 packages (A. B. C) where A is a peerDep (and devDep) of B and C, and B is also a peerDep (and devDep) of C.

I can work around this for now by also specifying the packages as optionalDependencies, but would you be open to a PR to add a flag to include devDeps when evaluating the publish hierarchy? At least that way if a user needs the behavior they can opt in.

[evocateur]

@sparebytes Yes, it is a best practice with npm packages (regardless of monorepo or not) to specify devDependencies for every member of peerDependencies.

@padraig-meaney I agree it's unfortunate that proper peer+dev deps aren't taken into account in lerna publish order. I would be open to a --graph-type flag whose value defaults to the current argument ("dependencies"), but with an optional "all" value that achieves what you're looking for.

(as much as it's tempting to automatically include devDependencies in the topological order if local peer + dev deps are detected, I feel that's a bit too magical at this point)

# equivalent to current behavior
npx lerna publish --graph-type dependencies

# adds devDependencies to the package graph, thus influencing the topological sort
npx lerna publish --graph-type all

Configured in lerna.json:

{
  "command": {
    "publish": {
      "graphType": "all"
    }
  },
  "packages": [
    "packages/*"
  ],
  "version": "independent"
}

I'd be very interested in flipping the default in the next major, always adding devDependencies to the package graph for lerna publish (which is the default for literally every other command that topologically sorts the package graph, such as lerna exec and lerna run). There's enough logging when circularity is encountered, and we could suggest --graph-type=dependencies as a (possible) fix for it.

Reopening to track the work.

[padraig-meaney]

@evocateur that sounds great to me. I may have some free bandwidth in the next 2 weeks to put together a PR for this.