MetaMask / metamask-extension

:globe_with_meridians: :electric_plug: The MetaMask browser extension enables browsing Ethereum blockchain enabled websites
https://metamask.io
Other
12.02k stars 4.91k forks source link

Stop reloading dapps on network change #3599

Open sulliwane opened 6 years ago

sulliwane commented 6 years ago

Expected behavior: Chain change in MM UI should not trigger a whole refresh of the browser page.

I experience this in Chrome AND firefox, with my react application.

Any input on this ?

Many thanks

2-am-zzz commented 6 years ago

This is expected behavior since users who switch chains should have an updated UI on the current status of that chain. Since many applications rely on being on a specific network to function, this is a way for us to avoid users getting confused with interacting with a dapp that has residual data from another network.

sulliwane commented 6 years ago

Thank you @Zanibas for your answer.

I truly belive this should be the responsability of the application developper to listen for chain change, and react accordingly.

Let's be honest, forcing a page reload is a disastrous user experience...I really hope there would be a way to avoid this page reload, and let the application developper responsibly react to chainID change...

In my case, I detect the chain change by checking the chainID every 100ms...

What do you think?

Many thanks!

sulliwane commented 6 years ago

Any input on this question? Thanks!

danfinlay commented 6 years ago

@sulliwane The team actually discussed this a bit, and we ultimately agree with you. Force reloading was a band-aid for a period where developers were just learning to make Dapps, and were almost never using good practices. It is probably time to grow up, and remove these training wheels. Changing the title of this issue to reflect the desired action.

sulliwane commented 6 years ago

Many thanks @danfinlay @lazaridiscom for your feedbacks.

I'm surprised no other frontend guys did raise this question before...

It feels like a terrible user experience to get full reload on each chain change :-(

Our Dapp is made from the ground up to support multiple chain ids.

olaf89 commented 6 years ago

Are there any cases where non-developer would switch between networks on single application? Full reload would be a terrible experience given users would need to use that. As for developers goes, i think refreshing actually saves time compared to debugging app.

danfinlay commented 6 years ago

Are there any cases where non-developer would switch between networks on single application?

Yes, more and more apps are recommending custom networks to their users, and as Plasma chains become more common, this will too. MetaMask needs to more seamlessly support this kind of behavior in the future, but in the meanwhile, user interaction is required for those sites.

stefek99 commented 6 years ago

Are there any cases where non-developer would switch between networks on single application?

Having 28 tabs open ! ! ! ! ! ! ! ! ! !

I hate when switching Metamask there is reloading app somewhere else...


EDIT / UPDATE:

It displays a popup preventing the reload:

        window.onbeforeunload = function(e) {
          var dialogText = 'Screw the MetaMask';
          e.returnValue = dialogText;
          return dialogText;
        };  

image

sulliwane commented 6 years ago

My sentiments exactly

danfinlay commented 6 years ago

This should be simple enough to implement, and our team is already stretched thin, so I'm marking this as bounty-worthy. We may post a bounty on it soon, and welcome community contributions.

stefek99 commented 6 years ago

Another use case, happened to me 1 second ago:

sulliwane commented 6 years ago

I believe this issue is still not resolved. I have latest version of MM plugin, but still always reloading.

bdresser commented 6 years ago

@sulliwane you're correct, this is not in production yet. We want to make sure dapp developers have plenty of time to prepare for this breaking change. Read more here.

We'll roll this out sometime in the next month and will share a specific timeline here when it's solidified. Thanks for being patient!

sulliwane commented 6 years ago

@bdresser got it, thank you

ghost commented 6 years ago

It seem that this is still happening in the new Beta version image This is really annoying as it breaks our flow! Is there a solution to that problem soon?

bdresser commented 6 years ago

hey @Lucas1313 - we haven't rolled this change out to production yet.

Right now, the plan is to include this update as part of our larger breaking change to support EIP 1102, scheduled to launch on Nov. 2. You can read more about 1102 here.

As plans for removing the reload finalize we'll post here and share a detailed rollout plan on Medium as well. Thanks for your patience!

lcdupree commented 5 years ago

Is there clarity on the current behavior in the Beta or when there will be rollout of this change? I'm on Metamask version 5.0.2 and the page-reload-on-network-change behavior appears inconsistent. It seemed like it stopped but when using Metamask with web3js 1.0.0-beta.36 the behavior reappears. I'm trying to ascertain if the behavior is coming from Metamask or from web3js 1.0.0-beta. The Metamask injected web3js 0.20.3 doesn't cause an auto-reload on network change on my installation.

I cannot find any reference to window.location or location.reload in any of the web3js 1.0.0-beta code so I'm concerned this is some interaction with Metamask causing the page refresh.

Apologies for posting to a closed issue but there didn't seem to be any notes on final the disposition...

bdresser commented 5 years ago

@lcdupree we still haven't rolled this change out to production yet. After EIP 1102 was finalized, we prioritized messaging & implementation of that breaking change, since its effects on the ecosystem's privacy layer are pretty significant.

We still hope to make this change as soon as we have the bandwidth to message & launch smoothly.

lcdupree commented 5 years ago

Thanks for the response, @bdresser. I'm still a bit confused about the current behavior as outlined in my original comment. By default, without importing a newer web3js library (using only the injected 0.20.3 instead of importing 1.0.0-beta36), the refresh behavior does not take place. Can you clarify?

Also, should this issue remain open until this ships? I do not see a good place to track progress of this feature or it's release state. Information is spread across several places and is pretty stale (for ex: Dan Finlay's blog post back in June which implies it's already shipped https://medium.com/metamask/breaking-change-no-longer-reloading-pages-on-network-change-4a3e1fd2f5e7).

hevans90 commented 5 years ago

My team and I have been watching this thread in anticipation - what's the latest on this?

Forced page reloads are a UX nightmare :(

bdresser commented 5 years ago

Hey all, sorry for the lack of clarity here. Reopening this issue, since removing the reload was reverted in https://github.com/MetaMask/metamask-extension/pull/4585.

@lcdupree you're right - it looks like this line isn't called if we detect another window.web3, which might be what you're seeing.

We're going to write some documentation for the on('networkChanged') event (tracked in https://github.com/MetaMask/metamask-extension/issues/4161), then we'll revisit this change. Thanks for your patience.

danfinlay commented 5 years ago

I've just merged #6330, I think it will be of interest to everyone in this thread. It may represent a new feature worthy of closing this issue!

Will probably be pushing to production early next week.

anymos commented 5 years ago

Hi there any good news regarding this issue?

joshstevens19 commented 5 years ago

Hey @anymos my PR has been merged here - https://github.com/MetaMask/metamask-extension/pull/6330 and you can get docs in how it works here:

https://metamask.github.io/metamask-docs/API_Reference/Ethereum_Provider#ethereum.autorefreshonnetworkchange

anymos commented 5 years ago

Wouah! that was the quickest answer ever!, can not believe it seriously thank you a lot!

anymos commented 5 years ago

Do you know when it s going to be released?

joshstevens19 commented 5 years ago

@anymos It's what happens when your company uses github, your online when your working 😄 . Yes it has been released for a while now - https://github.com/MetaMask/metamask-extension/releases/tag/v6.3.0

anymos commented 5 years ago

hahaha, awesome, was kind of used to the answer quickness on other github and that one was surprising :) keep it up that's great !

anymos commented 5 years ago

That s strange I still experiment the page reload despite using 6.4.1 metamask plugin.

Is there something I can check on my side to see what I might doing wrong?

joshstevens19 commented 5 years ago

As explained in the docs you need to do:

window.ethereum.autoRefreshOnNetworkChange = false; on page load and then it will not reload. The default behaviour had to stay as some dapps still depend on the refresh to change state.

anymos commented 5 years ago

Arg, shame on me I shall have read more carefully your doc, as it was clearly mentionned.

Regardless thank you so much for your help, it works perfectly once seting up the right flag.

Thank you again!

MicahZoltu commented 5 years ago

This is marked as bounty-worthy but when I tried to submit a PR (#6982) to "fix" the problem it was rejected saying that I should open an issue to discus the topic first. Upon searching for issues first, I came across this which basically (IIU the tags correctly) says to open a PR.

Can we get some clarity on this topic? While the current autoRefreshOnNetworkChange is a reasonable band-aid to buy time, I don't think that it is a good thing to leave in indefinitely. The primary reason is because it encourages dapp developers to continue to follow bad practices (not watching events for network changes), but also because as seen in #6989 this sort of undesirable behavior with a flag to prevent it is easy to accidentally sneak back in when doing other changes.

Generally speaking, I really don't think that auto-refresh from the plugin should be an option, and the code should be deleted from MetaMask as it is only hurting the Ethereum ecosystem (not helping) and causing more problems the longer it continues to exist (like users above who didn't know they had to tell MetaMask to not do the silly and unexpected thing).

danfinlay commented 5 years ago

Thanks for re-opening this discussion. Making breaking changes is a touchy matter, but we agree that this change needs to be made.

We'll propose a final rollout strategy early next week, not trying to push breaking changes to the ecosystem on a Friday afternoon.

MicahZoltu commented 5 years ago

To be clear, I would be satisfied with just about any "plan" that results in the eventual removal of this flag. Opening a PR and commenting on this issue is an attempt to get a plan into place, but if that plan is "we'll start warning people in 1 month and then in 6 months we'll invert the default and in 1 year we'll remove the flag" then I'm satisfied.

dortort commented 5 years ago

Is there an event/subscription model for network/ account change? A deprecation plan should include a recommendation for a suggested alternative to the deprecated behavior (and the polling approach seems like an anti-pattern IMHO).

danfinlay commented 5 years ago

Yes, we've had an alternative network change event available for a while now, so we are nicely prepared for a transition: https://metamask.github.io/metamask-docs/API_Reference/Ethereum_Provider