Skip to content

Safer isAsyncWrapper check (fix #4002) #6908

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

walmon
Copy link

@walmon walmon commented Oct 18, 2022

I've found scenarios when it's recursively onmounting components and face one that has type empty so isAsyncWrapper breaks the whole app instead of just returning false.

Cannot provide a live demo since it's hard to replicate the scenarios where this happens as seen in many reports like:

It breaks if the vnode passed has type empty, it should not break and just return `false` for isAsyncWrapper
@tiehm
Copy link

tiehm commented Dec 13, 2022

Hi! Would it be possible to merge this PR? I currently have the same issue and this would really help me out as it completely fixes the issue.

@LinusBorg
Copy link
Member

LinusBorg commented Dec 14, 2022

All vnodes should have a type property, it's a mandatory field. So I'm worried that simply "working around" a case where a vnode has an undefined type, might mask the true source of the issue - especially as we still don't have a proper reproduction for us to debug, even though the issue came up multiple times so far.

@sxzz What do you think about adding a dev warning when we encounter a malformed vnode, logging some context info? That way, the app may still gracefully recover, but users can report here in a new issue with maybe better information?

@sxzz
Copy link
Member

sxzz commented Dec 17, 2022

@LinusBorg Sure, I think so. It's a better solution.

@yc-huang
Copy link

yc-huang commented Feb 2, 2023

I have a page which use vxetable(https://vxetable.cn/v3) to display a large table. Everything works fine in dev mode, but in production it will trigger this error (type is null when calling isAsyncWrapper) when scrolling between table columns, and this fix seems to work fine for my use case...

@DmitryTar1
Copy link

@LinusBorg @yc-huang Is there any updates? Still have this issue and it stops from continue with vue3 migration.

@off-border
Copy link

Also having this issue during vue3 migration

@niksy
Copy link

niksy commented Jun 16, 2023

Also having the same issue and it’s hard to replicate, but applying these changes fixes the problem. What can we do to get this merged and ready for next release?

@alinnert
Copy link

I'm having the exact same issue as mentioned in #5040 which was linked in this PR. I'm not sure if the change in this PR would solve this exact issue because that error is thrown at a different location.

My problem (+ that in #5040) is that the parameter instance in unmountComponent is null (here). I just saw in the source code that unmountComponent gets called with vnode.component! as the argument that turns out to be null (here). So, using ! and not handling null here is causing the problem in this case.

Now, this still doesn't help me figure out what exactly causes vnode.component to be null in the first place. vnode.type hints to a custom component I've built. But replacing its content with something minimal doesn't solve the problem so the component itself doesn't seem to be the problem.

I'd just like to understand: Is Vue trying to unmount a component that has already been destroyed? Is that what's going on here?

@edison1105
Copy link
Member

This change looks simple, but we still need a minimal reproducible demo to confirm the exact scenario where the error occurs.

@edison1105 edison1105 added the need more info Further information is requested label Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need more info Further information is requested
Projects
Status: Needs Review
Development

Successfully merging this pull request may close these issues.