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

Remove peerDependencies #4

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

bsansouci
Copy link
Contributor

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

I think Reason already depends on npm3 (which deprecates but supports `peerDependencies`). They break when using yarn it seems.
@yunxing
Copy link
Owner

yunxing commented Oct 13, 2016

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: yarnpkg/yarn#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
Copy link
Collaborator

@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
Copy link
Owner

yunxing commented Oct 14, 2016

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants