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

Redirecting from beforeEnter guard will push state during popstate handling #3381

Closed
fabianeichinger opened this issue Nov 20, 2020 · 2 comments

Comments

@fabianeichinger
Copy link

fabianeichinger commented Nov 20, 2020

Version

3.4.9

Reproduction link

https://jsfiddle.net/jwx6q3hc/4

Steps to reproduce

  1. Create a router with
{
  mode: 'history',
  routes: [
    { path: '/a' },
    { path: '/b', beforeEnter: (to, from, next) => next({ name: 'redirect-target' }) },
    //  { path: '/b', redirect: { name: 'redirect-target' } },
    { name: 'redirect-target', path: '/b' }
  ]
}
  1. Initially navigate to /a
  2. Navigate to /b
  3. Navigate to /a
  4. Hit the browser back button
  5. Hit the browser back button again

What is expected?

After hitting the back button in step 5. we should be back on /a.
The forward button should be enabled.
The history should look like

/a --- pushed by 3.
/b --- pushed by 2.
/a <-- current, pushed by 1.

What is actually happening?

After hitting back in step 5., we stay on /b.
The forward button is disabled, browser history looks like this

/b <-- current, pushed by 4.
/a --- pushed by 1.

I hoped this setup would work for a proposal to simplify some dynamic routing in Vue Storefront (take a look at this gist if you're interested in the usecase and some more details). Essentially I want use beforeEnter as a substitute for redirect where I can do some async requests before returning the redirection target.

The issue here is that calling next with some redirection arguments is always handled by calling router.push or router.replace (in the case of next({ replace: true, ... }). But in this usecase there should be no change to the browser history during popstate handling.

Doing the equivalent redirection with redirect works just fine (though I can't use it for my usecase, as I can't return asynchronously from the redirect function). I've marked this as a bug as from what I understand vuejs/rfcs#150 has the goal that both should work the same way.

@posva
Copy link
Member

posva commented Nov 20, 2020

This is working as expected: next(location) without a replace: true will pushState. You need to use replace: true when doing a popstate in your specific case (which is not yet possible to check natively #1620) but the problem is inherent to using a beforeEnter like a redirect, they are still different because redirect ignores all navigation guards and is always happening. In theory, it should never be in the history so you can never reach it through back/forward buttons. The only case it can be present is with dynamic routing in v4.
But also, having two routes with the exact same path is an antipattern in routing and should be avoided because when directly accessing the route through the URL, the router will always pick the same route which could be different from where you were at

@posva posva closed this as completed Nov 20, 2020
@fabianeichinger
Copy link
Author

I'll keep an eye on #1620, conditionally setting the replace: true based on whether it's a popstate navigation seems like a good way to go. Though after reading the Dynamic Routing RFC, I'm not sure if it even makes sense to invest in my solution as the score-based reordering breaks with one of the assumptions (FCFS matching of paths/names of route records) I had for Vue Router's behavior.

Having multiple routes with overlapping path pattern isn't pretty, but something that happens when the data api shall dictate that /a has to be rendered by component X and /b by component Y. I feel like the routing table is the most logical place to encode that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants