Skip to content

Call next's callback in beforeRouteUpdate - #2054

Open
thatguystone wants to merge 3 commits into
vuejs:devfrom
thatguystone:dev
Open

Call next's callback in beforeRouteUpdate#2054
thatguystone wants to merge 3 commits into
vuejs:devfrom
thatguystone:dev

Conversation

@thatguystone

Copy link
Copy Markdown

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
Copy Markdown
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

posva commented Feb 12, 2018

Copy link
Copy Markdown
Member

I launched a rebuild

@posva posva left a comment

Copy link
Copy Markdown
Member

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
Copy Markdown
Author

No problem: added tests and updated en docs.

@thatguystone

Copy link
Copy Markdown
Author

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

@posva posva left a comment

Copy link
Copy Markdown
Member

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
Copy Markdown
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
Copy Markdown
Author

@posva Any thoughts here?

@jpicton

jpicton commented Nov 26, 2018

Copy link
Copy Markdown

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

@thatguystone

Copy link
Copy Markdown
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