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.

Sign up for GitHub

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

Jump to bottom

View Component unnecessarily updates when child route changes #1701

Open
LinusBorg opened this issue Feb 16, 2023 · 7 comments · May be fixed by #2082
Open

View Component unnecessarily updates when child route changes #1701

LinusBorg opened this issue Feb 16, 2023 · 7 comments · May be fixed by #2082
Assignees
@posva
Labels
bug Something isn't working

Comments

@LinusBorg
Copy link
Member

Version

4.1.6

Reproduction link

codesandbox.io

Steps to reproduce

  1. Open linked CodeSandbox
  2. Observe Console
  3. Click on "Home - A", and then on "Home - B"

What is expected?

Should not log at all, because the component has receives no props and does not depend on any reactive data from the router.

So when a child route changes, it should not update.

What is actually happening?

Home.vue console.logs 3 times, meaning it re-rendered multiple times.

I'm pretty sure the reason for that is that router-view needs to re-render multiple times, and each time, a new onVNodeUnmounted callback is added to the VieComponent vnode's props object here:

router/packages/router/src/RouterView.ts

Lines 160 to 170 in e008551

const onVnodeUnmounted: VNodeProps['onVnodeUnmounted'] = vnode => {
// remove the instance reference to prevent leak
if (vnode.component!.isUnmounted) {
matchedRoute.instances[currentName] = null
}
}
const component = h(
ViewComponent,
assign({}, routeProps, attrs, {
onVnodeUnmounted,

So from the POV of Vue, the props changed. So it will run an update cycle of the router view.
@LinusBorg LinusBorg changed the title View Component unnesseassarily update when child route changes View Component unnesseassarily updates when child route changes Feb 16, 2023
@LinusBorg
Copy link
Member Author

Aside from investigating how to fix this in the router, I think we could also look into optimizing this in Vue core. I think the renderer picks the vnode hooks directly from the vnode props when it needs to run them, so changing them should not require any processing/updating of the component itself.

@LinusBorg LinusBorg changed the title View Component unnesseassarily updates when child route changes View Component unnecessarily updates when child route changes Feb 16, 2023
@posva
Copy link
Member

posva commented Feb 16, 2023

did you try moving the onVnodeUnmounted() outside of the render function to fix the problem? Maybe it just needs that

@LinusBorg
Copy link
Member Author

I want to experiment with that, but I see potential problems in as that function has a closure over currentName and matchedRoute, which we would then also have to move out of the render function, and I'm worried we could run into timing issues where these change too fast?

Might not be a problem but needs thorough testing.

Couldn't we also just do that in the watch, though? like, check wether the oldInstance is unmounted (maybe after a tick)? or does that not cover all cases?

Need to test.

@posva
Copy link
Member

posva commented Feb 16, 2023

It's true having the closure avoided timing issues. Having a watcher didn't cover all cases if I remember well but I think I added tests so they should fail if the change is not enough

@LinusBorg
Copy link
Member Author

I'll give it a shot tomorrow

@posva posva added the bug Something isn't working label Dec 15, 2023
@posva
Copy link
Member

posva commented Dec 15, 2023

I think I found a way of fixing this. It still rerenders when using a <Transition /> component but it looks like a Vue behavior. I need to check that further.

posva added a commit that referenced this issue Dec 15, 2023
@posva
perf(RouterView): avoid parent rerenders when possible
1191594 
Close #1701
@posva posva linked a pull request Dec 15, 2023 that will close this issue
Open
@posva posva self-assigned this Dec 15, 2023
@kirkbushell
Copy link

I think I found a way of fixing this. It still rerenders when using a <Transition /> component but it looks like a Vue behavior. I need to check that further.

Did you end up figuring out what was going on here? I have the exact same problem - I have transition "appear" around my RouteView, and always having parents re-render (and re-fetch data) when it's unnecessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@posva posva

Labels
bug Something isn't working
Projects
Vue Router Roadmap
Status: Planned
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

perf(RouterView): avoid parent rerenders when possible vuejs/router
3 participants
@kirkbushell @posva @LinusBorg