core icon indicating copy to clipboard operation
core copied to clipboard

Safer isAsyncWrapper check (fix #4002)

Open walmon opened this issue 2 years ago β€’ 9 comments

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:

  • https://github.com/vuejs/core/issues/4002
  • https://github.com/vuejs/core/issues/5040

walmon avatar Oct 18 '22 18:10 walmon

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.

tiehm avatar Dec 13 '22 17:12 tiehm

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?

LinusBorg avatar Dec 14 '22 13:12 LinusBorg

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

sxzz avatar Dec 17 '22 13:12 sxzz

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...

yc-huang avatar Feb 02 '23 03:02 yc-huang

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

DmitryTar1 avatar Feb 14 '23 16:02 DmitryTar1

Also having this issue during vue3 migration

off-border avatar Jun 14 '23 08:06 off-border

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?

niksy avatar Jun 16 '23 13:06 niksy

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?

alinnert avatar Sep 21 '23 13:09 alinnert