fix(Transition): Stale nodes in transition callbacks

src/CSSTransition.js

@@ -90,72 +90,61 @@ class CSSTransition extends React.Component {
     exit: {},
   }
 
-  onEnter = (maybeNode, maybeAppearing) => {
-    const [node, appearing] = this.resolveArguments(maybeNode, maybeAppearing)
+  onEnter = (node, appearing) => {
     this.removeClasses(node, 'exit');
     this.addClass(node, appearing ? 'appear' : 'enter', 'base');
 
     if (this.props.onEnter) {
-      this.props.onEnter(maybeNode, maybeAppearing)
+      this.props.onEnter(node, appearing)
     }
   }
 
-  onEntering = (maybeNode, maybeAppearing) => {
-    const [node, appearing] = this.resolveArguments(maybeNode, maybeAppearing)
+  onEntering = (node, appearing) => {
     const type = appearing ? 'appear' : 'enter';
     this.addClass(node, type, 'active')
 
     if (this.props.onEntering) {
-      this.props.onEntering(maybeNode, maybeAppearing)
+      this.props.onEntering(node, appearing)
     }
   }
 
-  onEntered = (maybeNode, maybeAppearing) => {
-    const [node, appearing] = this.resolveArguments(maybeNode, maybeAppearing)
+  onEntered = (node, appearing) => {
     const type = appearing ? 'appear' : 'enter'
     this.removeClasses(node, type);
     this.addClass(node, type, 'done');
 
     if (this.props.onEntered) {
-      this.props.onEntered(maybeNode, maybeAppearing)
+      this.props.onEntered(node, appearing)
     }
   }
 
-  onExit = (maybeNode) => {
-    const [node] = this.resolveArguments(maybeNode)
+  onExit = (node) => {
     this.removeClasses(node, 'appear');
     this.removeClasses(node, 'enter');
     this.addClass(node, 'exit', 'base')
 
     if (this.props.onExit) {
-      this.props.onExit(maybeNode)
+      this.props.onExit(node)
     }
   }
 
-  onExiting = (maybeNode) => {
-    const [node] = this.resolveArguments(maybeNode)
+  onExiting = (node) => {
     this.addClass(node, 'exit', 'active')
 
     if (this.props.onExiting) {
-      this.props.onExiting(maybeNode)
+      this.props.onExiting(node)
     }
   }
 
-  onExited = (maybeNode) => {
-    const [node] = this.resolveArguments(maybeNode)
+  onExited = (node) => {
     this.removeClasses(node, 'exit');
     this.addClass(node, 'exit', 'done');
 
     if (this.props.onExited) {
-      this.props.onExited(maybeNode)
+      this.props.onExited(node)
     }
   }
 
-  // when prop `nodeRef` is provided `node` is excluded
-  resolveArguments = (maybeNode, maybeAppearing) => this.props.nodeRef
-    ? [this.props.nodeRef.current, maybeNode] // here `maybeNode` is actually `appearing`
-    : [maybeNode, maybeAppearing] // `findDOMNode` was used
-
   getClassNames = (type) => {
     const { classNames } = this.props;
     const isStringClassNames = typeof classNames === 'string';

src/Transition.js

@@ -224,29 +224,26 @@ class Transition extends React.Component {
   performEnter(mounting) {
     const { enter } = this.props
     const appearing = this.context ? this.context.isMounting : mounting
-    const [maybeNode, maybeAppearing] = this.props.nodeRef
-      ? [appearing]
-      : [ReactDOM.findDOMNode(this), appearing]
 
     const timeouts = this.getTimeouts()
     const enterTimeout = appearing ? timeouts.appear : timeouts.enter
     // no enter animation skip right to ENTERED
     // if we are mounting and running this it means appear _must_ be set
     if ((!mounting && !enter) || config.disabled) {
       this.safeSetState({ status: ENTERED }, () => {
-        this.props.onEntered(maybeNode)
+        this.props.onEntered(this.getNode())
       })
       return
     }
 
-    this.props.onEnter(maybeNode, maybeAppearing)
+    this.props.onEnter(this.getNode(), appearing)
 
     this.safeSetState({ status: ENTERING }, () => {
-      this.props.onEntering(maybeNode, maybeAppearing)
+      this.props.onEntering(this.getNode(), appearing)
 
       this.onTransitionEnd(enterTimeout, () => {
         this.safeSetState({ status: ENTERED }, () => {
-          this.props.onEntered(maybeNode, maybeAppearing)
+          this.props.onEntered(this.getNode(), appearing)
         })
       })
     })
@@ -255,26 +252,23 @@ class Transition extends React.Component {
   performExit() {
     const { exit } = this.props
     const timeouts = this.getTimeouts()
-    const maybeNode = this.props.nodeRef
-      ? undefined
-      : ReactDOM.findDOMNode(this)
 
     // no exit animation skip right to EXITED
     if (!exit || config.disabled) {
       this.safeSetState({ status: EXITED }, () => {
-        this.props.onExited(maybeNode)
+        this.props.onExited(this.getNode())
       })
       return
     }
 
-    this.props.onExit(maybeNode)
+    this.props.onExit(this.getNode())
 
     this.safeSetState({ status: EXITING }, () => {
-      this.props.onExiting(maybeNode)
+      this.props.onExiting(this.getNode())
 
       this.onTransitionEnd(timeouts.exit, () => {
         this.safeSetState({ status: EXITED }, () => {
-          this.props.onExited(maybeNode)
+          this.props.onExited(this.getNode())
         })
       })
     })
@@ -316,9 +310,7 @@ class Transition extends React.Component {
 
   onTransitionEnd(timeout, handler) {
     this.setNextCallback(handler)
-    const node = this.props.nodeRef
-      ? this.props.nodeRef.current
-      : ReactDOM.findDOMNode(this)
+    const node = this.getNode()
 
     const doesNotHaveTimeoutOrListener =
       timeout == null && !this.props.addEndListener
@@ -328,17 +320,20 @@ class Transition extends React.Component {
     }
 
     if (this.props.addEndListener) {
-      const [maybeNode, maybeNextCallback] = this.props.nodeRef
-        ? [this.nextCallback]
-        : [node, this.nextCallback]
-      this.props.addEndListener(maybeNode, maybeNextCallback)
+      this.props.addEndListener(node, this.nextCallback)
     }
 
     if (timeout != null) {
       setTimeout(this.nextCallback, timeout)
     }
   }
 
+  getNode() {
+    return this.props.nodeRef
+      ? this.props.nodeRef.current
+      : ReactDOM.findDOMNode(this);
+  }
+
   render() {
     const status = this.state.status
 

test/CSSTransition-test.js

@@ -122,18 +122,21 @@ describe('CSSTransition', () => {
           classNames='appear-test'
           in={true}
           appear={true}
-          onEnter={(isAppearing) => {
+          onEnter={(node, isAppearing) => {
             count++;
+            expect(node).toEqual(nodeRef.current);
             expect(isAppearing).toEqual(true);
             expect(nodeRef.current.className).toEqual('appear-test-appear');
           }}
-          onEntering={(isAppearing) => {
+          onEntering={(node, isAppearing) => {
             count++;
+            expect(node).toEqual(nodeRef.current);
             expect(isAppearing).toEqual(true);
             expect(nodeRef.current.className).toEqual('appear-test-appear appear-test-appear-active');
           }}
 
-          onEntered={(isAppearing) => {
+          onEntered={(node, isAppearing) => {
+            expect(node).toEqual(nodeRef.current);
             expect(isAppearing).toEqual(true);
             expect(nodeRef.current.className).toEqual('appear-test-appear-done appear-test-enter-done');
             expect(count).toEqual(2);
@@ -185,11 +188,13 @@ describe('CSSTransition', () => {
           classNames={{ appear: 'appear-test' }}
           in={true}
           appear={true}
-          onEnter={(isAppearing) => {
+          onEnter={(node, isAppearing) => {
+            expect(node).toEqual(nodeRef.current);
             expect(isAppearing).toEqual(true);
             expect(nodeRef.current.className).toEqual('appear-test');
           }}
-          onEntered={(isAppearing) => {
+          onEntered={(node, isAppearing) => {
+            expect(node).toEqual(nodeRef.current);
             expect(isAppearing).toEqual(true);
             expect(nodeRef.current.className).toEqual('');
             done();
@@ -215,19 +220,22 @@ describe('CSSTransition', () => {
       ).setProps({
         in: true,
 
-        onEnter(isAppearing){
+        onEnter(node, isAppearing){
           count++;
+          expect(node).toEqual(nodeRef.current);
           expect(isAppearing).toEqual(false);
           expect(nodeRef.current.className).toEqual('not-appear-test-enter');
         },
 
-        onEntering(isAppearing){
+        onEntering(node, isAppearing){
           count++;
+          expect(node).toEqual(nodeRef.current);
           expect(isAppearing).toEqual(false);
           expect(nodeRef.current.className).toEqual('not-appear-test-enter not-appear-test-enter-active');
         },
 
-        onEntered(isAppearing){
+        onEntered(node, isAppearing){
+          expect(node).toEqual(nodeRef.current);
           expect(isAppearing).toEqual(false);
           expect(nodeRef.current.className).toEqual('not-appear-test-enter-done');
           expect(count).toEqual(2);

test/SwitchTransition-test.js

@@ -10,9 +10,9 @@ describe('SwitchTransition', () => {
   beforeEach(() => {
     log = [];
     let events = {
-      onEnter: (m) => log.push(m ? 'appear' : 'enter'),
-      onEntering: (m) => log.push(m ? 'appearing' : 'entering'),
-      onEntered: (m) => log.push(m ? 'appeared' : 'entered'),
+      onEnter: (node, appearing) => log.push(appearing ? 'appear' : 'enter'),
+      onEntering: (node, appearing) => log.push(appearing ? 'appearing' : 'entering'),
+      onEntered: (node, appearing) => log.push(appearing ? 'appeared' : 'entered'),
       onExit: () => log.push('exit'),
       onExiting: () => log.push('exiting'),
       onExited: () => log.push('exited')

test/Transition-test.js

@@ -100,7 +100,7 @@ describe('Transition', () => {
   })
 
   it('should allow addEndListener instead of timeouts', done => {
-    let listener = jest.fn(end => setTimeout(end, 0))
+    let listener = jest.fn((node, end) => setTimeout(end, 0))
 
     const nodeRef = React.createRef()
     let wrapper = mount(
@@ -121,7 +121,7 @@ describe('Transition', () => {
 
   it('should fallback to timeouts with addEndListener', done => {
     let calledEnd = false
-    let listener = (end) =>
+    let listener = (node, end) =>
       setTimeout(() => {
         calledEnd = true
         end()
@@ -510,4 +510,48 @@ describe('Transition', () => {
       instance.setState({ in: false })
     })
   })
+
+  describe('node in callbacks', () => {
+    it('does not use stale nodes', done => {
+      const enteringNode = React.createRef();
+      const enteredNode = React.createRef();
+
+      function makeAssertions() {
+        expect(enteringNode.current.nodeName).toBe('H2');
+        expect(enteredNode.current.nodeName).toBe('H3');
+
+        done();
+      }
+
+      const hosts = {
+        exited: 'h1',
+        entering: 'h2',
+        entered: 'h3'
+      }
+
+      const wrapper = mount(
+        <Transition
+          in={false}
+          onEntering={handleEntering}
+          onEntered={handleEntered}
+          timeout={0}
+        >
+          {state => {
+            const Component = hosts[state];
+            return <Component />
+          }}
+        </Transition>
+      );
+      wrapper.setProps({ in: true });
+
+      function handleEntering(node) {
+        enteringNode.current = node;
+      }
+
+      function handleEntered(node) {
+        enteredNode.current = node;
+        makeAssertions();
+      }
+    })
+  })
 })

test/TransitionGroup-test.js

@@ -26,9 +26,9 @@ describe('TransitionGroup', () => {
 
     log = []
     let events = {
-      onEnter: (m) => log.push(m ? 'appear' : 'enter'),
-      onEntering: (m) => log.push(m ? 'appearing' : 'entering'),
-      onEntered: (m) => log.push(m ? 'appeared' : 'entered'),
+      onEnter: (node, appearing) => log.push(appearing ? 'appear' : 'enter'),
+      onEntering: (node, appearing) => log.push(appearing ? 'appearing' : 'entering'),
+      onEntered: (node, appearing) => log.push(appearing ? 'appeared' : 'entered'),
       onExit: () => log.push('exit'),
       onExiting: () => log.push('exiting'),
       onExited: () => log.push('exited'),