-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
fix(runtime-dom): fix getContainerType #12148
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
Conversation
Please provide a minimal reproducible demo. This will help us troubleshoot and identify the root cause of the issue. Error logs alone make it difficult for us to pinpoint the exact problem. |
Generally speaking, the exclamation marks are the main problem. I will try to do so as soon as possible, but the minimal reproduction of this one is tough because of app routing and a myriad of things that need to be ported in it, that could be in a way or another leading to this. And, by the way, #7086 this is still broken with latest Vue. The problem is exactly the same, just elsewhere. There's a reproduction repo for 2 years posted now, and noone seems to care enough. I need to know whether the half-working-day of porting our app's internal structure will go to waste or not. |
I think #7086 hasn't been resolved in two years likely because the reproduction demo is too complex. it contains many third-party libraries. This wastes a lot of time for contributors trying to help you find the bug. This is unreasonable because you are most familiar with your codebase, and if you can't extract a minimal reproduction demo, it's even more impossible for others to do so. please see https://antfu.me/posts/why-reproductions-are-required |
I can understand that reproductions are required, I am working in the same field in the end and face the same issues. For #7086 the reproduction demo there is quite simple. Third party libraries are qutie irrelevant. I will then get rid of GraphQL and make a simple For this one, it'll take me a considerable amount of time to copy/pasta the project structure in terms nested transition/suspense/router-view/transition/suspense/keep-alive/suspense etc, to make an accurate representation of the conditions where it does break. And then, it breaks every 5th run of a test case. The test hovers over an component that is async with lazy hydration very quickly. Every now and then, the above happens. |
Getting same error but only in Safari. It’s really hard to reproduce it :/ @wucdbm did you managed to fix this locally? |
@niksy Yes and No. More like no.
This one fixes the error in that it does not happen. But over my debugging sessions I managed to understand that the reason this happens is the first place is an error elsewhere, which somehow gets masked and this one gets to surface. Once you patch this with the above code, that underlying error will still happen, and will also render your app unusable, not responding to anything anyhow, just like if you didn't patch it. We managed to figure out that the issue is with The biggest problem here is that the vue codebase is littered with bullshit code with non-null assertions or whatever the F that evil thing with the exclamation mark is called. So now due to the lack of proper errors in place where this is "not supposed to happen, so we put an Fing And it seems like the whole JS/TS community loves writing teenage code like that that smells from the get-go, so hmpf. What do I know 🤷 . In light of the above (and also, thank you for reminding me about this one), I am closing this as it no longer makes sense. |
I still don’t know how to consistently reproduce the issue in Safari, currently I’ve managed to work around it and hope to return to it in the future or wait for some totally unrelated change in the codebase (or Safari update) fixes the issue 🤷♂️ |
TL;DR async components with lazy hydration break when interacted with very quickly after hydration
The
getContainerType
function does not accept null, however inpackages/runtime-core/src/hydration.ts:281
the following codeconst container = parentNode(node)!
assumescontainer
will not be null, but in fact, it is, and reading a field from it ingetContainerType
fails. I do not understand the need to use the non-null assertion operator throughout the code base of Vue, but I guess sometimes it can be OK for performance reasons.Something else I do not understand is, after fixing
getContainerType
locally, we did not experience any further crashes (for now), and looking at the code,mountComponent
expects to always get a container, but whenever that happens (container = null), and it does happen now and then sincegetContainerType
used to crash in 1 of 5 test runs, no observable malfunctions happen. How canmountComponent
still function properly when it requires a container, but gets passed a null?!