Add peerDependencies to packages

[ryanio]

Fixes #1200

I would appreciate a double check of every listed peerDep, that it is used in the public interface of the package.

I didn't add any for client since it is an executable that you pass in command line args, I don't think it has any peerDeps then, is this reasoning correct?

npm 7 installs peer dependencies by default so this goes great with our new developer setup, and should ease end user library incompatibilities as explained in the linked issue.

[codecov]

Codecov Report

Merging #1402 (0e6d1d2) into master (e2a0a4d) will decrease coverage by 0.17%.
The diff coverage is n/a.

Flag	Coverage Δ
block	86.60% <ø> (ø)
blockchain	83.43% <ø> (ø)
client	83.38% <ø> (-0.67%)	⬇️
common	94.17% <ø> (-0.23%)	⬇️
devp2p	83.39% <ø> (+0.07%)	⬆️
ethash	82.83% <ø> (ø)
tx	88.24% <ø> (ø)
vm	79.34% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

[acolytec3]

Here's the additional ones I think we need:
blockchain <-- block, @types/levelup
ethereum-js/util <-- Do we need rlp since the whole module is exported as part of util/externals?
vm <-- merkle-patricia-tree as it's the type for one of the VMOpts properties

General question - do we need @types/bn.js as a peerDependency wherever we have bn.js?

[holgerd77]

Hmm, can we please finish the discussion on this before opening PRs here?

I have got several issues here:

1. This would re-introduce on the sideline the need for breaking releases on all libraries, I thought we had some at least preliminary agreement that we only want to do VM, Blockchain(, Block)?
2. If I get this right this would require npm 7 for having peer dependencies automatically installed. Also thought that we had some somewhat agreeing discussion here: we can do some requirement like this in our own dev setup, but put this requirement on the library users is a totally other (and very likely not acceptable) thing (have no stats, but likely only a very tiny minority will have already upgraded to npm v7)
3. Last but not least, will just cite from my thread on #strategy: "Apart from what Patricio mentioned in the initial issue there had been not much complain about these kind of inconsistencies from other users, the UX is eventually questionable with the need to manually install dependencies (do I get this right?) and this is generally some strong intervention in our current installation process." -> so this is a very heavy change for a not yet very strongly proven benefit

I've switched the PR label here to "needs discussion" and "blocked".

Some general note here for reviewers: apart from the discussion outcome this is a super-delicate PR. Please only review and do not merge until we have a very clear agreement here.

[ryanio]

my apologies for getting ahead of the discussion

we can do some requirement like this in our own dev setup, but put this requirement on the library users is a totally other (and very likely not acceptable) thing (have no stats, but likely only a very tiny minority will have already upgraded to npm v7)

yeah, I forgot that's really important. I'll close this PR for now and we can reopen if we'd like to continue in the future.

[holgerd77]

No problem. 😄

This will likely be valuable for some future integration, have added an additional note to the breaking release issue on the PR. For now rather too early, but we might want to re-check on the subsequent round of breaking releases next year or so.