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

Call next's callback in beforeRouteUpdate #2054

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

Conversation

thatguystone
Copy link

From the discussion in #1582. This PR addresses a few important points:

  1. next in beforeRouteEnter and beforeRouteUpdate behaves the same.

  2. When router-view has a key, you can now access the new vm from next, making it possible to update the new component. Example:

  3. All callbacks added by next(cb) are called after navigation has finished, ensuring that torn updates don't happen. Example:

@thatguystone thatguystone changed the title Call next's callback in beforeRouteEnter Call next's callback in beforeRouteUpdate Feb 10, 2018
@thatguystone
Copy link
Author

As for failing tests, I'm having the same issue as #2043: all tests pass locally, and I don't have permissions to re-run the build.

@posva
Copy link
Member

posva commented Feb 12, 2018

I launched a rebuild

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.

Hey, completely forgot about asking it, but could you add one or multiple tests for this, please?

@thatguystone
Copy link
Author

No problem: added tests and updated en docs.

@thatguystone
Copy link
Author

Hey @posva, when you have a chance, could you take a look at this again? Thanks!

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.

Sorry, forgot to specify, but I was thinking about a unit test. Could you replace the test you added to a unit one? You should find an example at api.spec.js. You can add it there, I will reorganise later

@thatguystone
Copy link
Author

I've been looking into making these unit tests, and it's not looking promising. The problem is that Vue doesn't seem to be creating any children components when not in-browser, so none of the hooks on any children routes are being extracted since there's no instance for bindGuard. More concretely:

const app = new Vue({
  render(h) { return h('router-view') },
  router: new Router({
    routes: [
      { path: '/', component: Root 
        children: [
          { path: 'foo', component: Foo },
          { path: 'bar', component: Bar },
      ]}  
    ]
  })
}).$mount()

It looks like $mount() ends up calling vm._update(vm._render()), where _update calls __patch__, but outside of a browser, that's a noop (Vue.prototype.__patch__ = inBrowser ? patch : noop), meaning that we're only left with VNodes and no instances.

Using what's currently in app.spec.js obviously has the same problem: no component instances are actually created, so bindGuard can't extract hooks, and this can't be tested.

Any thoughts on this?

@thatguystone
Copy link
Author

@posva Any thoughts here?

@jpicton
Copy link

jpicton commented Nov 26, 2018

What's the progress on this pull request? Are we waiting for further changes?

@thatguystone
Copy link
Author

@jpicton I'm waiting on feedback for next steps. It's possible to rewrite parts of my tests as unit tests, but as I explained in my previous comment, it doesn't seem possible to test all functionality via unit tests.

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

Successfully merging this pull request may close these issues.

3 participants