Skip to content

Commit

Permalink
Alternative fix for #55 (#72)
Browse files Browse the repository at this point in the history
* Alternative fix for #55

* don't hold onto refs, bind them to callback functions
  • Loading branch information
frankwallis authored and jquense committed Jun 9, 2017
1 parent e694fa5 commit b54588a
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 32 deletions.
55 changes: 23 additions & 32 deletions src/TransitionGroup.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class TransitionGroup extends React.Component {
let initialChildMapping = this.state.children;
for (let key in initialChildMapping) {
if (initialChildMapping[key]) {
this.performAppear(key);
this.performAppear(key, this.childRefs[key]);
}
}
}
Expand All @@ -60,15 +60,15 @@ class TransitionGroup extends React.Component {
for (let key in nextChildMapping) {
let hasPrev = prevChildMapping && prevChildMapping.hasOwnProperty(key);
if (nextChildMapping[key] && !hasPrev &&
!this.currentlyTransitioningKeys[key]) {
!this.currentlyTransitioningKeys[key]) {
this.keysToEnter.push(key);
}
}

for (let key in prevChildMapping) {
let hasNext = nextChildMapping && nextChildMapping.hasOwnProperty(key);
if (prevChildMapping[key] && !hasNext &&
!this.currentlyTransitioningKeys[key]) {
!this.currentlyTransitioningKeys[key]) {
this.keysToLeave.push(key);
}
}
Expand All @@ -79,30 +79,27 @@ class TransitionGroup extends React.Component {
componentDidUpdate() {
let keysToEnter = this.keysToEnter;
this.keysToEnter = [];
keysToEnter.forEach(this.performEnter);
keysToEnter.forEach(key => this.performEnter(key, this.childRefs[key]));

let keysToLeave = this.keysToLeave;
this.keysToLeave = [];
keysToLeave.forEach(this.performLeave);
keysToLeave.forEach(key => this.performLeave(key, this.childRefs[key]));
}

performAppear = (key) => {
performAppear = (key, component) => {
this.currentlyTransitioningKeys[key] = true;

let component = this.childRefs[key];

if (component.componentWillAppear) {
component.componentWillAppear(
this._handleDoneAppearing.bind(this, key),
this._handleDoneAppearing.bind(this, key, component),
);
} else {
this._handleDoneAppearing(key);
this._handleDoneAppearing(key, component);
}
};

_handleDoneAppearing = (key) => {
let component = this.childRefs[key];
if (component && component.componentDidAppear) {
_handleDoneAppearing = (key, component) => {
if (component.componentDidAppear) {
component.componentDidAppear();
}

Expand All @@ -112,27 +109,24 @@ class TransitionGroup extends React.Component {

if (!currentChildMapping || !currentChildMapping.hasOwnProperty(key)) {
// This was removed before it had fully appeared. Remove it.
this.performLeave(key);
this.performLeave(key, component);
}
};

performEnter = (key) => {
performEnter = (key, component) => {
this.currentlyTransitioningKeys[key] = true;

let component = this.childRefs[key];

if (component.componentWillEnter) {
component.componentWillEnter(
this._handleDoneEntering.bind(this, key),
this._handleDoneEntering.bind(this, key, component),
);
} else {
this._handleDoneEntering(key);
this._handleDoneEntering(key, component);
}
};

_handleDoneEntering = (key) => {
let component = this.childRefs[key];
if (component && component.componentDidEnter) {
_handleDoneEntering = (key, component) => {
if (component.componentDidEnter) {
component.componentDidEnter();
}

Expand All @@ -142,28 +136,25 @@ class TransitionGroup extends React.Component {

if (!currentChildMapping || !currentChildMapping.hasOwnProperty(key)) {
// This was removed before it had fully entered. Remove it.
this.performLeave(key);
this.performLeave(key, component);
}
};

performLeave = (key) => {
performLeave = (key, component) => {
this.currentlyTransitioningKeys[key] = true;

let component = this.childRefs[key];
if (component.componentWillLeave) {
component.componentWillLeave(this._handleDoneLeaving.bind(this, key));
component.componentWillLeave(this._handleDoneLeaving.bind(this, key, component));
} else {
// Note that this is somewhat dangerous b/c it calls setState()
// again, effectively mutating the component before all the work
// is done.
this._handleDoneLeaving(key);
this._handleDoneLeaving(key, component);
}
};

_handleDoneLeaving = (key) => {
let component = this.childRefs[key];

if (component && component.componentDidLeave) {
_handleDoneLeaving = (key, component) => {
if (component.componentDidLeave) {
component.componentDidLeave();
}

Expand All @@ -173,7 +164,7 @@ class TransitionGroup extends React.Component {

if (currentChildMapping && currentChildMapping.hasOwnProperty(key)) {
// This entered again before it fully left. Add it again.
this.performEnter(key);
this.keysToEnter.push(key);
} else {
this.setState((state) => {
let newChildren = Object.assign({}, state.children);
Expand Down
80 changes: 80 additions & 0 deletions test/TransitionGroup-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -440,4 +440,84 @@ describe('TransitionGroup', () => {
// ' in Component (at **)'
// );
});

it('should not throw when enter callback is called and is now leaving', () => {
class Child extends React.Component {
componentWillReceiveProps() {
if (this.callback) {
this.callback();
}
}

componentWillEnter(callback) {
this.callback = callback;
}

render() {
return (<span />);
}
}

class Component extends React.Component {
render() {
return (
<TransitionGroup>
{this.props.children}
</TransitionGroup>
);
}
}

// render the base component
ReactDOM.render(<Component />, container);
// now make the child enter
ReactDOM.render(
<Component><Child key="child" /></Component>,
container,
);
// rendering the child leaving will call 'componentWillProps' which will trigger the
// callback. This would throw an error previously.
expect(ReactDOM.render.bind(this, <Component />, container)).not.toThrow();
})

it('should not throw when leave callback is called and is now entering', () => {
class Child extends React.Component {
componentWillReceiveProps() {
if (this.callback) {
this.callback();
}
}

componentWillLeave(callback) {
this.callback = callback;
}

render() {
return (<span />);
}
}

class Component extends React.Component {
render() {
return (
<TransitionGroup>
{this.props.children}
</TransitionGroup>
);
}
}

// render the base component
ReactDOM.render(<Component />, container);
// now make the child enter
ReactDOM.render(
<Component><Child key="child" /></Component>,
container,
);
// make the child leave
ReactDOM.render(<Component />, container);
// rendering the child entering again will call 'componentWillProps' which will trigger the
// callback. This would throw an error previously.
expect(ReactDOM.render.bind(this, <Component><Child key="child" /></Component>, container)).not.toThrow();
})
});

0 comments on commit b54588a

Please sign in to comment.