-
-
Notifications
You must be signed in to change notification settings - Fork 5k
Simplify transition hooks #321
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
Comments
I think one of the nice things about hooks like canActivate is that intent is expressed nicely in the hook name. An example of this is checking authentication status. I think being explicit like this has some value for readability. |
@chenkie as a general rule of thumb explicit naming is useful and a good practice, though I think in this context @yyx990803's proposal is a good start. One of the things that attracted myself and others to Vue was how its simple API is small enough for you to keep a mental model of it in your mind. A minimized set of transition hooks with more intuitive names will help maintain this trait of Vue. For example, I found the Vue instance lifecycle hooks to be very self-explanatory and along with the provided diagram I was able to have mental model of how everything worked. When first reading through the docs, I found the name of the Now, having more hooks does provide more granular control within the transition pipeline, though I don't particularly see a loss of functionality with the proposed 3 hooks. So a +1 for the proposed hook names. Have to do some thinking about the async stuff. |
👍 I like the simplification! |
I like that the hooks are EXPLICIT (previous). There are only 6 hooks to get started with, each one with clear intent & meaning (maybe not so on +1 for the hook renames, they seem more explicit (at least for me) |
The reason why I think canActivate (transition) {
if (!loggedIn) {
transition.redirect('/login')
} else {
transition.next()
}
} Now, doing the same thing in |
Are there any common use cases that would require the need for If no, then there is sufficient reason to adopt the proposal. I think it's a good goal to enhance the surface API to accommodate the 99% of use cases. |
love the proposal! |
|
There's at least one use case that i can think for reuse: when you need to prevent deactivate changing to/from a same route with different params See the example below. It prevents to change route when Person is dirty. Removing |
How drastic are the changes to the underlying code? Would it be possible to provide deprecation warnings for the old hooks before completely removing them?
|
@blikblum yeah, keeping @rpkilby we sure can do that, although as a |
@yyx990803 Fine. |
@yyx990803 that's a fair statement about compatibility guarantees. In this case, it's more about prompting the community for feedback before the changes are finalized. Users would still be able to use 0.8.x while discussing whether or not there are any use cases that justify keeping around the canActivate/canDeactivate hooks. |
Hi, regarding the Or taking this even further (than the fade in/out effect) - in my case I was experimenting with dynamic routes, e.g. picture a CMS where a user can add new pages with custom slugs. To go with one request only I fetch my data within ...so without |
Closing as this is now outdated and most things are implemented with a different naming. |
Here's what I think of the current API:
canActivate
andcanDeactivate
? It seemsactivate
anddeactivate
can already handle most use cases withtransition.abort()
andtransition.redirect()
.data
hook name is not that self-explanatory.Idea
Some potential breaking changes for 0.8.0:
canActivate
andcanDeactivate
.canReuse
. The same component is always reusable. Handle route change inonChange
.onEnter
: whatactivate
used to be.onLeave
: whatdeactivate
used to be.onChange
: whatdata
used to be. The new name better explains when it is called (on every route change when the current component is active)onEnter
andonLeave
in the route config as well.The text was updated successfully, but these errors were encountered: