Merge pull request #11 from gpoole/handle-null-undefined-children

Handle null or undefined children
react-component · Jul 5, 2016 · 3e611db · 3e611db
2 parents 775bb3e + 0a256c4
commit 3e611db
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 19 deletions.
diff --git a/src/Animate.js b/src/Animate.js
@@ -76,7 +76,9 @@ const Animate = React.createClass({
       });
     }
     children.forEach((child) => {
-      this.performAppear(child.key);
+      if (child) {
+        this.performAppear(child.key);
+      }
     });
   },
 
@@ -100,7 +102,7 @@ const Animate = React.createClass({
     let newChildren = [];
     if (showProp) {
       currentChildren.forEach((currentChild) => {
-        const nextChild = findChildInChildrenByKey(nextChildren, currentChild.key);
+        const nextChild = currentChild && findChildInChildrenByKey(nextChildren, currentChild.key);
         let newChild;
         if ((!nextChild || !nextChild.props[showProp]) && currentChild.props[showProp]) {
           newChild = React.cloneElement(nextChild || currentChild, {
@@ -114,7 +116,7 @@ const Animate = React.createClass({
         }
       });
       nextChildren.forEach((nextChild) => {
-        if (!findChildInChildrenByKey(currentChildren, nextChild.key)) {
+        if (!nextChild || !findChildInChildrenByKey(currentChildren, nextChild.key)) {
           newChildren.push(nextChild);
         }
       });
@@ -131,11 +133,11 @@ const Animate = React.createClass({
     });
 
     nextChildren.forEach((child) => {
-      const key = child.key;
-      if (currentlyAnimatingKeys[key]) {
+      const key = child && child.key;
+      if (child && currentlyAnimatingKeys[key]) {
         return;
       }
-      const hasPrev = findChildInChildrenByKey(currentChildren, key);
+      const hasPrev = child && findChildInChildrenByKey(currentChildren, key);
       if (showProp) {
         const showInNext = child.props[showProp];
         if (hasPrev) {
@@ -152,11 +154,11 @@ const Animate = React.createClass({
     });
 
     currentChildren.forEach((child) => {
-      const key = child.key;
-      if (currentlyAnimatingKeys[key]) {
+      const key = child && child.key;
+      if (child && currentlyAnimatingKeys[key]) {
         return;
       }
-      const hasNext = findChildInChildrenByKey(nextChildren, key);
+      const hasNext = child && findChildInChildrenByKey(nextChildren, key);
       if (showProp) {
         const showInNow = child.props[showProp];
         if (hasNext) {
@@ -288,7 +290,7 @@ const Animate = React.createClass({
     let children = null;
     if (stateChildren) {
       children = stateChildren.map((child) => {
-        if (child === null) {
+        if (child === null || child === undefined) {
           return child;
         }
         if (!child.key) {

diff --git a/src/ChildrenUtils.js b/src/ChildrenUtils.js
@@ -15,7 +15,7 @@ export function findChildInChildrenByKey(children, key) {
       if (ret) {
         return;
       }
-      if (child.key === key) {
+      if (child && child.key === key) {
         ret = child;
       }
     });
@@ -27,7 +27,7 @@ export function findShownChildInChildrenByKey(children, key, showProp) {
   let ret = null;
   if (children) {
     children.forEach((child) => {
-      if (child.key === key && child.props[showProp]) {
+      if (child && child.key === key && child.props[showProp]) {
         if (ret) {
           throw new Error('two child with same key for <rc-animate> children');
         }
@@ -45,7 +45,7 @@ export function findHiddenChildInChildrenByKey(children, key, showProp) {
       if (found) {
         return;
       }
-      found = child.key === key && !child.props[showProp];
+      found = child && child.key === key && !child.props[showProp];
     });
   }
   return found;
@@ -56,10 +56,14 @@ export function isSameChildren(c1, c2, showProp) {
   if (same) {
     c1.forEach((child, index) => {
       const child2 = c2[index];
-      if (child.key !== child2.key) {
-        same = false;
-      } else if (showProp && child.props[showProp] !== child2.props[showProp]) {
-        same = false;
+      if (child && child2) {
+        if ((child && !child2) || (!child && child2)) {
+          same = false;
+        } else if (child.key !== child2.key) {
+          same = false;
+        } else if (showProp && child.props[showProp] !== child2.props[showProp]) {
+          same = false;
+        }
       }
     });
   }
@@ -74,7 +78,7 @@ export function mergeChildren(prev, next) {
   const nextChildrenPending = {};
   let pendingChildren = [];
   prev.forEach((child) => {
-    if (findChildInChildrenByKey(next, child.key)) {
+    if (child && findChildInChildrenByKey(next, child.key)) {
       if (pendingChildren.length) {
         nextChildrenPending[child.key] = pendingChildren;
         pendingChildren = [];
@@ -85,7 +89,7 @@ export function mergeChildren(prev, next) {
   });
 
   next.forEach((child) => {
-    if (nextChildrenPending.hasOwnProperty(child.key)) {
+    if (child && nextChildrenPending.hasOwnProperty(child.key)) {
       ret = ret.concat(nextChildrenPending[child.key]);
     }
     ret.push(child);

diff --git a/tests/multiple.spec.js b/tests/multiple.spec.js
@@ -47,8 +47,20 @@ const TodoList = React.createClass({
     this.setState({ items: newItems });
   },
 
+  insertUndefined(i) {
+    const newItems = this.state.items;
+    newItems.splice(i, 1, undefined);
+    this.setState({ items: newItems });
+  },
+
   render() {
     const items = this.state.items.map((item, i) => {
+      // Allow testing of null/undefined values by just passing them directly to
+      // Animate instead of wrapping them with a Todo component
+      if (item === null || item === undefined) {
+        return item;
+      }
+
       return (
         <Todo key={item} onClick={this.handleRemove.bind(this, i)}>
           {item}
@@ -138,4 +150,31 @@ describe('Animate', () => {
       done();
     }, 1400);
   });
+
+  it('does not generate an error when a null or undefined child is present', () => {
+    list.handleAdd(null);
+    expect(TestUtils.scryRenderedDOMComponentsWithClass(list, 'item').length).to.be(4);
+    list.handleAdd(undefined);
+    expect(TestUtils.scryRenderedDOMComponentsWithClass(list, 'item').length).to.be(4);
+  });
+
+  it('transitionLeave works when a child becomes undefined', function t4(done) {
+    this.timeout(5999);
+    list.insertUndefined(0);
+    setTimeout(() => {
+      expect(TestUtils.scryRenderedDOMComponentsWithClass(list, 'item').length).to.be(4);
+      if (!window.callPhantom) {
+        expect(TestUtils.scryRenderedDOMComponentsWithClass(list, 'item')[0].className)
+          .to.contain('example-leave');
+        expect(TestUtils.scryRenderedDOMComponentsWithClass(list, 'item')[0].className)
+          .to.contain('example-leave-active');
+      }
+    }, 100);
+    setTimeout(() => {
+      if (!window.callPhantom) {
+        expect(TestUtils.scryRenderedDOMComponentsWithClass(list, 'item').length).to.be(3);
+      }
+      done();
+    }, 1400);
+  });
 });