-
Notifications
You must be signed in to change notification settings - Fork 21
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
feat(display-emitter): support Vue3 to the component #1555
Conversation
…display-emitter-vue3 # Conflicts: # packages/_vue3-migration-test/src/components/index.ts # packages/_vue3-migration-test/src/main.ts # packages/_vue3-migration-test/src/router.ts
}); | ||
|
||
onUnmounted(unwatchDisplay); | ||
onUnmounted(() => { | ||
unwatchDisplay(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unwatchDisplay(); | |
unwatchDisplay && unwatchDisplay(); |
I guess this will not happen but there's a chance that unwatchDisplay is undefined now, so I think we should just check for it before invoking it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code LGTM, tested locally and it works just fine.
…display-emitter-vue3 # Conflicts: # packages/_vue3-migration-test/src/router.ts
Support Vue3 to the
DisplayEmitter
component becuase the same issue we already saw with theMainScroll
#1552.A template ref over a slot (AKA
NoElement
) in vue3 renders aFragment
and the ref is lost, but in vue2 renders the firstvNode
of the slot and assign the ref in a right way.