CSS Custom Properties (var(--foo)) in theme secondary colour causes fatal error

[bennypowers]

Describe the bug
Using CSS variables in theme breaks storybook.

To Reproduce
Use this theme:

import { create } from '@storybook/theming';

export default create({
  base: 'light',

  colorSecondary: 'var(--my-var, red)',

})

Expected behavior
I expect the styles to be applied.

Code snippets

The above error occurred in the <Context.Consumer> component:
    in Styled(a) (created by Link)
    in Link (created by KnobPanel)
    in div (created by Context.Consumer)
    in Styled(div) (created by Placeholder)
    in div (created by Context.Consumer)
    in Styled(div) (created by Placeholder)
    in Placeholder (created by KnobPanel)
    in KnobPanel
    in div (created by Context.Consumer)
    in Styled(div)
    in div (created by Context.Consumer)
    in Styled(div)
    in Unknown
    in Unknown
    in Unknown
    in Unknown (created by Context.Consumer)
    in _default (created by Layout)
    in div (created by Context.Consumer)
    in Styled(div) (created by Panel)
    in Panel (created by Layout)
    in div (created by Context.Consumer)
    in Styled(div) (created by Main)
    in div (created by Context.Consumer)
    in Styled(div) (created by Main)
    in Main (created by Layout)
    in Layout (created by Context.Consumer)
    in WithTheme(Layout)
    in Unknown
    in Unknown (created by ResizeDetector)
    in ChildWrapper (created by ResizeDetector)
    in ResizeDetector
    in div (created by Context.Consumer)
    in Styled(div)
    in Unknown
    in Unknown (created by Manager)
    in ThemeProvider (created by Manager)
    in Manager (created by Context.Consumer)
    in Location (created by QueryLocation)
    in QueryLocation (created by Root)
    in LocationProvider (created by Root)
    in HelmetProvider (created by Root)
    in Root

React will try to recreate this component tree from scratch using the error boundary you provided, LocationProvider. vendors~main.5bfe49427a0ecbcae866.bundle.js:155228:5
The above error occurred in the <LocationProvider> component:
    in LocationProvider (created by Root)
    in HelmetProvider (created by Root)
    in Root

Consider adding an error boundary to your tree to customize error handling behavior.
Visit https://fb.me/react-error-boundaries to learn more about error boundaries. vendors~main.5bfe49427a0ecbcae866.bundle.js:155228:5
Error: Couldn't parse the color string. Please provide the color as a string in hex, rgb, rgba, hsl or hsla notation.

vendors~main.5bfe49427a0ecbcae866.bundle.js:19244:14
Error: Couldn't parse the color string. Please provide the color as a string in hex, rgb, rgba, hsl or hsla notation.

System:

• OS: MacOS
• Device: Macbook Pro 2018
• Browser: Firefox, Chrome
• Framework: Web Components
• Addons: theming
• Version: 5.1.0-alpha.7

Additional context
You can try this yourself just by setting the secondary color prop in react dev tools under ThemeProvider

[Armanio]

Storybook under hood use polished.js for transforming theme colors: eg set opacity, darken color, etc.
Unfortunately, now polished not support css vars.
I see 2 ways for solve this problem:

• don't use css vars in storybook (probably we set it in doc 🤔)
• not use polished and transforming css manually (it big question...)

[bennypowers]

CSS Custom Properties have been supported in browsers since 2014. It seems less than ideal to commit "Our CSS-in-JS library doesn't support browser standards" to documentation.

[Armanio]

Of course you are right. I will think about it. Maybe it is not as difficult as it seems.

[bennypowers]

If I can help, please let me know.

[Armanio]

Any PR is welcome. You can try to support CSS variables yourself. 🥇
Anywhere tomorrow i investigate this question.

[bennypowers]

@Armanio did your investigations turn up anything helpful?

[Armanio]

@bennypowers honestly, no. Sorry. 😥
But first, I'll want to get @shilman and @ndelangen opinions.
SB use 'polished' not many times:

Some features can realize by native CSS (with rgba, filters, etc). But not all. It's can be a design problem and @domyen will be unhappy.
In conclusion, I think it's a good point for SB future:

• it reduces bundle size
• CSS vars has a good browser support (>90%)
• change theme colors in runtime

Anyway, wait for maintainers thoughts.

[shilman]

Thanks for investigating @Armanio. Since it's not used much we should be able to switch it out for something more permissive without too much hassle. But I'll defer to @ndelangen and @domyen on theming decisions since they did the work, and will be maintaining our themes

[Hypnosphi]

You can resolve root values of css custom properties like that:

getComputedStyle(document.documentElement).getPropertyValue(`--my-var`)

@bennypowers where do you define your custom properties?

[bennypowers]

I define them in a css file that's linked in preview-head.html

The good-old-fashioned way :D

[Hypnosphi]

On :root level?

[bennypowers]

apologies, it's manager-head, not preview-head. this is my manager-head.html

<link rel="stylesheet" href="./my-theme.css"></link>

and here's my-theme.css:

html,
body {
  padding: 0;
  margin: 0;
  font-family: var(--ftr-font-family-primary);
}

html {
  --ftr-font-family-primary: "proxima-nova", arial, sans-serif;

  /*  General */
  --white: #fff;
  --white-rgb: 255, 255, 255;
  --black: #000;
  --black-rgb: 0, 0, 0;
/* etc*/
}

[Hypnosphi]

OK, if you move it to preview-head.html, you can work around your issue like this.

import { create } from '@storybook/theming'

const cssVar = (property, defaultValue) =>
  getComputedStyle(document.documentElement).getPropertyValue(property) || defaultValue

export default create({
  base: 'light',

  colorSecondary: cssVar('--my-var', 'red'),

})

The downside is that dynamic updates to custom properties values won't be reflected

[bennypowers]

In my case, though, I want to theme the storybook manager. I don't need to dynamically change values later on.

[Hypnosphi]

Ok, then the workaround above should work for you. The variables need to be in preview-head.html because your config.js is executed in the context of preview iframe

[ndelangen]

I'm not tied to polished personally, if we can find a way to make things work without polished that works with css custom properties, that'd be a win for all i think.

@Armanio @bennypowers do you know of such a library?

[bennypowers]

looks like emotion does

[domyen]

We use polished to transform user-supplied colors for use in the theme. I'd love to support css-vars as well if we can. I'd be open to jettisoning polished if there was a suitable color transformation alternative or if someone can recommend another method.

As far as I can tell emotion does not do color transformation by itself.

Edit: More context from this comment from polished issues

Why I asked if it was supposed to be a variable is because right now polished does not have a good way to interact with CSS variables, since they are evaluated outside of the JavaScript.

For example, with currentColor our JavaScript code doesn't know what it's value is (as is the case with any CSS variable.) in order to grab that value then do the color transform on it. CSS variables are evaluated once the browser actually interprets the CSS, which is after polished methods are evaluated.

[ndelangen]

@bennypowers can you link to docs for that?

PS. can I ask what exactly the point is? I'm not asking in a bad way, I'd like to understand what improvement you're trying to get by moving JS variables into CSS?

You're defining a global scope CSS variable and then referencing that global variable by name in a JS file. I'm sceptical about that being a good idea.

"Our CSS-in-JS library doesn't support browser standards"

I'm all in favor of using standards, but JS variables are a standard as well?

AFAIK there are no easy ways to read the value of a CSS custom property from JS. If that's possible, we can likely load the value, and keep using polished.

Of course CSS custom properties are designed to be mutable & cascading at runtime.

[bennypowers]

AFAIK there are no easy ways to read the value of a CSS custom property from JS. If that's possible, we can likely load the value, and keep using polished.

Well, you could

const getCssVarVal = cssVar =>
  getComputedStyle(window)
    .getPropertyValue(cssVar)

That's all built in to the platform, so it seems pretty easy to me, don't think you need any libraries for that.

PS. can I ask what exactly the point is? I'm not asking in a bad way, I'd like to understand what improvement you're trying to get by moving JS variables into CSS?

CSS custom properties are an inherent part of CSS the web platform, so it makes sense that a web-development tool that understands them. Your question is sort of like "switch statements can be done as ternary expressions, so we don't support switch statements".

So, from first principles, CSS custom properties should be supported because they are a part of the platform, just like <table>, IntersectionObserver, or display: grid;.

But if you really need the kind of corporate business use case, here's one example of many:

Your company's design department defines their style guide in CSS files, using CSS custom properties. You, as the developer, want to use your company's branding both in your components, but also to brand the storybook interface, so you'd like to load up the theme.css file and have it work properly, just like any other web page.

You're defining a global scope CSS variable and then referencing that global variable by name in a JS file. I'm sceptical about that being a good idea.

In my case, it's just a CSS value passed as a string, like any other CSS value.

Please forgive errors and typos, as I don't have access to my regular machines.

[ndelangen]

I ask because if our code is to become significantly more complex to handle CSS custom properties, we better have a good understanding of the reason. Principle aren't enough reason.

What I'm trying to avoid is that we add complex code that reaches into the runtime global document namespace, does global (possible ui-thread-blocking) operations to convert 1 value to another. And the storybook maintainer team has to maintain that code forever.

I'm worried that the user would be under the assumption they could change the CSS custom property's value at runtime, and those changed would be reflected in storybook's theme. (which would not be the case, we'd read the value just once).

I hope you can understand my concerns.

I interpret your use-case of a design system using CSS custom properties extensively and having some sort of vars.css that would be injected into storybook, and thus easily referenced instead of being copied over. I find this a good enough reason to support this, though I'd prefer us to not be responsible for the code that does the transformation.

Would you be interested in providing us with a package/library that turns 'var(--black, black)' into '#000'? We'd be happy to use that, provided it only does the work it needs to do if necessary.

Thank for your time & passion 🙇

---

PS. storybook runs in more platforms then the browser, it also runs in Node & React-Native.
The theming functionality is shared code.

[Hypnosphi]

@domyen Which exactly kinds of color transformations do we use? Is there a chance that those usecases can be covered by opacity CSS prop + maybe some additional backgrounds?

[ndelangen]

Valid question @Hypnosphi, though I think the options of using the opacity property is slim. The way opacity work is it affects the entire element, making it so it only affects the textColor or borderColor means restructuring the html in weird ways.

transforming transparency is best done using rgba or hsla, hsla is also especially useful for transforming hue saturation & lightness. That is what what hsl stands for.

transforming rgb values to hsl is possible in JS, afaik not in CSS.

[Hypnosphi]

Well you can always put the thing that you want to make half-transparent to a separate element or pseudoelement

UPD: Ok that's probably what you meant by "restructuring the html in weird ways". Except for the fact that pseudoelements don't affect markup

transforming hue saturation & lightness

where do we do that?

BTW you can use https://github.com/postcss/postcss-color-function to transform colors in CSS. But unfortunately, it works only at compile time, so it won't work with dynamic CSS variables

[ndelangen]

I've just gone through this document, and AFAICS there are no...
color: lighten(0.2, var(--my-var)),
color: darken(0.2, var(--my-var)),
color: transparentize(0.2, var(--my-var)),

https://www.w3.org/TR/css-color-4/#changes-from-3

...in the CSS spec. There are 12 ways of defining a color, but no ways of converting 1 to another or slightly adjusting a color after it's been defined.

This seems to be a staple of why css processors are so useful. That, and JS of course.

All code-examples in the document from W3C about the CSS spec, are in JS.
https://www.w3.org/TR/css-color-4/#color-conversion-code

@Hypnosphi Those 3 listed above are what we use.

import { darken, lighten, transparentize } from 'polished';

We also use rgba but doesn't really do anything other than:

(r,g,b,a) => `rgba(${r}, ${g}, ${b}, ${a})`;

We might as well just write that, no transformation, just convenience.

[Hypnosphi]

That's why you can't find it in spec
https://github.com/postcss/postcss-color-function#deprecated

And that's how you do darken/lighten using the plugin which still works despite the deprecation
https://github.com/postcss/postcss-color-function#notes-for-former-sass-users

[Hypnosphi]

Those 3 listed above are what we use.

Ok, so we're actually only transforming the lightness and alpha. This should be pretty easy to achieve using CSS filters. The downside is the same as with opacity: you need a separate (pseudo)element for each color usage that you want to transform

[ndelangen]

SO we're left with 2 scenarios for a potential fix:

1:
Never try to read the css value, never try to transform it, just pass it around as a value, unchangeable. In order to do lightness and darkness and hue changes we use pseudo-elements + css filters.

possible side-effects and dangers:

• possibly need to position the pseudo elements in 'funny' ways, to emulate things like borders, underlines, box-shadows, etc.
• filters makes the element have their own rendering stack, which could have side-effects

2:
We detect the use of vars() in the value and try to resolve the actual color before putting it into the storybook theme.

possible side-effects and dangers:

• Users might do runtime changes to the css vars's value and expect them to have effect. Which they will not.

---

Personally my vote is to NOT do option 1. I find it too fragile, too hacky for it too work for too long. Filters are also CPU & memory hogs, and sometimes cause odd buggy behavior in browser in my experience.

[Hypnosphi]

Users might do runtime changes to the css vars's value and expect them to have effect. Which they will not.

Looks like a proper place for a warning

[bennypowers]

FWIW I'm comfortable with a fix that just reads the initial value once. Just please make sure user CSS is loaded before reading the values, and documentation should be super clear on why Storybook doesn't yet fully support this built-in feature of the web platform.

A later, more comprehensive fix might prefer built-in CSS filters to tooling, as @Hypnosphi suggested.

Thanks for your consideration

[ndelangen]

Cool, so we come to the consensus that the solution is:

Reading the value before creating the theme just once and warning the user somehow that this is a one-off operation.

@bennypowers my question still stands, would you be able to provide us with the code to do it? Preferable as an external module. Or point us to an existing solution of course.

[Hypnosphi]

Storybook doesn't yet fully support this built-in feature of the web platform

That's just not true. All the web platform fully works in your stories (just because they run in browser). Theming the Storybook UI itself is an additional feature which has its intrinsic limitations. For example, you can't pass gradients as backgrounds. And gradients have been a part of web standarts for quite a while.

[bennypowers]

Storybook UI [...] has its intrinsic limitations

Right, so we agree that this CSS feature isn't supported. No big deal, I just think that should be explicit so developers are informed.

@ndelangen You need something fancier than this?

const getCssVarVal = cssVar =>
  getComputedStyle(document.body)
    .getPropertyValue(cssVar)

This will return black for, for example

html {
  --foo: black;
}

[Hypnosphi]

Right, so [...] CSS [..] should be [...] fancier

Ok, I'm able to strip the context as well

[bennypowers]

I'm going to bow out of this thread, since it seems to me that the discussion isn't progressing in a particularly productive direction. I really hope this fix gets in soon. Thanks all for your consideration.

[ndelangen]

@bennypowers Code looks good to me, if that works for you, my suggestion is to use that code, when you create the theme. Putting that code into the storybook core, feels like unwise. I'll try to explain:

The solution makes a lot of assumptions and have a lot of nuanced limitations.
It assumes:

• the value passed is always a var(--something)
• the var exists on body/html element
• there will be a document to read this value from
• users understand the limitation of it being read only once, and being 'bound', and even if we put a warning, that will be noise to many users who do know. Making them pay less attention to warnings that do matter.

Is this an option for you:

import { create } from '@storybook/theming/create';

export default create({
  base: 'light',
  colorSecondary: getCssVarVal('var(--my-var, red)'),
});

This works best for everyone right? You get the support, we don't have to worry about this global render-blocking function.

I'm sorry if you feel the thread was/is unproductive.

[stale]

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

[ndelangen]

This is fixed in 5.2!

[rikkit]

Apologies for the bump, just curious to know what was changed in 5.2? Using var(--name) still causes an error when loading Storybook as of 5.3. I am ok with using the workaround noted above, just wondering if I'm missing something as I've scanned the changelogs for 5.2 and I can't see any mention of CSS variables.

[ndelangen]

It should be fixed by:
https://github.com/storybookjs/storybook/pull/6939/files

[carriemorrison]

Is this still the way to get theming working with css custom properties?

import { create } from '@storybook/theming/create';

export default create({
    base: 'light',
    appContentBg: getCssVarVal('var(--white)'),
    textColor: getCssVarVal('var(--batcave)'),
});

I'm using "@storybook/react": "^6.5.14", and still see issues on some storybook pages eg mdx files...

index-681e4b07.js:41 Uncaught Error: Couldn't parse the color string. Please provide the color as a string in hex, rgb, rgba, hsl or hsla notation.


    at parseToRgb (index-681e4b07.js:41:1)
    at parseToHsl (index-681e4b07.js:56:1)
    at darken (index-681e4b07.js:219:1)
    at fn (index-681e4b07.js:195:1)
    at _storybook_theming__WEBPACK_IMPORTED_MODULE_55__.styled.table.isLoading (index-681e4b07.js:1965:1)
    at handleInterpolation (index.js:3482:1)
    at serializeStyles (index.js:3597:1)
    at index.js:3805:1
    at index.js:2509:1
    at renderWithHooks (react-dom.development.js:16305:1)