vuejs / vue-router

🚦 The official router for Vue 2
http://v3.router.vuejs.org/
MIT License
19.01k stars 5.07k forks source link

empty 'beforeRouteEnter' hook causes component not working #655

Closed Centaur closed 7 years ago

Centaur commented 7 years ago

Vue.js & vue-router.js version

2.0.0-rc.5 & 2.0.0-rc.4

Reproduction Link

https://jsfiddle.net/oldpig/qos7px6j/2/

Steps to reproduce

click /p1 link

What is Expected?

content of component Page1 1 should show up

What is actually happening?

No content appear.

Try uncomment the next ... part in the code, then Page1 comes back to life. If calling next is mandatory (it should not be) there should be somewhere that mention it.

fnlctrl commented 7 years ago

It is mandatory (and quite obviously), what you're doing is like defining a promise that will never resolve (new Promise(resolve => {})).

Centaur commented 7 years ago

Is it really that Obvious? For a user not a developer of vue-router ? @yyx990803

LinusBorg commented 7 years ago

@Centur: Maybe wait for the documentation to be released? 2.0 isn't officially released and documented yet.

Anyway, it should be obvious for anyone who is familiar with async callbacks. Which you should be if you wnt to create a SPA.

fnlctrl commented 7 years ago

@Centaur Then what did you expect otherwise? By using beforeRouteEnter, you are telling the router to wait before entering the route, and it expects you to tell it to continue with a next() call (which is obvious for anyone who has a proper knowledge of javascript callbacks or Promises). Now you give it a handler where next is never called, and expect the router to stop waiting for your permission and magically call next by itself?

Centaur commented 7 years ago

@fnlctrl Where in the name "beforeRouteEnter" imply that I am telling the router to wait? As a hook, in commonsense, it just means I can do something at some certain time. Why can't I choose to do something as simple as console.log('before route enter') ? So you are still speaking as a developer that knows everything. @LinusBorg You should really wait for the documentation to be released before marking the code as RC , or tell 1.x users not to try upgrading your project to 2.x before the final release. Either me or you have the incorrect understanding of RC.

LinusBorg commented 7 years ago

The hook itself is meant to run before the component is shown, agreed? So the component can only be shown when the hook resolves

The example in the release notes explicitly talks about how you can use this hook to asynchronously fetch some data, so the hook has to run asynchonously, and the example shows component will not be rendered until you call next().

Now, of course an unexperienced user could think "If I don't do something async, I don't have to call next(), I can see how that could happen. (While for experienced users, the presence of a a callback called next should be enough).

But RC versions are not meant to be used by unexperienced users. They shoul wait for the documentation and the official release

fnlctrl commented 7 years ago

Upgrading to RC is your own choice. It is common sense that RC is not meant for production.

And by using beforeRouteEnter, it is expected that you've read documentation about it and knows how it works (If you don't know how it works and decide to use it anyway then it's not our responsibility). Since the 2.0 docs are not finished yet, the only place you could have learned about its existence is the release notes which you obviously didn't read carefully.

In addition, all route hook functions in 2.0 share the same simplified signature: guard (toRoute, redirect, next) { // call redirect to redirect to another route // call next to confirm current route // or do nothing to cancel the navigation }

Now, is that note clear enough? call next to confirm current route or do nothing to cancel the navigation

And again, it's not our responsibility if you choose to use something that you don't actually understand how it works.

Centaur commented 7 years ago

So that concludes your attitude towards the community.

LinusBorg commented 7 years ago

Okay, everyone cooll down, this dicscussion got unnessessarily heated.

@Centaur, I apologize that the responses from me and @fnlctrl have been quite snappy and clearly not as professional as they should have been. Please understand that we are humans too, and sometimes stress and other influences allow that emotions get the best of us (which is not an excuse, just an explanation) - although we thrive to not let that happen. If you look at other issues, I think you will find that we succeed in having a professional discourse most of the time.

I hope you give us another chance here.

Now, about the topic at hand:

What is an RC?

We might indeed have a different understanding of what a Release Candidate is. We see it as a beta version that is feature complete and should not introduce any breaking changes in the API unless nessessary to fix bugs. We don't see complete documentation as nessessary to publish a RC.

The RC is not guaranteed to have no more errors, nor is it meant to be adopted by every user. It's still a test-phase version, meant for early adopters, and for those, documentation can be expected to be sub-par while the priority is on getting the codebase ready for release.

You might have a different understanding and that is fine. All we can do is ask to understand our position.

Is current documentation sufficient?

As @fnlctrl showed, the release notes do document how all hooks work, so the information is there.

I admit that this documentation is sparse and not easy to follow as it is spread over multiple -rc.* release notes, but the information is there.

Also, all examples in the next branch are fully functional with the latest RC and throughly documented.

We are also working on getting full documentation up until official release, see this branch. (If you see room for improvement, you can open an issue or provide a PR to help out anytime, we welcome any contribution).

If the sparse documentation that is currently in place is not enough for you, we are sorry about that, but all we can do is ask you to wait until we get the full documentation finished, and to ask any questions you might have on our forum or gitter chat.

Once again, apologies for how this went so far, I hope we can start over.

Centaur commented 7 years ago

@LinusBorg Yes. The information is there. I did read the release note of rc.4, several times actually, but not the beta.1 one containing 'or do nothing to cancel the navigation'. So I agree that this issue is a non-issue and should be closed.

What made me feel offended is the "quite obvious" argument. Those lines were buried in a very long page and did not appear in the body but in some greyed out comments. So it is not that obvious after all. I brought up this issue to help, not to troll. So if I overlooked something, you can just close the issue and point me where I should look into, especially when I had said that "there should be somewhere that mention it" in my original post, instead of throwing "It's quite obvious" at my face, making me feel I have done something wrong. Again, it is a design choice, an implementation detail, not a natural one, which may be obvious to those who already know it, but not to everyone. Still, I don't think "do nothing to cancel the navigation" is a good api design. I don't mean my opinion is the right one, but your attitude and the unbelievable addiction to closing issue made the possibility of further thinking and discussion disappear.

I was excited when vue 2.0 went into the rc cycle and started upgrading my 1.x project a couple of weeks ago. I am aware of the lack of documentation and have gathered knowledge little by little, from here and there. Now I am 100% finished, have made some tiny contributions to vue and vue-router, and only encountered this issue when doing some performance tuning. So optimistically I have categorized myself into "the early adopters", though apparently you and I have different understanding (again) of "the early adopters". My point is, since you cannot provide enough documentation in rc version, you should show more patience to those trying the new version. I don't see any good in limiting RC test among just a few core team members. BTW, I never said I am using 2.0 in production.

Admitting that "I have done something wrong, or at least something not so alright" is the most difficult thing to do. So I suggest we turn this page over and focus on making vuejs ecosystem better together.

LinusBorg commented 7 years ago

What made me feel offended is the "quite obvious" argument. Those lines were buried in a very long page and did not appear in the body but in some greyed out comments. So it is not that obvious after all.

That was indeed uncalled for. It may be obvious to us, familiar with the library, and maybe obvious to "top coders" (amongst which I definitely do not see myself...), but not nessessarily obvious to the normal user, early adopter or not.

My point is, since you cannot provide enough documentation in rc version, you should show more patience to those trying the new version.

Agreed. We will.

Admitting that "I have done something wrong, or at least something not so alright" is the most difficult thing to do. So I suggest we turn this page over and focus on making vuejs ecosystem better together.

It is, thank you for accepting.

yyx990803 commented 7 years ago

@Centaur as a side note, it is as important for users like you to be understanding and less confrontational when you receive an answer that you don't like. We should all be nice to each other, not just us to you.