-
-
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(css-vars): nullish v-bind in style should not lead to unexpected inheritance #12461
base: main
Are you sure you want to change the base?
Conversation
Size ReportBundles
Usages
|
@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: |
/ecosystem-ci run |
📝 Ran ecosystem CI: Open
|
I think this would also close #7474 and the associated PR #7475. There's an underlying question here about whether we consider Then there's a follow up decision about where to draw the line. What about an empty string (Playground)? Or a white-space value like I don't want to expand the scope of this PR unnecessarily, but I do think it's important to be clear why we're drawing the line at nullish values. |
@skirtles-code Thanks for the feedback! Great questions! Here’s what I thought:
The situation is more complicated than I expected. I'm investigating the current behavior of Vue and CSS custom properties. Current behavior:
For Vue, For CSS custom properties:
Since we are binding values to CSS, we should align with CSS behavior, where an empty string is treated as equivalent to white spaces. To fix the SSR/CSR mismatch and ensure consistent resetting, I propose handling values as follows:
For other string values, we should output them as-is. For unsupported types of values, we should warn about the binding and retain the current behavior (rendering everything as a string, likely using WDYT? /cc @adamdehaven @skirtles-code @edison1105 |
@Justineo I thought we determined that a whitespace character would be ideal? |
After rereading the spec, I believe that only the |
The white space character seems to work in practice if you can try? |
White space sets a valid empty value, which will prevent fallback value to take effect. |
I'm not sure injecting In my testing, See the example taken from here |
It is the standard way to reset a custom property.
https://codepen.io/Justineo/pen/VYZZPOz As shown in this pen,
That’s why we should use
We certainly don’t want to invalidate the fallback value |
The reproduction looks correct; thanks for putting that together |
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.
Copilot reviewed 5 out of 9 changed files in this pull request and generated no suggestions.
Files not reviewed (4)
- packages/compiler-sfc/tests/snapshots/compileScript.spec.ts.snap: Language not supported
- packages/runtime-core/src/component.ts: Evaluated as low risk
- packages/compiler-sfc/tests/compileScript.spec.ts: Evaluated as low risk
- packages/runtime-dom/src/helpers/useCssVars.ts: Evaluated as low risk
v-bind()
should not add custom properties to the component's style attribute when their value isundefined
. #7474Currently, we generate CSS custom properties on component roots for each
v-bind
in SFC<style>
blocks. When one component instance is nested inside another, the nested instance can inherit values from the outer instance, even if its own binding value isundefined
ornull
. Ideally, style bindings should be scoped per component instance since they depend on the instance's state. However, with the current approach using CSS custom properties, we can't generate unique hashed names for each instance because CSS can only reference a single property.To address this, the improvement here is to "reset" these custom properties when the binding value is nullish.
Current Behavior:
--<hash>-<name>:undefined
forundefined
but ignorenull
, which causes inconsistencies and is related to the hydration warning in SSR hydration errors when combining inline styles with v-bind() in CSS #12439.This PR:
--<hash>-<name>:initial
for nullish values. This properly resets the custom property, as explained in the CSS spec. It behaves as if no custom property is defined in ancestor elements.SSR Note:
:
prefix to distinguish style entries coming fromv-bind
in styles. This allows us to process them separately from user-defined custom properties inssrRenderStyle
. Alternatively, we can also create a new runtime helper and modify the codegen here:Not sure if my current approach is preferred. Let me know if you have other opinions.