Radium breaks ReactTransitionGroup leave transition

[RoccoC]

While working with Radium, I've noticed that components that are enhanced with Radium and which contain a ReactTransitionGroup do not call the componentWillLeave animation lifecycle method. Have a look at this example:

https://jsbin.com/wolopuzubu/edit?js,console,output

If you comment out the following line in the example (towards the bottom)

var App = Radium(App);

you will see that the leave transition works properly. Also worth noting is that when enhanced with Radium, the "appear" lifecycle hooks are run, as opposed to the "enter" and "leave" hooks when not enhanced with Radium.

After some investigation into the Radium source I found that removing the following logic fixes the problem, although I am not sure of why, nor of any other implications of removing this code:

  // src/resolve-styles.js line 337

  // If nothing changed, don't bother cloning the element. Might be a bit
  // wasteful, as we add the sentinal to stop double-processing when we clone.
  // Assume benign double-processing is better than unneeded cloning.
  if (
    newChildren === renderedElement.props.children &&
    newProps === renderedElement.props
  ) {
    return renderedElement;
  }

From what I can tell, removing this code forces Radium to clone each element regardless of whether the children or props have changed, but again I am not sure why this fixes things.

Please let me know if I can offer any more information.

[ianobermiller]

Thanks for the details repro and investigation! I'll take a look and see what we can do.

[ianobermiller]

Did a little debugging, and noticed that ReactTransitionGroup's componentWillUpdate lifecycle method is never called; instead it always calls componentDidMount, indicating that something in Radium is causing the component instance not to be reused.

After further digging, the key on the ReactTransitionGroup element is rendered with a slash / before it when the children are rendered, and without it when they are not. This causes React to ditch the original instance. Fixing this would also fix the real issue behind #496, where we were seeing server rendering problems due to this slash.

[RoccoC]

Great, I appreciate the quick response and will keep an eye out for a fix.

[ianobermiller]

So, this all has to do with using React.Children.map, which "helpfully" adds keys if you didn't have any before. This key generation algorithm, however, is not very predictable, and adds an extra slash depending something about the element's children. If we avoid it entirely, we can't just use children.map, since that would create a new, non-blessed array, one that requires keys. But, if we modify the children in-place, this problem is mitigated entirely. We could also get the "blessed" children by calling React.createElement directly. Here is some working code I hacked together. I'll submit a PR soon:

if (Array.isArray(children)) {
  const resolvedChildren = children.map(child => {
    if (React.isValidElement(child)) {
      return resolveStyles(component, child, config, existingKeyMap);
    }

    return child;
  });

  // We don't want to use React.Children.map, since it adds keys and modifies the children
  // in a way component authors don't expect. To get a "blessed" array and avoid errors
  // about missing keys, call `React.createElement` and steal the children.
  return React.createElement.apply(
    null, 
    ['div', {}].concat(resolvedChildren)
  ).props.children;
}

[ianobermiller]

Instead of doing that, though, I fixed the bug in React: facebook/react#5892

Not sure what we should do in the interim here...

[RoccoC]

@ianobermiller , thanks for figuring this out so quickly. It's not a blocker (for us, anyway) so we should be able to wait for the PR to be merged and released in the next React patch. Thanks again.

[alexlande]

I believe that's released as of React 15, so going to close this one.