Merge pull request #39 from aligon/patch-1

Proposed fix for #29

src/TransitionGroup.js

@@ -191,24 +191,33 @@ class TransitionGroup extends React.Component {
       let child = this.state.children[key];
       if (child) {
         let isCallbackRef = typeof child.ref !== 'string';
+        let factoryChild = this.props.childFactory(child);
+        let ref = (r) => {
+          this.childRefs[key] = r;
+        };
+
         warning(isCallbackRef,
           'string refs are not supported on children of TransitionGroup and will be ignored. ' +
           'Please use a callback ref instead: https://facebook.github.io/react/docs/refs-and-the-dom.html#the-ref-callback-attribute');
 
+        // Always chaining the refs leads to problems when the childFactory
+        // wraps the child. The child ref callback gets called twice with the
+        // wrapper and the child. So we only need to chain the ref if the
+        // factoryChild is not different from child.
+        if (factoryChild === child && isCallbackRef) {
+          ref = chain(child.ref, ref);
+        }
+
         // You may need to apply reactive updates to a child as it is leaving.
         // The normal React way to do it won't work since the child will have
         // already been removed. In case you need this behavior you can provide
         // a childFactory function to wrap every child, even the ones that are
         // leaving.
         childrenToRender.push(React.cloneElement(
-          this.props.childFactory(child),
+          factoryChild,
           {
             key,
-            ref: chain(
-              isCallbackRef ? child.ref : null,
-              (r) => {
-                this.childRefs[key] = r;
-              }),
+            ref,
           },
         ));
       }

test/TransitionGroup-test.js

@@ -1,6 +1,7 @@
 import tsp from 'teaspoon';
 
 let React;
+let PropTypes;
 let ReactDOM;
 let TransitionGroup;
 
@@ -11,6 +12,7 @@ describe('TransitionGroup', () => {
 
   beforeEach(() => {
     React = require('react');
+    PropTypes = require('prop-types');
     ReactDOM = require('react-dom');
     TransitionGroup = require('../src/TransitionGroup');
 
@@ -72,6 +74,80 @@ describe('TransitionGroup', () => {
     expect(ref).toHaveBeenCalled();
   });
 
+  it('properly calls ref if childFactory doesn\'t create a wrapper', () => {
+    let transition;
+
+    const transitionRef = (r) => { transition = r; };
+    const childRef = jest.fn();
+
+    class Child extends React.Component {
+      render() {
+        return (<span />);
+      }
+    }
+
+    const rendered = tsp(
+      <TransitionGroup ref={transitionRef}>
+        <Child key="child" ref={childRef} />
+      </TransitionGroup>,
+    )
+    .render();
+
+    expect(transition.childRefs['.$child']).toEqual(jasmine.any(Child));
+
+    rendered.unmount();
+
+    for (let i in childRef.mock.calls) {
+      let call = childRef.mock.calls[i];
+      let valid = (call[0] === null || call[0] instanceof Child) && call.length === 1;
+
+      expect(valid).toBeTruthy();
+    }
+  });
+
+  it('properly calls ref if childFactory does create a wrapper', () => {
+    let transition;
+
+    const transitionRef = (r) => { transition = r; };
+    const childRef = jest.fn();
+
+    class Wrapper extends React.Component {
+      static propTypes = {
+        children: PropTypes.element,
+      }
+
+      render() {
+        return this.props.children;
+      }
+    }
+
+    class Child extends React.Component {
+      render() {
+        return (<span />);
+      }
+    }
+
+    const childFactory = x => React.createElement(Wrapper, null, x);
+
+    const rendered = tsp(
+      <TransitionGroup ref={transitionRef} childFactory={childFactory}>
+        <Child key="child" ref={childRef} />
+      </TransitionGroup>,
+    )
+    .render();
+
+    expect(transition.childRefs['.$child']).toEqual(jasmine.any(Wrapper));
+
+    rendered.unmount();
+
+    for (let i in childRef.mock.calls) {
+      let call = childRef.mock.calls[i];
+      let valid = (call[0] === null || call[0] instanceof Child) && call.length === 1;
+
+      expect(valid).toBeTruthy();
+    }
+  });
+
   it('should handle willEnter correctly', () => {
     let log = [];