Make `cx` function support `object.toString()`

[liamqma]

The problem

The cx function from packages/react/src/class-names.js doesn't support object.toString().. It's not a bug, but another popular library classnames supports it. Can we please make this feature into EmotionJS as well?

Example:

const obj = {
  toString: function () {
    return 'css-class';
  },
};

<ClassNames>
  {({ css, cx }) => {
    return (
      <div
        className={cx(
          css`
            color: green;
          `,
          obj
        )}
      />
    )
  }}
</ClassNames>

The rendered html of above component is expected to be <div class="css-class emotion-0" />. It is currently <div class="emotion-0" /> on main branch.

Proposed solution

I've created a PR for it.

Alternative solutions

NA

Additional context

NA

[Andarist]

While it is annoying, this is intentional. We need a wrapper component to support SSR scenarios. .toString()-based solution doesn't work with SSR at all.

Sorry, I jumped the gun here in a hurry. This issue is about something else.

I don't think this is a good idea because we can't guarantee that the received object stringifies to what is a class name. I think that it makes sense for the consumer to be responsible for this conversion if they trust the given object.

[liamqma]

Thank you for reviewing my request. ❤️

I don't think this is a good idea because we can't guarantee that the received object stringifies to what is a class name.

This is why we have the extensive check arg.toString !== Object.prototype.toString && !arg.toString.toString().includes('[native code]').

Just want to point it out that ReacJS as an example, if you will pass object to className it will call toString on it, if it exists. Also, this PR adds this feature to the classnames library, and gives a reason for it.

I am happy to leave as-is. We will use patch-package for our projects for now. 🙂

[Andarist]

Just want to point it out that ReacJS as an example, if you will pass object to className it will call toString on it, if it exists.

It doesn't quite call it, at least not directly - but it's true that it tries to coerce it somewhat explicitly (through ''+value): https://github.com/facebook/react/blob/a1f97589fd298cd71f97339a230f016139c7382f/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js#L4554

I don't really think that it should be relied upon though. That's also a TypeScript issue: TS playground