TS: CSS component expression is incorrectly typed as a `string`

[rafgraph]

Bug report

Describe the bug

CSS component expression is incorrectly typed as a string

import { css } from '@stitches/react'

// expression is typed as a "StyledExpression & string"
// but it is not a string and does not have string properties
const expression = css({ color: 'green' })();

// this passes TS type checking because indexOf exists on the string type
// but throws a runtime error because indexOf doesn't exist on the expression
expression.indexOf('foo')

Expected behavior

For the expression to either be a string (and typed as such), or to not be typed as a string. The former is preferred because the React className type requires a string and because of #570 which causes memoized components to always re-render.

System information

• Version of Stitches: 0.2.3
• Version of Node.js: 14.7.0

[jonathantneal]

The typing here bothers me, but this is an unfortunate requirement, because otherwise expression throws a TS error when used as a string. For example:

<button className={expression} />

That code works, but TS would throw an error if expression were only typed as an object and not also a string, despite the fact that expression has a toString() function for this occasion.

One might argue the fault lies with react types not supporting this kind of usage. One might argue React users should not use Stitches this way. I’d be interested to know what you think.

If there’s a way we can get expression to pass without typing it as a string, we can make that change.

[rafgraph]

For the React package why not have the expression be a string? Since that what the react types require. Or make a PR to update the types if they are wrong? Or remove the css(…) function from the react package if it’s not meant to be used with react?

[jonathantneal]

Good points, @rafgraph. One thing to add, tho, is that the expression can also be the props, which I believe means <button {...expression} /> also works.

For my part, I think the options are, ultimately:

1. Remove the false typing. This will improve the suggestion experience. Let anyone using toString() side-effect behavior to get TS errors. Redirect their ire to other typing packages or other ways of accessing the actual string?
2. Leave the false typing. This will prevent TS errors from side-effect behavior. Let anyone using string prototype functions get JS errors. Redirect ire for false typing to this issue?

Despite how my comments might read, know that I’m not very pragmatic, and so I’m actually on the fence about this. I’d prefer if things were right. The strongest reason to keep things wrong, I think, would be to cover over problems outside of Stitches. That being said, I feel like most pragmatists would leave the bad typing for now, at least anywhere it was critical, until the issue was fixed upstream. 🤷

[rafgraph]

FWIW it seems like an unnecessary complexity overload to have the expression be both a props object and a class name string. Why not remove the toString function on the expression and require the user to do expression.className when the className string is needed?

[peduarte]

From a DX point of view, forcing users to do .className is a no go. The most consistent way is what we currently have, as expressions can be called as functions in order to invoke specific variants, etc.