search

Keydonix / liquid-long

The Unlicense
8 stars 1 forks source link

Adds LiquidLong factories. #76

Closed MicahZoltu closed 5 years ago

MicahZoltu commented 5 years ago

The goal here is to make it more obvious/clear how the library is intended to be used, and to also hide the ethers implementation detail from users thus allowing us to swap it out in the future. The primary change is removing almost all exports from index.ts. This makes it clear to userse that they should import LiquidLong and nothing else. To facilitate this, the LiquidLong class now has a couple factory methods on it that take mostly mundane parameters (such as strings and numbers). The one exception is the web3Provider parameter which is really just asking for window.web3.currentProvider (MetaMask and MetaMask like things).

To make this work, the library needs to reach inside the library in a couple places for testing. I'm generally not a fan of this pattern, but have left it in for now as something to be addressed later. I believe the correct solution here would be to split the project up into several packages that are tested and released separately, but that is probably overkill at this point.

epheph commented 5 years ago

The construction interface is simpler. It does seem like in the short-run, it is more tightly bound to ethers, but it's fine for MVP

MicahZoltu commented 5 years ago

Internally it always had a hard dependency on ethers, but the public surface area made it appear that wasn't the case. This fixes the bug where we incorrectly give people the idea they have a choice. 😬