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(css-vars): nullish v-bind in style should not lead to unexpected inheritance #12461

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -884,9 +884,9 @@ export default {

return (_ctx, _push, _parent, _attrs) => {
const _cssVars = { style: {
"--xxxxxxxx-count": (count.value),
"--xxxxxxxx-style\\\\.color": (style.color),
"--xxxxxxxx-height\\\\ \\\\+\\\\ \\\\\\"px\\\\\\"": (height.value + "px")
":--xxxxxxxx-count": (count.value),
":--xxxxxxxx-style\\\\.color": (style.color),
":--xxxxxxxx-height\\\\ \\\\+\\\\ \\\\\\"px\\\\\\"": (height.value + "px")
}}
_push(\`<!--[--><div\${
_ssrRenderAttrs(_cssVars)
Expand Down
6 changes: 3 additions & 3 deletions packages/compiler-sfc/__tests__/compileScript.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -646,10 +646,10 @@ describe('SFC compile <script setup>', () => {
expect(content).toMatch(`return (_ctx, _push`)
expect(content).toMatch(`ssrInterpolate`)
expect(content).not.toMatch(`useCssVars`)
expect(content).toMatch(`"--${mockId}-count": (count.value)`)
expect(content).toMatch(`"--${mockId}-style\\\\.color": (style.color)`)
expect(content).toMatch(`":--${mockId}-count": (count.value)`)
expect(content).toMatch(`":--${mockId}-style\\\\.color": (style.color)`)
expect(content).toMatch(
`"--${mockId}-height\\\\ \\\\+\\\\ \\\\\\"px\\\\\\"": (height.value + "px")`,
`":--${mockId}-height\\\\ \\\\+\\\\ \\\\\\"px\\\\\\"": (height.value + "px")`,
)
assertCode(content)
})
Expand Down
7 changes: 6 additions & 1 deletion packages/compiler-sfc/src/style/cssVars.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,12 @@ export function genCssVarsFromList(
return `{\n ${vars
.map(
key =>
`"${isSSR ? `--` : ``}${genVarName(id, key, isProd, isSSR)}": (${key})`,
// The `:` prefix here is used in `ssrRenderStyle` to distinguish whether
// a custom property comes from `ssrCssVars`. If it does, we need to reset
// its value to `initial` on the component instance to avoid unintentionally
// inheriting the same property value from a different instance of the same
// component in the outer scope.
`"${isSSR ? `:--` : ``}${genVarName(id, key, isProd, isSSR)}": (${key})`,
)
.join(',\n ')}\n}`
}
Expand Down
4 changes: 2 additions & 2 deletions packages/runtime-core/src/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -582,13 +582,13 @@ export interface ComponentInternalInstance {
* For updating css vars on contained teleports
* @internal
*/
ut?: (vars?: Record<string, string>) => void
ut?: (vars?: Record<string, unknown>) => void

/**
* dev only. For style v-bind hydration mismatch checks
* @internal
*/
getCssVars?: () => Record<string, string>
getCssVars?: () => Record<string, unknown>

/**
* v2 compat only, for caching mutated $options
Expand Down
6 changes: 2 additions & 4 deletions packages/runtime-core/src/hydration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -938,10 +938,8 @@ function resolveCssVars(
) {
const cssVars = instance.getCssVars()
for (const key in cssVars) {
expectedMap.set(
`--${getEscapedCssVarName(key, false)}`,
String(cssVars[key]),
)
const value = String(cssVars[key] ?? 'initial')
expectedMap.set(`--${getEscapedCssVarName(key, false)}`, value)
}
}
if (vnode === root && instance.parent) {
Expand Down
19 changes: 19 additions & 0 deletions packages/runtime-dom/__tests__/helpers/useCssVars.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -465,4 +465,23 @@ describe('useCssVars', () => {
render(h(App), root)
expect(colorInOnMount).toBe(`red`)
})

test('should set vars as `initial` for nullish values', async () => {
const state = reactive<Record<string, unknown>>({
color: undefined,
size: null,
})
const root = document.createElement('div')
const App = {
setup() {
useCssVars(() => state)
return () => h('div')
},
}
render(h(App), root)
await nextTick()
const style = (root.children[0] as HTMLElement).style
expect(style.getPropertyValue(`--color`)).toBe('initial')
expect(style.getPropertyValue(`--size`)).toBe('initial')
})
})
13 changes: 8 additions & 5 deletions packages/runtime-dom/src/helpers/useCssVars.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ export const CSS_VAR_TEXT: unique symbol = Symbol(__DEV__ ? 'CSS_VAR_TEXT' : '')
* Runtime helper for SFC's CSS variable injection feature.
* @private
*/
export function useCssVars(getter: (ctx: any) => Record<string, string>): void {
export function useCssVars(
getter: (ctx: any) => Record<string, unknown>,
): void {
if (!__BROWSER__ && !__TEST__) return

const instance = getCurrentInstance()
Expand Down Expand Up @@ -64,7 +66,7 @@ export function useCssVars(getter: (ctx: any) => Record<string, string>): void {
})
}

function setVarsOnVNode(vnode: VNode, vars: Record<string, string>) {
function setVarsOnVNode(vnode: VNode, vars: Record<string, unknown>) {
if (__FEATURE_SUSPENSE__ && vnode.shapeFlag & ShapeFlags.SUSPENSE) {
const suspense = vnode.suspense!
vnode = suspense.activeBranch!
Expand Down Expand Up @@ -94,13 +96,14 @@ function setVarsOnVNode(vnode: VNode, vars: Record<string, string>) {
}
}

function setVarsOnNode(el: Node, vars: Record<string, string>) {
function setVarsOnNode(el: Node, vars: Record<string, unknown>) {
if (el.nodeType === 1) {
const style = (el as HTMLElement).style
let cssText = ''
for (const key in vars) {
style.setProperty(`--${key}`, vars[key])
cssText += `--${key}: ${vars[key]};`
const value = String(vars[key] ?? 'initial')
style.setProperty(`--${key}`, value)
cssText += `--${key}: ${value};`
}
;(style as any)[CSS_VAR_TEXT] = cssText
}
Expand Down
11 changes: 11 additions & 0 deletions packages/server-renderer/__tests__/ssrRenderAttrs.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,4 +203,15 @@ describe('ssr: renderStyle', () => {
}),
).toBe(`color:&quot;&gt;&lt;script;`)
})

test('useCssVars handling', () => {
expect(
ssrRenderStyle({
fontSize: null,
':--color': undefined,
':--size': null,
'--foo': 1,
}),
).toBe(`--color:initial;--size:initial;--foo:1;`)
})
})
20 changes: 19 additions & 1 deletion packages/server-renderer/src/helpers/ssrRenderAttrs.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import {
escapeHtml,
isArray,
isObject,
isRenderableAttrValue,
isSVGTag,
stringifyStyle,
Expand Down Expand Up @@ -93,6 +95,22 @@ export function ssrRenderStyle(raw: unknown): string {
if (isString(raw)) {
return escapeHtml(raw)
}
const styles = normalizeStyle(raw)
const styles = normalizeStyle(ssrResetCssVars(raw))
return escapeHtml(stringifyStyle(styles))
}

function ssrResetCssVars(raw: unknown) {
if (!isArray(raw) && isObject(raw)) {
const res: Record<string, unknown> = {}
for (const key in raw) {
// `:` prefixed keys are coming from `ssrCssVars`
if (key.startsWith(':--')) {
res[key.slice(1)] = raw[key] ?? 'initial'
} else {
res[key] = raw[key]
}
}
return res
}
return raw
}