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

Make START_LOCATION (as defined in /src/util/route.js) accessible through the public API #2718

Closed
cjblomqvist opened this issue Apr 16, 2019 · 7 comments · Fixed by juicyfx/juicy#677, juicyfx/juicy#681 or Netflix/dispatch#961
Labels
contribution welcome fixed on 4.x This issue has been already fixed on the v4 but exists in v3 good first issue
Projects

Comments

@cjblomqvist
Copy link

What problem does this feature solve?

It makes it possible to compare the latest route to START. Now you have to manually compare to ensure it's the same (or access START through the source, which I'd rather not).

Why do I want to compare with START? To check if this is the first routing made. Why would I want to do that? If coming from server-side and auth check fail, I want to replace the current route (START) with the login one, since the browser has already made an entry to the history stack (if I push you can side step the auth guard and actually access the content by pressing the back button). If not the first one (and routing client side) I want to "push" the user into the login view, so that the previous history entry is intact.

What does the proposed API look like?

I'm open for any way of accessing START properly, for example router.START

@posva
Copy link
Member

posva commented Apr 16, 2019

if I push you can side step the auth guard and actually access the content by pressing the back button

I'm not following here, if you navigate during SSR, there won't be an entry in the browser. I don't see exactly the problem you want to solve

@cjblomqvist
Copy link
Author

Scenario A:

  1. User enters mydomain.com/restrictedpath in browser
  2. Browser loads mydomain.com/restrictedpath
    2.1 Browser adds history entry mydomain.com/restrictedpath
  3. Vue is loaded with router
    3.1. beforeEach triggers
    3.2. In beforeEach, the auth check fails for mydomain.com/restrictedpath
    3.3 In order to prevent the user from being able to access mydomain.com/restrictedpath by using the back button, I need to replace the history entry (using router.replace('/login'));

The history will look like: ['login']

Scenario B:

  1. User enters mydomain.com/okpath in browser
  2. Browser loads mydomain.com/okpath
    2.1 Browser adds history entry mydomain.com/okpath
  3. User clicks link to mydomain.com/restrictedpath
  4. Router kicks in and befireEach triggers
    4.1 In beforeEach, the auth check fails for mydomain.com/restrictedpath
    4.2In order to prevent the user from being able to access mydomain.com/restrictedpath I need to push the history entry (using router.push('/login'));

4.2 Will cancel the routing/transition to mydomain.com/restrictedpath, and initiate a new routing/transition to mydomain.com/login instead. History will then contain two entries looking like ['okpath', 'login']

In scenario A, if I use replace, the history will look like ['myrestrictedpath', 'login']

If you think something is setup wrong/not according to best practices (which causes this), then please let me know and I'll close the issue! :)

@posva
Copy link
Member

posva commented Apr 16, 2019

thanks for the detailed explanation!

by push and replace I imagine you meant calling next in a guard, that would be the right way to prevent the user navigation. The thing is you shouldn't be facing any problem

In future versions, it may be possible to detect if there was no previous navigation, it's part of giving providing more information relative to the navigation, but I'm not sure that covers the problem you are describing

@cjblomqvist
Copy link
Author

@posva I actually mean calling router.replace(...) and router.push(...) inside the beforeEach hook. I always call next afterwards.

It seems from the docs this is not the natural way to do it (I wasn't the one setting it up for our current project, so I just assumed he'd used best practice for that). Anyway, I doubt it matters actually, since when I call next the only thing it will do is abort. And doing it using next you'll have to specify if it should push or replace, which basically means it will do the same stuff regardless if I side-step next and user router.replace(...) and router.push(....). I also check the code and it seems almost to be doing the exact stuff (in a little bit different order but otherwise basically the same).

Anyway, the issue is still the same - I want to replace/push based on whether this was the first navigation/routing/transition or not, and it would be nice to be able to do that based on comparing the previous (from) route with START (now I worked around by checking the name and path, but it's not really elegant and also possibly error prone since there could be routing made to something that matches these criterias that isn't START).

@posva
Copy link
Member

posva commented Apr 17, 2019

I actually mean calling router.replace(...) and router.push(...) inside the beforeEach hook. I always call next afterwards.

you should indeed call only next instead of router.replace/push

Matching path and name should be okay, it won't break unless you remove the name from your initial route. The matched property is also an empty array, so if you have an asterisk route, it will be the only route to have an empty matched array

@cjblomqvist
Copy link
Author

cjblomqvist commented Apr 17, 2019

you should indeed call only next instead of router.replace/push

Yepp, but it seems the code already takes care of people like me (or my previous co-worker) who doesn't know how to properly use next :) Anyway, the issue remains...

Matching path and name should be okay, it won't break unless you remove the name from your initial route. The matched property is also an empty array, so if you have an asterisk route, it will be the only route to have an empty matched array

Yepp, but these are all quite hacky and relatively unreliable ways of checking this. As it's such a minor thing to expose START (and I can't think of any downsides), it seems better to expose it in the public API so one can properly check if it's truly the first routing/transitioning or not.

@posva posva added the fixed on 4.x This issue has been already fixed on the v4 but exists in v3 label Apr 1, 2020
@posva posva changed the title Make START (as defined in /src/util/route.js) accessible through the public API Make START_LOCATION (as defined in /src/util/route.js) accessible through the public API May 19, 2020
@posva posva added this to To Do in v3 pre v4 May 19, 2020
@daisuke85a
Copy link

@posva
I want to challenge this. Can you assign me?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment