search

yunxing / opam-npm

convert opam packages to npm
8 stars 12 forks source link

Remove peerDependencies #4

Open bsansouci opened 7 years ago

bsansouci commented 7 years ago

I think Reason already depends on npm3 (which deprecates but supports peerDependencies). They break when using yarn it seems.

yunxing commented 7 years ago

I think we should still stay with "peerDependencies". This gives hint to the resolver that only one copy of the package should only be installed and we can use it to make sure only one version of ocaml is installed.

I'm aware of the current problem where yarn fail to resolve the peerDependencies correctly. People are discussing a related issue here: https://github.com/yarnpkg/yarn/issues/579, and the proposed solution is to make peerDependencies what they designed for.

To fix the problem for now in your own project, try something similar here: https://github.com/reasonml/RebelExampleProject/pull/5

jordwalke commented 7 years ago

@yunxing In that model, would yarn treat peerDependencies as --flat? That would be kind of nice to say "these dependencies must be flat, but the other ones don't need to be".

I think there's a couple of possibilities.

  1. These must be flattened.
  2. These don't need to be flattened but try hard to flatten them anyways.
  3. These shouldn't be flattened.

I think we can hold off on 3, but 1 and 2 seem nice. Maybe number one could be peerDependencies, and everything else could be 2.

yunxing commented 7 years ago

yes, the proposed model in that thread of yarn is a combination of 1 and 2.