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

add the replace attribute in location for hooking in beforeEach #1906

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

wanyaxing
Copy link

@wanyaxing wanyaxing commented Nov 26, 2017

Closes #1620

@wanyaxing
Copy link
Author

e...
I should to study something about CircleCI now...
to be continued.

@posva
Copy link
Member

posva commented Dec 6, 2017

Hey, sorry for the delay. What are you trying to achieve? The issue you linked is closed. Why are you always setting replace to true if location is an object?

@wanyaxing
Copy link
Author

If somebody reset the path in beforeEach, the new path will be open as push type, actually, it will be puzzled if the original router path is replace type.

In this PR, the replace setting of location in the $router.replace() will be set as true, so the developer can find the route's type in the beforeEach hooking.

image

@posva
Copy link
Member

posva commented Dec 10, 2017

Aaah, is the issue you're referring to #1620?

@wanyaxing
Copy link
Author

yes.

And the vue-router has supported the replace type checking in the route guards next() in this commit: d53ec3c

This pr is an expand of that commit.

@posva
Copy link
Member

posva commented Dec 10, 2017

ok, could you add some tests to make sure the property is added in different scenarios but not others? (push/replace/calling it with next)

@wanyaxing
Copy link
Author

I have added an example for this commit.
Hopes to be useful.

@luozhihua
Copy link

Is there any progress? I waiting for this PR to be merged for a long time.

luozhihua referenced this pull request in wanyaxing/vue-router May 2, 2018
@choegyumin
Copy link

I am waiting for this PR to be merged too..

@posva
Copy link
Member

posva commented Jun 26, 2018

This will probably come along with other changes that would allow knowing what kind of navigation happened (back, forward, push, replace)

@hxlniada
Copy link

hxlniada commented Aug 4, 2018

@posva is there any plan to add these features? we are waiting for this feature too too too too too long. almost a year.

@reckert
Copy link

reckert commented Apr 8, 2019

Around one year after the last post, I would like to ask if this branch will be merged some day.

Copy link
Member

@posva posva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! There is no need to add a new e2e spec, we can add a test to the existing basic e2e test instead

Comment on lines +58 to +60
if (typeof location === 'object' && !location.replace) {
(location: Object).replace = true
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since location is an object at this point, we can directly set replace to true

Suggested change
if (typeof location === 'object' && !location.replace) {
(location: Object).replace = true
}
(location: Object).replace = true

@@ -52,10 +52,13 @@ export function normalizeLocation (
hash = `#${hash}`
}

const replace = next.replace || false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to create a new variable. It can also be written as !!next.replace

@@ -23,6 +23,7 @@ export function createRoute (
meta: (record && record.meta) || {},
path: location.path || '/',
hash: location.hash || '',
replace: location.replace || false,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
replace: location.replace || false,
replace: !!location.replace,

iceprosurface added a commit to iceprosurface/vue-router that referenced this pull request Oct 26, 2021
iceprosurface added a commit to iceprosurface/vue-router that referenced this pull request Oct 26, 2021
iceprosurface added a commit to iceprosurface/vue-router that referenced this pull request Oct 26, 2021
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

Successfully merging this pull request may close these issues.

ability to check push vs replace in navigation guard
6 participants