From acbddd26413a20afe48ca6ad681c9229933ba848 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20H=C3=B8jberg?= Date: Wed, 13 Nov 2013 20:54:22 -0500 Subject: [PATCH 1/2] ReactTransitions: Don't animate undefined children --- .../transitions/ReactTransitionGroup.js | 5 +++++ .../__tests__/ReactTransitionGroup-test.js | 21 +++++++++++++------ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/addons/transitions/ReactTransitionGroup.js b/src/addons/transitions/ReactTransitionGroup.js index 623838c49f4ce..9e2d4911abec4 100644 --- a/src/addons/transitions/ReactTransitionGroup.js +++ b/src/addons/transitions/ReactTransitionGroup.js @@ -44,6 +44,11 @@ var ReactTransitionGroupMixin = { 'getTransitionConfig() method.' ); + // don't animate undefined children + if (typeof sourceChildren === 'undefined') { + return; + } + var children = {}; var childMapping = ReactTransitionKeySet.getChildMapping(sourceChildren); var transitionConfig = this.getTransitionConfig(); diff --git a/src/addons/transitions/__tests__/ReactTransitionGroup-test.js b/src/addons/transitions/__tests__/ReactTransitionGroup-test.js index bfddd7be5ae93..8455d3fba1cab 100644 --- a/src/addons/transitions/__tests__/ReactTransitionGroup-test.js +++ b/src/addons/transitions/__tests__/ReactTransitionGroup-test.js @@ -26,18 +26,18 @@ var mocks; // Most of the real functionality is covered in other unit tests, this just // makes sure we're wired up correctly. describe('ReactTransitionGroup', function() { + var container; + beforeEach(function() { React = require('React'); ReactTransitionGroup = require('ReactTransitionGroup'); mocks = require('mocks'); + + container = document.createElement('div'); }); it('should warn after time with no transitionend', function() { - var container; - var a; - - container = document.createElement('div'); - a = React.renderComponent( + var a = React.renderComponent( , @@ -65,7 +65,6 @@ describe('ReactTransitionGroup', function() { }); it('should keep both sets of DOM nodes around', function() { - var container = document.createElement('div'); var a = React.renderComponent( @@ -83,4 +82,14 @@ describe('ReactTransitionGroup', function() { expect(a.getDOMNode().childNodes[0].id).toBe('two'); expect(a.getDOMNode().childNodes[1].id).toBe('one'); }); + + describe('with an undefined child', function () { + it('should fail silently', function () { + var a = React.renderComponent( + + , + container + ); + }); + }); }); From 5b1e4c0324c984be96d4792b1ed13d457862a5bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20H=C3=B8jberg?= Date: Wed, 13 Nov 2013 23:02:06 -0500 Subject: [PATCH 2/2] Transitions: Handle undefined input to mergeKeySet Gracefully handle undefined input to mergeKeySet. --- .../transitions/ReactTransitionGroup.js | 6 +---- .../transitions/ReactTransitionKeySet.js | 3 +++ .../__tests__/ReactTransitionKeySet-test.js | 26 +++++++++++++++++++ 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/src/addons/transitions/ReactTransitionGroup.js b/src/addons/transitions/ReactTransitionGroup.js index 9e2d4911abec4..f846f208c2a09 100644 --- a/src/addons/transitions/ReactTransitionGroup.js +++ b/src/addons/transitions/ReactTransitionGroup.js @@ -44,13 +44,9 @@ var ReactTransitionGroupMixin = { 'getTransitionConfig() method.' ); - // don't animate undefined children - if (typeof sourceChildren === 'undefined') { - return; - } - var children = {}; var childMapping = ReactTransitionKeySet.getChildMapping(sourceChildren); + var transitionConfig = this.getTransitionConfig(); var currentKeys = ReactTransitionKeySet.mergeKeySets( this._transitionGroupCurrentKeys, diff --git a/src/addons/transitions/ReactTransitionKeySet.js b/src/addons/transitions/ReactTransitionKeySet.js index debb3073ca3f8..ebb1cdeeb930d 100644 --- a/src/addons/transitions/ReactTransitionKeySet.js +++ b/src/addons/transitions/ReactTransitionKeySet.js @@ -68,6 +68,9 @@ var ReactTransitionKeySet = { * in `next` in a reasonable order. */ mergeKeySets: function(prev, next) { + prev = prev || {}; + next = next || {}; + var keySet = {}; var prevKeys = Object.keys(prev).concat([MERGE_KEY_SETS_TAIL_SENTINEL]); var nextKeys = Object.keys(next).concat([MERGE_KEY_SETS_TAIL_SENTINEL]); diff --git a/src/addons/transitions/__tests__/ReactTransitionKeySet-test.js b/src/addons/transitions/__tests__/ReactTransitionKeySet-test.js index 72aba5ca6f21e..993ebbc6e09d5 100644 --- a/src/addons/transitions/__tests__/ReactTransitionKeySet-test.js +++ b/src/addons/transitions/__tests__/ReactTransitionKeySet-test.js @@ -127,4 +127,30 @@ describe('ReactTransitionKeySet', function() { five: true }); }); + + it('should support mergeKeySets with undefined input', function () { + var prev = { + one: true, + two: true + }; + + var next = undefined; + + expect(ReactTransitionKeySet.mergeKeySets(prev, next)).toEqual({ + one: true, + two: true + }); + + prev = undefined; + + next = { + three: true, + four: true + }; + + expect(ReactTransitionKeySet.mergeKeySets(prev, next)).toEqual({ + three: true, + four: true + }); + }); });