`css` returns an `object` not a `string` so memoized components always re-render

[rafgraph]

Bug report

Describe the bug

const c = css({...}), calling c() returns an object not a string so memoized components re-render. Also this hurts React's internal performance optimizations on intrinsic elements, e.g. <div className={c()}>...</div>

Setting className={String(c())} fixes the issue. So does turning css into an IIFE outside of a component like const c = (css({...}))() and then className={c}, assuming the object is treated as immutable by Stitches. But I don't think these work arounds should be necessary.

To Reproduce

https://codesandbox.io/s/stitches-react-css-object-bug-drsx0

Expected behavior

const c = css({...}), calling c() should return a string.

EDIT: an alternative would be for Stitches to memoize the object that's returned (so the return value of c() is referentially equivalent), but my preference would be for a string because an object won't work with TS types which require the className to be a string.

System information

• Version of Stitches: 0.1.7

[jonathantneal]

A component created with css() returns an object by design. An alternative resolution might be to memoize the resulting object.

The css() function will always return an object, because the object has more than its own classNames string.

For instance, a css() component forwards any of its props not swallowed by variants. If we wrote something like expression = component({ type: 'button', color: 'red' }), where component had a color variant, then our expression.props would be a the fall-thru object of { type: 'button' }.

This is very useful in applications where you’re assigning more than the class name to an element. Here’s an example.

const $buttonElement = Object.assign(
  document.createElement('button'),
  c({ type: 'button', color: 'red' })
)

[rafgraph]

Hi @jonathantneal, thanks for the explanation, I think I understand a bit better what's going on. I think for the narrow use case of "A function to generate class names from a style object" (quote from docs), it may make sense to wrap the super powerful css() function instead of exposing it directly. This would solve this issue and would probably make for a better TS experience as well.

Regarding the above use case to create a $buttonElement, that's super neat you can do that with a Stitches css component, but I don't think it is relevant to the React package. I personally can't think of why I'd need to use the css() function like that when using React. The super powerful css() function, which is used internally by the styled() function, can still be exported from the core package for such use cases (perhaps under a different name, maybe something like createStitchesComponent() or createComposition(), see note below).

Here's how exporting a wrapped version could potentially work, and I think it could provide a better TS experience as well:

// note that in this code I renamed the current sheet.css function
// to createComposition, the functionality remains unchanged
export const css = (styleObject) => {
  const composition = createComposition(styleObject) // currently sheet.css

  // the function that's returned from css() should be typed to only
  // accept an object with known variant key value pairs from the composition,
  // as well as a `css` key to add additional styles
  // (the same as the `css` prop that can be passed to a styled React component),
  // it should NOT accept any random key/values like `composition(...)` does
  return (activeVariantsAndCssObject: CompositionVariantsAndCss) => (
    String(composition(activeVariantsAndCssObject))
  )
}

As a side note, I've found it confusing to have the word "component" refer to both React components and Stitches components, which are two very different things (something I only just realized). In the @stitches/react code, a Stitches component is called a composition, which may be a better name to use in the core package as well to avoid confusion, especially among open source contributors who may not be familiar with the inner workings of Stitches. I only just now realized that the word "component" in this comment refers to a Stitches component and not a React component. Also I'm pretty sure, but not positive, that the "component" in this test refers to a React component, while "component" in this test refers to a Stitches component, which to me confusing. It may also make sense to rename the sheet.css() function to something like createComposition() or createStitchesComponent() to make it clear what it's doing. Any thoughts on this @peduarte?

[rafgraph]

Also, just a note about TS, which I know isn't something that's going be addressed for a little bit. I think the current type assigned to the expression of StyledExpression & string is not correct and can lead to not type safe code. The expression is an object, and shouldn't be typed as a string (I'm guessing it is typed this way so an expression can be passed to the React className prop, which only accepts the string type).

String methods cannot be run on the expression, for example, expression.indexOf('foo') doesn't work. However, with the current typing of StyledExpression & string, TypeScript thinks it is fine to do expression.indexOf('foo') even though it results in a runtime error.

[jonathantneal]

Now that we’ve firmly decided not to support props, it might be in line with Stitches to stop letting them slip thru, which would mean components could return true class name strings. Do I correctly infer from your comments here and in #687 that you are open to this? Note that if we decided to use regular strings, it would mean prop forwarding (at least in React) would be lost, and thus any spread usage when using styled components. And if we decided not to use regular strings, then these would become #wontfix type issues.

[rafgraph]

Making it a string sounds good.