search

Uniswap / examples

107 stars 141 forks source link

feat: web3-react #27

Closed dannydaniil closed 1 year ago

grabbou commented 1 year ago

Also, as I already highlighted via DMs, we do have two additional examples already in web3-react, so you may consider checking them too and deciding whether there's anything helpful/useful that could be borrowed. Personally, I feel like we don't need that many examples and would love to see this residing in one place. I am going to follow-up internally and kick off discussion to see what can be done in this regard.

dannydaniil commented 1 year ago

@grabbou Thanks so much for the detailed review!

On the point about the examples on the web3-raect repo, they are pretty much empty, so there is not much to pick from.

On the point of where the examples should live, we did put a fair amount of thinking and captured why there should be a separate examples repo. Now that there is no dedicated DevX team to work on this, we could perhaps kick off the discussion again. Given the amount of work done, let's try to get this one merged, and move it later if required.

grabbou commented 1 year ago

On the point about the examples on the web3-raect repo, they are pretty much empty, so there is not much to pick from.

Agreed, that's why we can hopefully come up with a unified super example out of two!

On the point of where the examples should live, we did put a fair amount of thinking and captured why there should be a separate examples repo. Now that there is no dedicated DevX team to work on this, we could perhaps kick off the discussion again. Given the amount of work done, let's try to get this one merged, and move it later if required.

Right! Just for the record to those that follow our discussion, we will continue to discuss this on our Slack and most likely come up with some action items. But regardless of the outcomes, this PR is going to be merged once approved.

grabbou commented 1 year ago

Small comment:

error web3-react@0.1.0: The engine "node" is incompatible with this module. Expected version ">=16.0.0". Got "14.21.1"

Looks like the minimum required Node version is 16. I am generally having 14 across different Uniswap applications, might be worth checking what is the version we plan to set as a baseline.

Update: 14 is required for interface right now, so setting bar at 16 will require folks switching (not saying it's a bad thing, but will definitely take some time. I guess we can relax it to 14 or 16, as per web3-react itself for now).