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.

Sign up for GitHub

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

Jump to bottom

beforeEnter fire twice after async next call on Android 4.4 #1508

Closed
xuemengfei opened this issue Jun 14, 2017 · 9 comments
Closed

beforeEnter fire twice after async next call on Android 4.4 #1508

xuemengfei opened this issue Jun 14, 2017 · 9 comments
Labels
improvement

Comments

@xuemengfei
Copy link

xuemengfei commented Jun 14, 2017 •
edited
Loading

Version

2.5.3

Reproduction link

https://jsfiddle.net/bLw2tkv1/8/

Steps to reproduce

Android 4.4.
HTML5 mode.
Use async next in beforeEach.

What is expected?

Enter the global beforeEach and page beforeEach once.

What is actually happening?

Enter the global beforeEach and page beforeEach both twice.

Android 5.0 (include) is correct.
@xuemengfei xuemengfei changed the title beforeEnter fire twice after async next call on Android 4.4 beforeEnter fire twice after async next call used HTML5 mode on Android 4.4 Jun 14, 2017
@xuemengfei xuemengfei changed the title beforeEnter fire twice after async next call used HTML5 mode on Android 4.4 beforeEnter fire twice after async next call on Android 4.4 Jun 14, 2017
@xuemengfei
Copy link
Author

A bit like #725

@xuemengfei
Copy link
Author

Finally, have time to dig out what's going on.

In MDN. It says.

Browsers tend to handle the popstate event differently on page load. Chrome (prior to v34) and Safari always emit a popstate event on page load, but Firefox doesn't.

There is also a chromium bug about this.

The popstate event will really fire on page load in old Chrome. And I saw you have already do the SameRouter Check in history/base.js#L100-L107. But this check will only work in synchronous guard.

For my case, async next call in guard.

  1. transitionTo get matched router, then runQueue on that router
  2. router's beforeRouteEnter fired
  3. popstate fired
    1. this.current is START
    2. isSameRoute will return false
  4. transitionTo will get the same router base on step 2's result, so runQueue on the same router
  5. router's beforeRouteEnter fired again
  6. first next fired in beforeRouteEnter
  7. second next fired in beforeRouteEnter
  8. Finally, updateRoute will update this.current

@yyx990803 @posva Do you have time to view this issue?

I will continue to find a way to solve this problem in tomorrow evening.

@posva
Copy link
Member

posva commented Jul 31, 2017

Hello, Thanks a lot for digging it and sharing what you found. I don't have a way to test it so I never tested it 😅.
Android 4.4 being still supported and being around 17% of the market, it's a fix worth looking at

@Jinjiang
Copy link
Member

Jinjiang commented Aug 1, 2017

I have installed a Android 4.4 emulator and reproduced it, Both 'global beforeEach' & 'page beforeEach' alert twice. Problem confirmed.

@Jinjiang
Copy link
Member

Jinjiang commented Aug 1, 2017

I found in my Android 4.4 emulator the popstate event will always be fired on page load. But my safari and other Android device doesn't.

<!DOCTYPE html>
<html>

<head>
<title>Vue Router Example</title>
<meta charset="utf-8">
<script>
window.addEventListener('popstate', function (e) {
  alert('Will be fired automatically in Android 4.4')
});
</script>
</head>

<body>
  hello world
</body>

</html>

@xuemengfei
Copy link
Author

xuemengfei commented Aug 1, 2017 •
edited
Loading

Yes @Jinjiang . In this comment. It's the right behavior by designed. And then it changed.

Maybe we can check window.history.state when popstate fired? I will try this later.

@Jinjiang
Copy link
Member

Jinjiang commented Aug 1, 2017

Gotcha. I am still working on that.

@Jinjiang
Copy link
Member

Jinjiang commented Aug 1, 2017

Now this bug can easily reproduced by just dispatching another popstate event in all modern browsers. @posva

https://jsfiddle.net/bLw2tkv1/11/

p.s. Debugging with Android emulator is really a hard thing 😭

@Jinjiang Jinjiang mentioned this issue Aug 1, 2017
Merged
@xuemengfei
Copy link
Author

@Jinjiang that's worked. 👍

I try to check the window.history.state last night. When the value is null, replaceState. But the test case can't pass. I think your solution is the best way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
improvement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@Jinjiang @posva @xuemengfei