From c779ad4da3c2895cc9d9b65722eb5b3c5750e529 Mon Sep 17 00:00:00 2001 From: Alexander Shtuchkin Date: Mon, 17 Nov 2014 14:50:18 -0800 Subject: [PATCH] Fix ReactTransitionGroup behavior when removing several children at once If several children complete leaving before rendering TransitionGroup, only the last one was removed. This could easily happen if callback in componentWillLeave is called synchronously and several items are removed from array. The other case is when ReactCSSTransitionGroup has transitionLeave={false} and array is also cleaned up. The bug was happening because this.state.children was used as a base for children removal and it wasn't updated until the render, so only the last removal was actually happening. Fix involves keeping the updated children state between invocations of _handleDoneLeaving. After updating this.state and rendering, updatedState is cleaned up and ready for subsequent array modifications. Test case included. --- .../transitions/ReactTransitionGroup.js | 12 +++- .../__tests__/ReactTransitionGroup-test.js | 61 +++++++++++++++++++ 2 files changed, 70 insertions(+), 3 deletions(-) diff --git a/src/addons/transitions/ReactTransitionGroup.js b/src/addons/transitions/ReactTransitionGroup.js index 80296e4e48..7e2505ddaf 100644 --- a/src/addons/transitions/ReactTransitionGroup.js +++ b/src/addons/transitions/ReactTransitionGroup.js @@ -89,6 +89,8 @@ var ReactTransitionGroup = React.createClass({ }, componentDidUpdate: function() { + this.updatedChildren = null; + var keysToEnter = this.keysToEnter; this.keysToEnter = []; keysToEnter.forEach(this.performEnter); @@ -193,9 +195,13 @@ var ReactTransitionGroup = React.createClass({ // This entered again before it fully left. Add it again. this.performEnter(key); } else { - var newChildren = assign({}, this.state.children); - delete newChildren[key]; - this.setState({children: newChildren}); + // As this.state.children will not be updated until next render, we keep + // this.updatedChildren state to avoid losing all but the last removal. + // It's cleaned after this.state is updated, in componentDidUpdate. + if (!this.updatedChildren) + this.updatedChildren = assign({}, this.state.children); + delete this.updatedChildren[key]; + this.setState({children: this.updatedChildren}); } }, diff --git a/src/addons/transitions/__tests__/ReactTransitionGroup-test.js b/src/addons/transitions/__tests__/ReactTransitionGroup-test.js index 1a4b23fc25..6afdc7c7bd 100644 --- a/src/addons/transitions/__tests__/ReactTransitionGroup-test.js +++ b/src/addons/transitions/__tests__/ReactTransitionGroup-test.js @@ -208,4 +208,65 @@ describe('ReactTransitionGroup', function() { 'didMount', 'didMount', 'willEnter', 'didEnter' ]); }); + + it('should handle entering/leaving several elements at once', function() { + var log = []; + var cb; + + var Child = React.createClass({ + componentDidMount: function() { + log.push('didMount'+this.props.id); + }, + componentWillEnter: function(cb) { + log.push('willEnter'+this.props.id); + cb(); + }, + componentDidEnter: function() { + log.push('didEnter'+this.props.id); + }, + componentWillLeave: function(cb) { + log.push('willLeave'+this.props.id); + cb(); + }, + componentDidLeave: function() { + log.push('didLeave'+this.props.id); + }, + componentWillUnmount: function() { + log.push('willUnmount'+this.props.id); + }, + render: function() { + return ; + } + }); + + var Component = React.createClass({ + getInitialState: function() { + return {count: 1}; + }, + render: function() { + var children = []; + for (var i = 0; i < this.state.count; i++) { + children.push(); + } + return {children}; + } + }); + + var instance = React.render(, container); + expect(log).toEqual(['didMount0']); + log = []; + + instance.setState({count: 3}); + expect(log).toEqual([ + 'didMount1', 'didMount2', 'willEnter1', 'didEnter1', + 'willEnter2', 'didEnter2' + ]); + log = []; + + instance.setState({count: 0}); + expect(log).toEqual([ + 'willLeave0', 'didLeave0', 'willLeave1', 'didLeave1', + 'willLeave2', 'didLeave2', 'willUnmount0', 'willUnmount1', 'willUnmount2' + ]); + }); });