Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop reloading dapps on network change #3599

Open
sulliwane opened this issue Mar 16, 2018 · 36 comments
Open

Stop reloading dapps on network change #3599

sulliwane opened this issue Mar 16, 2018 · 36 comments

Comments

@sulliwane
Copy link

@sulliwane sulliwane commented Mar 16, 2018

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

@Zanibas
Copy link
Contributor

@Zanibas Zanibas commented Mar 19, 2018

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
Copy link
Author

@sulliwane sulliwane commented Mar 19, 2018

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
Copy link
Author

@sulliwane sulliwane commented Mar 26, 2018

Any input on this question? Thanks!

@danfinlay
Copy link
Contributor

@danfinlay danfinlay commented Mar 26, 2018

@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.

@danfinlay danfinlay changed the title Why is browser page refreshing/reloading on chain switch? Stop reloading dapps on network change Mar 26, 2018
@danfinlay danfinlay added this to the Milestone 4 milestone Mar 26, 2018
@danfinlay danfinlay modified the milestones: Sprint 04, Sprint 05 Apr 23, 2018
@sulliwane
Copy link
Author

@sulliwane sulliwane commented May 18, 2018

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
Copy link

@olaf89 olaf89 commented May 18, 2018

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
Copy link
Contributor

@danfinlay danfinlay commented May 18, 2018

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
Copy link

@stefek99 stefek99 commented Jun 1, 2018

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
Copy link
Author

@sulliwane sulliwane commented Jun 1, 2018

My sentiments exactly

@danfinlay
Copy link
Contributor

@danfinlay danfinlay commented Jun 5, 2018

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
Copy link

@stefek99 stefek99 commented Jun 9, 2018

Another use case, happened to me 1 second ago:

  • Remix.
  • I want to deploy to Ropsten.
  • Switch network...
  • Hah!
  • Back in a day, I was thinking Remix needs refreshing for some reason.
  • Now I know it is Metamask behavior.
  • No reload required...
@sulliwane
Copy link
Author

@sulliwane sulliwane commented Jul 18, 2018

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

@bdresser
Copy link
Contributor

@bdresser bdresser commented Jul 18, 2018

@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
Copy link
Author

@sulliwane sulliwane commented Jul 26, 2018

@bdresser got it, thank you

@Lucas1313
Copy link

@Lucas1313 Lucas1313 commented Sep 18, 2018

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
Copy link
Contributor

@bdresser bdresser commented Sep 19, 2018

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
Copy link

@lcdupree lcdupree commented Nov 13, 2018

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
Copy link
Contributor

@bdresser bdresser commented Nov 14, 2018

@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.

@danfinlay
Copy link
Contributor

@danfinlay danfinlay commented Mar 21, 2019

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
Copy link

@anymos anymos commented Apr 29, 2019

Hi there any good news regarding this issue?

@joshstevens19
Copy link
Contributor

@joshstevens19 joshstevens19 commented Apr 29, 2019

Hey @anymos my PR has been merged here - #6330 and you can get docs in how it works here:

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

@anymos
Copy link

@anymos anymos commented Apr 29, 2019

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

@anymos
Copy link

@anymos anymos commented Apr 29, 2019

Do you know when it s going to be released?

@joshstevens19
Copy link
Contributor

@joshstevens19 joshstevens19 commented Apr 29, 2019

@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
Copy link

@anymos anymos commented Apr 29, 2019

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
Copy link

@anymos anymos commented Apr 29, 2019

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
Copy link
Contributor

@joshstevens19 joshstevens19 commented Apr 29, 2019

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
Copy link

@anymos anymos commented Apr 29, 2019

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
Copy link

@MicahZoltu MicahZoltu commented Aug 9, 2019

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
Copy link
Contributor

@danfinlay danfinlay commented Aug 9, 2019

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.

@danfinlay danfinlay removed the bounty worthy label Aug 9, 2019
@MicahZoltu
Copy link

@MicahZoltu MicahZoltu commented Aug 9, 2019

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
Copy link

@dortort dortort commented Aug 9, 2019

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
Copy link
Contributor

@danfinlay danfinlay commented Aug 9, 2019

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

Gudahtt added a commit to Gudahtt/metamask-extension that referenced this issue Aug 21, 2019
We are soon removing the automatic refresh on network change behavior.
A warning has been added to ensure sites know about this upcoming
change.

Any site that calls `.enable` is advised to either add a
`networkChanged` event handler to reload manually, or set the flag
`autoRefreshOnNetworkChange` to `false` to opt-out of the reload
behavior early.

This warning might be irritating for certain sites, as they might be
indifferent to whether or not the site reloads, and not eager to set a
flag to opt-in early just to silence the warning. However there was no
other obvious way to warning the right people about this change, as
any warning prior to an actual reload would only be seen by the few
people that set their browser console to preserve logs.

Relates to MetaMask#3599
Gudahtt added a commit to Gudahtt/metamask-extension that referenced this issue Aug 21, 2019
We are soon removing the automatic refresh on network change behavior.
A warning has been added to ensure sites know about this upcoming
change.

Any site that calls `.enable` is advised to use a
`networkChanged` event handler to reload manually if they rely upon
that behavior. They are also advised to set the flag
`autoRefreshOnNetworkChange` to `false` to opt-out of the reload
behavior early.

This warning might be irritating for certain sites, as they might be
indifferent to whether or not the site reloads, and not eager to set a
flag to opt-in early just to silence the warning. However there was no
other obvious way to warning the right people about this change, as
any warning prior to an actual reload would only be seen by the few
people that set their browser console to preserve logs.

Relates to MetaMask#3599
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

You can’t perform that action at this time.