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(runtime): CSSVars can work with Teleport #7344

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

Conversation

baiwusanyu-c
Copy link
Member

close: #7312

@baiwusanyu-c baiwusanyu-c changed the title fix(runtime): cssVars can work with Teleport fix(runtime): CSSVars can work with Teleport Dec 14, 2022
}
}
if (isArray(vnode.children)) {
vnode.children.forEach(n => observeTeleportTarget(n as VNode))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

walk vnode tree and observe all teleport node,there may be a extra performance cost
Indeed,we need further consideration

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

walk vnode tree and observe all teleport node,there may be a extra performance cost Indeed,we need further consideration

I made some modifications, but as you said, I also think that doing so will consume a lot of performance.
I think a better approach should be like suspense , which handles the cssvars update on teleport at the right time during the runtime process, but that is a big change

@moushicheng
Copy link
Contributor

see here
bug seem still alive

@baiwusanyu-c
Copy link
Member Author

I store the 'data-owner-value' in the 'parentComponent' when the element is unmounted, and reset it when it is updated. And maintain a variable 'teleportUTMap' globally, store 'ut', re-execute 'ut' when updating,

@edison1105
Copy link
Member

@baiwusanyu-c
great work! but not working with this scenario

@baiwusanyu-c
Copy link
Member Author

@baiwusanyu-c great work! but not working with this scenario

ok,let me see how to fix

@baiwusanyu-c
Copy link
Member Author

Do some refinement and unit testing tomorrow 🤣

@netlify
Copy link

netlify bot commented Dec 16, 2022

Deploy Preview for vuejs-coverage failed.

Name Link
🔨 Latest commit f4a605c
🔍 Latest deploy log https://app.netlify.com/sites/vuejs-coverage/deploys/639c7209abc1860008079a4f

@baiwusanyu-c baiwusanyu-c marked this pull request as draft December 16, 2022 13:29
@baiwusanyu-c baiwusanyu-c marked this pull request as ready for review December 17, 2022 04:45
…SVars

# Conflicts:
#	packages/runtime-dom/__tests__/helpers/useCssVars.spec.ts
@github-actions
Copy link

github-actions bot commented Oct 20, 2023

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 91.3 kB (+675 B) 34.7 kB (+212 B) 31.2 kB (+156 B)
vue.global.prod.js 148 kB (+675 B) 54 kB (+212 B) 48.2 kB (+140 B)

Usages

Name Size Gzip Brotli
createApp 51.1 kB (+345 B) 20 kB (+118 B) 18.2 kB (+90 B)
createSSRApp 54.4 kB (+345 B) 21.3 kB (+103 B) 19.4 kB (+134 B)
defineCustomElement 53.4 kB (+345 B) 20.7 kB (+108 B) 18.9 kB (+140 B)
overall 64.8 kB (+345 B) 25 kB (+102 B) 22.6 kB (+89 B)

# Conflicts:
#	packages/runtime-core/__tests__/components/Suspense.spec.ts
# Conflicts:
#	packages/runtime-dom/__tests__/helpers/useCssVars.spec.ts
Copy link

codspeed-hq bot commented Dec 19, 2023

CodSpeed Performance Report

Merging #7344 will not alter performance

Comparing baiwusanyu-c:bwsy/fix/teleportCSSVars (e3e85d5) with main (bae79dd)

Summary

✅ 53 untouched benchmarks

baiwusanyu-c and others added 4 commits January 3, 2024 11:26
# Conflicts:
#	packages/runtime-core/__tests__/components/Suspense.spec.ts
#	packages/runtime-core/src/components/Teleport.ts
#	packages/runtime-core/src/renderer.ts
#	packages/runtime-dom/__tests__/helpers/useCssVars.spec.ts
#	packages/runtime-dom/src/helpers/useCssVars.ts
…SVars

# Conflicts:
#	packages/runtime-core/__tests__/components/Suspense.spec.ts
@sodatea
Copy link
Member

sodatea commented Apr 2, 2024

/ecosystem-ci run

@vue-bot
Copy link

vue-bot commented Apr 2, 2024

📝 Ran ecosystem CI: Open

suite result latest scheduled
language-tools success success
nuxt failure success
pinia success success
quasar success success
radix-vue success success
router success success
test-utils success success
vant success success
vite-plugin-vue success success
vitepress success success
vue-i18n success success
vue-macros success success
vuetify success success
vueuse success success
vue-simple-compiler success success

@sodatea
Copy link
Member

sodatea commented Apr 2, 2024

Since this PR adds a new export to the @vue/runtime-core package, I think it should be shipped in a minor version.

The Nuxt test failure was due to this. Not a blocker, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Needs Review
Development

Successfully merging this pull request may close these issues.

v-bind style not working in some edge cases (teleport + transition, slots)
5 participants