fix(hydration): resolveCssVars filters non-string and number value #12442
Conversation
@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: |
Size ReportBundles
Usages
|
yangxiuxiu1115
changed the title
There was a problem hiding this comment.
Copilot reviewed 2 out of 2 changed files in this pull request and generated no suggestions.
Comments skipped due to low confidence (1)
packages/runtime-core/tests/hydration.spec.ts:2213
- The variable name 'backGroundColor' is inconsistent with naming conventions. It should be renamed to 'backgroundColor'.
const backGroundColor = ref<any>(null)
| String(cssVars[key]), | ||
| ) | ||
| const value = cssVars[key] | ||
| if (isString(value) || typeof value === 'number') { |
There was a problem hiding this comment.
should also take into account an empty string
see Playground with this PR
There was a problem hiding this comment.
Do we even need to take care of values apart from null and undefined here? Even a string can cause mismatch here. eg. ";"
edison1105
added
🔨 p3-minor-bug
|
The |
| style.setProperty(`--${key}`, vars[key]) | ||
| cssText += `--${key}: ${vars[key]};` | ||
| const value = vars[key] | ||
| if (isRenderableAttrValue(value)) { |
There was a problem hiding this comment.
A potential breaking change might have been introduced here. Previously, the client would render undefined value, effectively setting the rule value to unset. However, now undefined value will not be rendered, causing the rule to inherit value from the parent node. However, this is an edge case.
Notably, this change aligns with SSR behavior.
There was a problem hiding this comment.
Note: I don’t think rendering undefined was proper to render regardless; it was essentially setting the value to a string of ’undefined’ which simply doesn’t map to a value.
This worked because undefined is never a valid value in CSS.
In these scenarios where we want to unset and prevent inheritance, it should actually be set to a value of initial instead, which unsets the value in the current scope.
I was thinking you could use @property syntax to declare the custom property and control inheritance; however, this cannot be defined inline on an element.
There was a problem hiding this comment.
Unfortunately setting the custom property to initial works the same way as an invalid value as it's treated as a string instead of a global CSS value.
edison1105
added
ready to merge
| @@ -904,7 +904,7 @@ function toStyleMap(str: string): Map<string, string> { | |||
| let [key, value] = item.split(':') | |||
| key = key.trim() | |||
| value = value && value.trim() | |||
| if (key && value) { | |||
| if (key && isRenderableAttrValue(value)) { | |||
There was a problem hiding this comment.
I don't think this is necessary, value here is string | undefined.
| String(cssVars[key]), | ||
| ) | ||
| const value = cssVars[key] | ||
| if (isRenderableAttrValue(value)) { |
There was a problem hiding this comment.
And I think it's not semantically correct to use isRenderableAttrValue here as the value isn't to be rendered as a DOM attribute. /cc @edison1105
fix #12439