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

SFC state driven CSS variables (<style vars>) #226

Closed
wants to merge 16 commits into from
Closed

Conversation

yyx990803
Copy link
Member

This is previously part of #182 - but moved into a separate PR so that it can advance on its own.

<template>
  <div class="text">hello</div>
</template>

<script>
export default {
  data() {
    return {
      color: 'red'
    }
  }
}
</script>

<style vars="{ color }">
.text {
  color: var(--color);
}
</style>

Rendered

@yyx990803 yyx990803 added final comments This RFC is in final comments period sfc Single File Components labels Nov 9, 2020
@yyx990803
Copy link
Member Author

This RFC is now in final comments stage (prior discussion and feedback in #182).

An RFC in final comments stage means that:

The core team has reviewed the feedback and reached consensus about the general direction of the RFC and believe that this RFC is a worthwhile addition to the framework.
Final comments stage does not mean the RFC's design details are final - we may still tweak the details as we implement it and discover new technical insights or constraints. It may even be further adjusted based on user feedback after it lands in an alpha/beta release.
If no major objections with solid supporting arguments have been presented after a week, the RFC will be merged and become an active RFC.

@yyx990803 yyx990803 mentioned this pull request Nov 9, 2020
@Demivan
Copy link

Demivan commented Nov 10, 2020

Note that when scoped and vars are both present, all CSS variables are considered to be local. In order to reference a global CSS variable here, use the global: prefix:

<style scoped vars="{ color }">
h1 {
  color: var(--color);
  font-size: var(--global:fontSize);
}
</style>

What is the reason to make all variables local if there is list of variables to bind? Why not make local only ones that are in vars declaration?

@privatenumber
Copy link

privatenumber commented Nov 12, 2020

@Demivan

Good question. Consider the case where a non-var'd CSS variable is declared within the scoped style tags. If it's not scoped, it will leak out into the global scope and affect child styles using the same var name:

<style scoped vars="{ color }">
h1 {
  --fontSize: 12px;   /* ⬅ If this is not scoped, it will cascade down and affect children */

  color: var(--color);
  font-size: var(--fontSize);
}
</style>

Similarly, unused classnames in a SFC are also scoped because it might match against something outside of the SFC and cause a collision.

@asv7c2
Copy link

asv7c2 commented Nov 12, 2020

<template>
  <div class="text">hello</div>
</template>

<script>
export default {
  data() {
    return {
      color: 'red'
    }
  }
}
</script>

<style> // vars are deduced from usage
.text {
  color: var(--color); // auto import color if it used in style
}
</style>

Why not auto import variables by their usage?

@Demivan
Copy link

Demivan commented Nov 12, 2020

I think scoping declared variables should not even be a part of this RFC. And in this example variable will not leak out into global scope because h1 itself will be scoped. It could leak into child elements though.

My issue with current approach is that adding scoped attribute to existing <style> block requires adding v-deep only in rare cases. But adding vars with one variable will force to add global: prefix to all currently used global variables.
When developing site that already uses css variables (for example for theming) it will be annoying to use.

You can use global classes in template when using scoped style. I think it will be intuitive that you can use global variables in style with vars too.

My suggestion is to use :global prefix only for variables with conflicting names to allow access to global variable. And maybe have different <style> attribute if there is a need to scope all variables.

@yyx990803
Copy link
Member Author

yyx990803 commented Nov 16, 2020

@Asv1 unlike JavaScript variables, CSS variables may be referencing something defined outside of the component. Without explicit variable passing the scoping will become ambiguous.

@Demivan the var(--color) to var(--hash-color) conversion is done at compile time. If we want to apply this only to passed variables, we will need to know what variables are passed at compile time. However, the vars expression can be any valid JavaScript expression: it may be a dynamic function call which isn't statically analyzable.

So the trade-off is either limit vars to a static identifier list (which makes it static analyzable), or requiring the global prefix. I believe the dynamic expression is more important in this case, since it enables variables as injected themes:

<script setup>
import { inject } from 'vue'

const themeVars = inject('theme-variables')
</script>

<style vars="themeVars">
/* use injected variables from theme */
</style>

@Demivan
Copy link

Demivan commented Nov 16, 2020

@yyx990803 Thanks for explanation.
Another solution could be to prefix injected variables instead of global. I would rather prefix them and have my global ones same in vue files and in scss (css). And it will be easier to find injected css variables in a component.

<style scoped vars="themeVars">
h1 {
  color: var(--bind:color); // or --var:color
  font-size: var(--fontSize);
}
</style>

But what is preferable probably depends on a project. If project does not use global variables automatic prefixing would be more convenient. Maybe option to switch this behavior could be added as vue-loader config?

@asv7c2
Copy link

asv7c2 commented Nov 16, 2020

@yyx990803
Understand. And one question, vars passed to CSS is reactive?

@privatenumber
Copy link

privatenumber commented Nov 16, 2020

@yyx990803

If I'm understanding the problem correctly, I think hashing can be moved to run-time and eliminate the need for the --global: prefix if the styles gets compiled into a function where all CSS vars are left as placeholders.

Given the SFC:

<style vars="{ textColor }">
.class {
    color: var(--text-color);
}
</style>

vue-loader compiles this to:

function compileStyles(vars) {
    return `
    .class{
        color: var(--${vars.textColor}); // could be '--text-color' or '--HASH-text-color'
    }
    `;
}

There's probably some overhead I'm not thinking of though. I'm not sure if the theme-injection code-snippet reacts to every theme-variables change, but with this implementation, it will definitely have to and trigger a re-render (not that I can think of a use-case where the var names would be constantly changing).

Interesting use-case by the way. I didn't think vars="themeVars" would be possible. I thought the vars="{ color }" syntax was doing an object-spread on the view model. Or did you mean vars="{ themeVars }"?

If the following syntax can be supported vars="somePropertyOnVM", it could also be an alternative to prefixing and marking local CSS vars:

<style vars="somePropertyOnVM">
.class {
    color: var(--somePropertyOnVM.textColor);
    font-size: var(--someGlobalVar);
}
</style>

Supporting object paths could be beneficial as well since theme objects can be nested for organizational purposes:

{
    colors: {
        primary: "red",
        ...
    },
    ...
}
<style vars="themeVars">
.class {
    color: var(--themeVars.colors.primary);
}
</style>

@yyx990803 yyx990803 removed the final comments This RFC is in final comments period label Nov 16, 2020
@yyx990803
Copy link
Member Author

Thanks for the feedback - this actually made me rethink the proposal and identified a number of issues that can be improved.

A new version of this feature is proposed in #231.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sfc Single File Components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants