0xProject / 0x-monorepo

0x protocol monorepo - includes our smart contracts and many developer tools
Other
1.41k stars 465 forks source link

subproviders pkg: dependency is breaking dist builds targetting es5 #1110

Closed pkieltyka closed 5 years ago

pkieltyka commented 6 years ago

the 0xproject/subproviders package uses "web3-provider-engine": "14.0.6" and unfortunately, this dependency in its dist/ files breaks es5 compatibility as it does not transpile the const keywords from its distribution files. As a result, this breaks distribution of my app that uses it.

Another thought is to get rid of web3-provider-engine and standardize on ethers

fabioberger commented 6 years ago

@pkieltyka thanks for opening the issue. We already know about this limitation and have contemplated dropping web3-provider-engine, however it hasn't been high-pri enough for us yet.

Even if we dropped it, there are other packages that also don't have ES5 compatibility in the ecosystem (e.g EthereumJS/RLP).

It is not as easy as "get rid of providerEngine and standardize on ethers". Ethers.js providers don't provide the same middleware-type cascading behavior as ProviderEngine. It doesn't have the same compos-ability. If we were to get rid of providerEngine, we would replace it with something very similar.

Please let me know if I'm missing some information about hows Ethers providers work, etc...

pkieltyka commented 6 years ago

The middleware subproviders and cascading composeable stuff should be a pretty small amount of code to make that happen. I’ll check but I think ethers has a fallback but perhaps it’s not a middleware approach if the interfaces are not consistent, could be wrapped or augmented. I’ll have a look at this tomm.

Es5 issues can be solved upstream too, which is where they should be solved. But generally I feel those mistakes are just an example of the sloppiness of those libs.

On Oct 8, 2018, at 10:35 PM, Fabio Berger notifications@github.com wrote:

@pkieltyka thanks for opening the issue. We already know about this limitation and have contemplated dropping web3-provider-engine, however it hasn't been high-pri enough for us.

Even if we dropped it, there are other packages that also don't have ES5 compatibility in the ecosystem (e.g EthereumJS/RLP).

It is not as easy as "get rid of providerEngine and standardize on ethers". Ethers.js providers don't provide the same middleware-type cascading behavior of several subproviders. It doesn't have the same compos-ability. If we were to get rid of providerEngine, we would replace it with something very similar.

Please let me know if I'm missing some information about hows Ethers providers work, etc...

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

fabioberger commented 6 years ago

Hm, ok, looked a bit into Ethers provider and see what you mean. Let me know if you can point me in the right direction on how you would do about wrapping/augmenting it.

Yeah, agreed. I guess we could fork web3-provider-engine, compile it to ES5 and point our dep to the compiled version. Is that what you mean by fixing it upstream?

pkieltyka commented 6 years ago

@fabioberger hey man, I was distracted at ETHSF/CESC the last few days, but back at it. Have you tried the FallbackProvider from ethers? For the composable providers, do you want to cascade down for retries in case one fails, or do you want to have a group of middlewares ahead to build a request? lmk and I'll try to draft something.

also, for upstream, I meant the maintainers of web3-provider-engine should be fixing the es5 compatibility issue. Forking it is one solution too, but it then becomes another dep you need to keep track of which isn't ideal

fabioberger commented 6 years ago

Hey @pkieltyka, no worries.

I went ahead and opened an issue with ethereumjs-rlp: https://github.com/ethereumjs/rlp/issues/34.

Here are the subproviders we currently use:

It has been convenient until now to have a middleware stack of JSON RPC request handlers as you can see from the above. Depending on your needs, you can compose a provider that handles signing for a particular provider, and also has some custom retry logic for requests to backing nodes, and by-passes gas estimation entirely. My question is whether Ethers.js is flexible enough to enable this level of customization?

Also, we've recently run into a bunch of issues with Metamask where they do not comply to the JSON RPC spec (see: Metamask Issue Comment and MetamaskSubprovider). I believe these issues still exist with Ethers providers (see: Ethers.js one-off special-case) but I don't feel like this is the correct place to handle such issues. It can get ugly really fast, especially as we move towards a world where people want to support tons of signers.

Excited to hear how you think we can re-orient our subproviders to play nicely with Ethers.js. We really respect the Ethers project and would be willing to work on crafting and supporting a providers standard together. We are not stuck to our ways, we just want to make sure we move to something that makes the most rational sense, and allows us the flexibility we need.