Make web3.js side-effect free

[bpierre]

Web3.js is modifying the window.ethereum object, which can cause different issues with other libraries trying to interact with window.ethereum.

I think window.ethereum should be considered immutable, and any modification to its API, if needed, should be done internally.

More generally, I think a library like web3.js should not have any side effect.

[cgewecke]

@bpierre Have some further questions about this...

which can cause different issues with other libraries trying to interact with window.ethereum

Could you go into greater detail about what issues you're seeing / give an example?

I think window.ethereum should be considered immutable, and any modification to its API, if needed, should be done internally.

Could you clarify this as well? window.ethereum is Metamask's API - they are establishing its identity and as the code you've linked to shows, Web3 isn't directly modifying that key.

For further background, there's also an effort underway to formalize the ethereum provider JS API in EIP 1193 and it looks like Metamask is planning to roll this out as a breaking change in "early 2020"

[bpierre]

Could you go into greater detail about what issues you're seeing / give an example?

An app or library should expect Metamask to provide the .send(method, params) API, but this wouldn’t work since web3.js replaces ethereum.send by ethereum.async.

We could also imagine a library relying on sendAsync that would suddenly stop working after the web3.js dependency gets added to a project, since web3.js attempts to delete it. Luckily this delete doesn’t work with the ethereum object injected by MetaMask because it is a proxy object.

On the web3.js side, MetaMask could decide to freeze the ethereum object, making the ethereum.send = ethereum.sendAsync inoperant.

I think window.ethereum should be considered immutable, and any modification to its API, if needed, should be done internally.

Could you clarify this as well? window.ethereum is Metamask's API - they are establishing its identity and as the code you've linked to shows, Web3 isn't directly modifying that key.

Sure! window.ethereum is provided by MetaMask, as well as a web3 object (corresponding to web3.js 0.20.7) for compatibility reasons. This object also contains web3.currentProvider, which is the provider object:

window.ethereum === window.web3.currentProvider // true

Objects are passed by reference in JavaScript, so any modification to web3.currentProvider is reflected on ethereum, which is the same object. Note that even if it wasn’t the case, I don’t think modifying anything in the environment would be a good idea, even if the object in question happens to be a previous version of this library! 😄

For further background, there's also an effort underway to formalize the ethereum provider JS API in EIP 1193 and it looks like Metamask is planning to roll this out as a breaking change in "early 2020"

Yes this API is available in Metamask already − and it’s exactly the issue I encountered: I was trying to interact with .send() (new API) but web3.js was overwriting it with the old-style API (.sendAsync()).

[cgewecke]

it’s exactly the issue I encountered: I was trying to interact with .send() (new API) but web3.js was overwriting it with the old-style API .sendAsync().

Ah! Thanks...will investigate this.

[cgewecke]

For further background, per this Metamask breaking change notice there is an interim 6 week period where Metamask will continue to inject web3 while it rolls out the ethereum API.

I think at the end of this the following should hold (and side-effects for the MM case should disappear):

window.ethereum === window.web3.currentProvider // false

There's additional info about MM no longer injecting Web3 in this Metamask blog post).

If anyone is looking for an example patch, @bpierre has written one at aragon/use-wallet PR 21