joaniefromtheblock commented 4 months ago

Description

Adds removeAllListeners method and example to docs

Issue(s) fixed

Fixes #563

Preview

https://docs.metamask.io/add-listen/wallet/reference/provider-api/#remove-listeners

Checklist

Complete this checklist before merging your PR:

[x] If this PR contains a major change to the documentation content, I have added an entry to the top of the "What's new?" page.
[ ] The proposed changes have been reviewed and approved by a member of the documentation team.

github-actions[bot] commented 4 months ago

Preview published: add-listen

github-actions[bot] commented 4 months ago

Preview published: add-listen

github-actions[bot] commented 4 months ago

Preview published: add-listen

github-actions[bot] commented 4 months ago

Preview published: add-listen

httpJunkie commented 4 months ago

Considering the current version of the page:

When we are talking about the code that has the removeAllListeners() below the code box. We say that this happens on component unMount which is putting the example in context of a React application. The code in the example is ok, because the code can make sense in any JS code environment.

My concern is that the explanation mentions something that happens in React, but if this were plain javascript code, we might just want to say:

Text above the code (Warning)

Use removeAllListeners with caution. This method clears all event listeners associated with the emitter, not only the listeners set up by other application code. Using this method can unexpectedly clear important event handlers, interfere with scripts, and make debugging more complex. You can use the method removeListeners to safely remove specific listeners.

Text below the code:

In the provided code example, removeAllListeners is called to remove all event listeners attached to the window.ethereum object. This cleanup callback function ensures that when the code block is no longer needed, there are no lingering event listeners.

httpJunkie commented 4 months ago

The code example could be reduced to:

window.ethereum.on('_initialized', updateWalletAndAccounts);
window.ethereum.on('connect', updateWalletAndAccounts);
window.ethereum.on('accountsChanged', updateWallet);
window.ethereum.on('chainChanged', updateWalletAndAccounts);
window.ethereum.on('disconnect', disconnectWallet);

return () => {
  window.ethereum.removeAllListener()

This example is more efficient code that instead of setting multiple .on()'s we are just calling some function that does everything needed for that specific event. It also is familiar to those working with MetaMask and doesn't introduce calls to SDK methods that don't make sense in the context of this reference.

This will reduce the complexity of the example in a way that doesn't compromise the illustrated point.