jsx(…) fails when “cloning” element with css prop

[coreyward]

Current behavior:

When attempting to use jsx to replace React.cloneElement on a component with a css prop, React throws the following error:

Element type is invalid: expected a string (for built-in components) or a class/function (for composite components) but got: undefined. You likely forgot to export your component from the file it's defined in, or you might have mixed up default and named imports.

I suspect this has something to do with the context nodes being inserted around elements that use the css prop, but I don't really grok the internals of Emotion or React well enough to understand how to resolve the issue.

To reproduce:
I've created a CodeSandbox demonstrating the issue here. If you remove the css prop from the h1 the operation will succeed.

Expected behavior:

jsx(element, {}) should behave similarly to React.createElement or React.cloneElement.

Environment information:

• react version: 16.8.6
• emotion version: 10.0.10

[ryanswanson]

We encountered this issue / error as well and have a potential solution that should ultimately live within the emotion library.

Diagnosis

Using jsx pragma for components with a css prop works fine. Using the jsx function directly as a cloneElement substitute works fine. We encountered the error above when combining both uses of the jsx function, e.g. the element returned from a jsx call via pragma being passed again to a jsx call when cloning a child.

We found that the jsx function stores type information in a private prop: __EMOTION_TYPE_PLEASE_DO_NOT_USE__. When cloning the element returned from the original jsx call, the type should conditionally be pulled from the private prop rather than element.type (see code below).

Our solution (partial)

import {jsx} from '@emotion/core';

export default function cloneElement(element, config, ...children) {
  return jsx(
    element.props['__EMOTION_TYPE_PLEASE_DO_NOT_USE__']
      ? element.props['__EMOTION_TYPE_PLEASE_DO_NOT_USE__']
      : element.type,
    {
      key: element.key !== null ? element.key : undefined,
      ref: element.ref,
      ...element.props,
      ...config,
    },
    ...children,
  );
}

What is missing?

There are probably additional things that a cloneElement function should do similar to that in React, such as validating config.ref and config.key.

Library Consideration

The primary reason to consider adding a cloneElement equivalent into the emotion library proper is to maintain the abstraction over the private prop (__EMOTION_TYPE_PLEASE_DO_NOT_USE__). Otherwise, the code example from #1102 was sufficient.

[Andarist]

Sorry that you had to wait for the response so long. We prepare v11 right now and it seems like maybe it will just get rid of this problem because we'll be able to use hooks internally.

[coreyward]

@Andarist That would be fantastic and improve the overall experience dramatically. What's the estimated timeline for that?

[Andarist]

I'm going through all of the issues right now (tackled nearly 80 of them in the last couple of days, which has included answering to them - sometimes comprehensively, giving guidance, creating dozens of PRs etc). Once I get through it all - I will have to implement what can be implemented without breaking changes (just few minor improvements) and then I will be able to proceed with v11, which is not expected to have major breaking changes (like v10 did).

Hard to give an exact estimate as this is all done in my free time, but I move things forward on a daily basis right now. It's also not guaranteed that v11 will fix this, but I have every intention of trying to figure out a solution that would cover this (0-config SSR seems to be the biggest challenge in this regard).

[emmatown]

I'm not sure I totally understand this issue - Why don't you want to use cloneElement?

[Andarist]

Hm, I can't figure this out either. I've adjusted the @coreyward's demo to use regular cloneElement and it works - https://codesandbox.io/s/emotion-jsx-fails-when-element-has-css-prop-xunqi . I've also created my own demo before doing that and also couldn't repro anything - https://codesandbox.io/s/restless-leaf-jlnby

@coreyward could you specify how those demos differ in functionality from what you have wanted to achieve?

[Andarist]

According to our tests, this "just works" when using plain React.cloneElement. Because we have not received any answer from the OP I'm going to close this assuming it's not an issue. If you provide a repro case for this please open a new issue and I will gladly take a look at it. Hope you understand.

[hmust92]

@Andarist The problem is it won't work if you do not provide a css prop to the h1.

[Andarist]

@hmust92 please provide a repro case on CodeSandbox

[jmca]

@hmust92
I'm having the same issue. I forked an example from the previous CodeSandbox mentioned above.

https://codesandbox.io/s/emotion-jsx-fails-when-element-has-css-prop-8mfi7?file=/src/index.js

Container in this case is just a component where you want all children to have a default background of yellow.

[Andarist]

@jmca thanks for repro, we'll try to figure out a solution for this

[sami616]

I wrote this little utility function that helps with this issue, not hugely tested but works for my use case:

• Works when the cloned elements css is dependant on the theme being passed.
• Supports both css`` and () => css`` being passed to cloned elements

@Andarist, id be interested to hear your thoughts on this approach, im fairly new to emotion and struggled a bit with this!

const emotionClone = (child, props) => {
  if (child.props.css) {
    return React.cloneElement(child, {
      ...props,
      css: theme => [child.props.css instanceof Function ? child.props.css(theme) : child.props.css, props.css],
    })
  }
  return jsx(child.type, {
    key: child.key,
    ...child.props,
    ...props,
  })
}

Usage:

  React.Children.map(children, child =>
    emotionClone(child, {
      // 👇🏼 Optionally add styles when cloning, these will take precedence  over any styles the children may already have
      css: css`
        color: pink;
      `,
    })
  )

Sandbox example: https://codesandbox.io/s/emotion-jsx-fails-when-element-has-css-prop-92n0l

[coreyward]

@sami616 For what it's worth, I use the snippet shared by @ryanswanson to work around this issue. I'd be curious if it works for you as well.

[sami616]

@coreyward , this didn't seem to work for me, i need styles to compose and override each other even if a child i'm passing to be cloned is a react component with css already applied to it. I've updated my comment above with a little sandbox to demonstrate. 👍🏼

[theodoretliu]

@sami616 I know this is an old issue, but I think I was able to get the composition of styles you wanted by doing

css: [
  element.props.css,
  css`override styles here`
]

while using the snippet from @ryanswanson .

[Andarist]

In the end, we've decided not to "fix" this nor provide a builtin workaround - we added a documentation note about this caveat: https://github.com/emotion-js/emotion/pull/1985/files . Feel free to use workarounds posted in this thread - they should continue to work with React 16.

[coreyward]

Seeing the caveat documented makes it much more clear when this is or isn't an issue, and why it's not trivial to fix. It might be worthwhile to link to this thread or another where the jsx-function-based cloneElement is demonstrated.

[alexreardon]

I've run into this issue with hot module reloading with https://github.com/parcel-bundler/parcel