compound-finance/compound-js

Suggestion to mark as "ethers.js" as peer dependency

Closed this issue · 3 comments

PaulRBerg commented

It is not an error to list a package as both a normal dependency and a peer dependency. In fact, in certain cases it is the recommended approach. More on that here.

Compound.Js relies heavily on Ethers - if, say, a user has installed v4 of Ethers but then installs Compound.Js, things might break. It's better to explicitly ask the user to import v5.

ajb413 commented

@PaulRBerg Hey take a look at #32. Does that look right to you? I think the only other effect of the change is that users of npm <7 will need to do npm i @compound-finance/compound-js ethers@5 instead of only npm i @compound-finance/compound-js.

PaulRBerg commented

Re #32 - I think that you should keep "ethers" as a normal dependency .. I can't remember off the top of my head why exactly, but in the past I had problems when the peer dep was not marked as either a normal dep or a dev dep as well.

users of npm <7 will need to do npm i @compound-finance/compound-js ethers@5 instead of only npm i @compound-finance/compound-js

True, but they would have been in trouble had they not used ethers v5. Better safe than sorry.

  • 👍1
ajb413 commented

Added in 0.4.0.

  • 🎉1