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.

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

fix(teleport): ensure correct rendering when target is empty #9783

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

Conversation

baiwusanyu-c
Copy link
Member

@baiwusanyu-c baiwusanyu-c commented Dec 8, 2023

close: #9782
close: #8146

Copy link

github-actions bot commented Dec 8, 2023

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 89.5 kB (+4 B) 34.1 kB (+3 B) 30.7 kB (+47 B)
vue.global.prod.js 147 kB (+4 B) 53.3 kB (+2 B) 47.7 kB (-25 B)

Usages

Name Size Gzip Brotli
createApp 49.8 kB 19.5 kB 17.8 kB
createSSRApp 53.1 kB 20.8 kB 19 kB
defineCustomElement 52.1 kB 20.2 kB 18.5 kB
overall 63.3 kB 24.4 kB 22.3 kB

Copy link

codspeed-hq bot commented Dec 8, 2023

CodSpeed Performance Report

Merging #9783 will not alter performance

Comparing baiwusanyu-c:bwsy/fix/teleportEmptyTarget (42dee55) with main (9fa8241)

Summary

✅ 53 untouched benchmarks

Copy link

netlify bot commented Dec 11, 2023

Deploy Preview for vue-sfc-playground failed.

Name Link
🔨 Latest commit d5fe8ba
🔍 Latest deploy log https://app.netlify.com/sites/vue-sfc-playground/deploys/659d419edfe77e0008231bff

Copy link

netlify bot commented Jan 9, 2024

Deploy Preview for vue-next-template-explorer failed.

Name Link
🔨 Latest commit d5fe8ba
🔍 Latest deploy log https://app.netlify.com/sites/vue-next-template-explorer/deploys/659d419eb7ff7f0008a5b31e

@xvaara
Copy link

xvaara commented May 23, 2024

@yyx990803 could we have this merged? It's weird that you have to have have an "or" in the to prop:

<Teleport :to="props.teleportTo || 'body'" :disabled="!props.teleportTo">

if you forget, the ssr doesn't render the content anywhere.

@skirtles-code
Copy link
Contributor

@xvaara I'm not sure exactly what problem you're having, but based on the example you gave I don't think this PR will fix it. This change only impacts cases where disabled is false and the to is null/undefined, which doesn't seem to be what you're describing.

@xvaara
Copy link

xvaara commented Jun 3, 2024

@xvaara I'm not sure exactly what problem you're having, but based on the example you gave I don't think this PR will fix it. This change only impacts cases where disabled is false and the to is null/undefined, which doesn't seem to be what you're describing.

@skirtles-code
That is exactly the case I'm describing and we are having. Maybe my example was missing the fact that teleportTo is undefined, but I though that was given in this context.

We created out own internal teleport component that basically has a render function:

 return () =>
      props.disabled || !props.to
        ? slots.default?.()
        : h(Teleport, {to: props.to}, [slots.default?.()])

That way we don't have hydration mismatches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants