-
-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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-dom): before update should also set css vars #11561
Conversation
Size ReportBundles
Usages
|
@@ -47,6 +48,10 @@ export function useCssVars(getter: (ctx: any) => Record<string, string>) { | |||
watchPostEffect(setVars) | |||
}) | |||
|
|||
onBeforeUpdate(() => { | |||
watchPostEffect(setVars) |
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.
Should just call setVars
and not create another watcher on each update.
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.
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.
or use queuePostFlushCb
instead?
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.
thanks
Maybe we should add corresponding test cases to prevent regression. |
Of course, I'll add the necessary test cases to prevent any regression. |
If I want to reproduce this issue through single test, it seems that I can only do it through e2e test, and simple unit test does not seem to be able to reproduce it. |
@linzhe141 // useCssVars.spec.ts
test('with delay mount child', async () => {
const state = reactive({ color: 'red' })
const value = ref(false)
const root = document.createElement('div')
const Child = {
setup(){
onMounted(()=>{
const childEl = root.children[0]
expect(getComputedStyle(childEl!).getPropertyValue(`--color`)).toBe(`red`)
})
return () => h('div',{id:'childId'})
}
}
const App = {
setup() {
useCssVars(() => state)
return () => value.value ? h(Child) :[h('span')]
},
}
render(h(App), root)
await nextTick()
// css vars use with fallback tree
for (const c of [].slice.call(root.children as any)) {
expect((c as HTMLElement).style.getPropertyValue(`--color`)).toBe(`red`)
}
// mount child
value.value = true
await nextTick()
for (const c of [].slice.call(root.children as any)) {
expect((c as HTMLElement).style.getPropertyValue(`--color`)).toBe(`red`)
}
}) |
thanks!!!!!! |
@vue/compiler-core
@vue/compiler-dom
@vue/compiler-ssr
@vue/compiler-sfc
@vue/reactivity
@vue/runtime-core
@vue/runtime-dom
@vue/server-renderer
@vue/shared
vue
@vue/compat
commit: |
This actually seems to be fixing a difference case from #11533 #11533 is fixed by 2d5c5e2, but this PR is also a legit fix for the different test case it demonstrated. |
before update should also set css vars
this pr playground case1
this pr playground case2