linaria icon indicating copy to clipboard operation
linaria copied to clipboard

Using `styled`, undefined values in interpolations are turned into strings

Open jpnelson opened this issue 2 years ago • 2 comments

Environment

  • Linaria version: 3.0.0-beta.13
  • Bundler (+ version): next / codesandbox

Description

It seems as though setting an undefined null value for css variables is at runtime turns that value into a string. For example,

const Heading = styled.h1`
  color: ${(props) => (props.color ? props.color : undefined)};
`;
const App = () => (
  <>
    <Heading color="blue">Hello world blue</Heading>
    <Heading>Hello world – no props</Heading>
  </>
);

At runtime, the first heading will be blue, and the second will be the default text color (expected). However, the heading gets --var: undefined set on its inline styles:

Screen Shot 2022-09-28 at 11 40 30 AM

I think this would be because we stringify the values to add units:

https://github.com/callstack/linaria/blob/master/packages/react/src/styled.ts#L164

It would seem to be a pretty simple fix to add a special case for undefined values at least (probably null too?)

I would think this is probably a bug fix and not a breaking change since I can't see anyone usefully relying on the fact that undefined values get stringified? But I'm interested in your thoughts too.

Thanks!

Reproducible Demo

Demo here: https://codesandbox.io/s/angry-lovelace-3onv7p

Note that on the second header, with no color, we still get the --variable: undefined on the inline styles

jpnelson avatar Sep 28 '22 18:09 jpnelson

I've seen this and I agree with the change as a bug fix – ultimately is just bloating the HTML markup for no behavioral change (eg. undefined/null/undefinedpx/nullpx/etc. are all invalid at computed-value time which AFAIK equals the same output that simply skipping the inline custom property declaration)

There's already a warnIfInvalid() method which could filter them out but is disabled in NODE_ENV=production (I'm guessing for perf purposes but I don't think there could be much of a difference?)

I didn't submit a PR with the change yet because I was exploring if there could be any options for unlocking conditionality (or any way to bail out of a declaration) through a similar API, using CSS custom properties fallback values or something like the "space toggle hack".

Whilst I didn't find any worthwhile option yet, your example is one of the few where the implementation details of Linaria using CSS custom properties leak through when compared to other CSS-in-JS alternatives. When the Browser throws a syntax error it can fallback to the previously assigned value, but when using CSS custom properties it can't and ultimately falls back to either inherit or initial which can be quite confusing for developers (more info and examples).

juanferreras avatar Sep 29 '22 17:09 juanferreras

Ich bin hier gelandet, weil ich dasselbe Problem beobachtet habe, als ich eine Anwendung von Emotion zu Linaria portieren wollte.

Mit Emotion (und Styled Components) ist es üblich, dass Properties undefined sein können, denn dadurch lassen sich ganz einfach optionale Eigenschaften definieren:

const Title = styled.div<{mb?: string}>`
  font-size: 2em;
  margin-bottom: ${props => props.mb};
`;

Now, not only is this causing invalid CSS but it also gives me a TypeScript error ('undefined' is not assignable to ...) and a compilation warning (An interpolation evaluated to 'undefined' in the component ...).

baba43 avatar Mar 02 '23 13:03 baba43