fix: issue with dynamically applied classes not being properly removed for reentering items (#499)

* Add failing test for not removing dynamically applied classes

* Fixed issue with not removing dynamically applied classes

* Clear cached applied classes when removing them

* Few stylistic changes suggested at code review

* Fix linting errors

Author: Andarist

src/CSSTransition.js

@@ -74,71 +74,62 @@ class CSSTransition extends React.Component {
   static defaultProps = {
     classNames: ''
   }
-  onEnter = (node, appearing) => {
-    const { className } = this.getClassNames(appearing ? 'appear' : 'enter')
 
+  appliedClasses = {
+    appear: {},
+    enter: {},
+    exit: {},
+  }
+
+  onEnter = (node, appearing) => {
     this.removeClasses(node, 'exit');
-    addClass(node, className)
+    this.addClass(node, appearing ? 'appear' : 'enter', 'base');
 
     if (this.props.onEnter) {
       this.props.onEnter(node, appearing)
     }
   }
 
   onEntering = (node, appearing) => {
-    const { activeClassName } = this.getClassNames(
-      appearing ? 'appear' : 'enter'
-    );
-
-    this.reflowAndAddClass(node, activeClassName)
+    const type = appearing ? 'appear' : 'enter';
+    this.addClass(node, type, 'active')
 
     if (this.props.onEntering) {
       this.props.onEntering(node, appearing)
     }
   }
 
   onEntered = (node, appearing) => {
-    const appearClassName = this.getClassNames('appear').doneClassName;
-    const enterClassName = this.getClassNames('enter').doneClassName;
-    const doneClassName = appearing
-      ? `${appearClassName} ${enterClassName}`
-      : enterClassName;
-
-    this.removeClasses(node, appearing ? 'appear' : 'enter');
-    addClass(node, doneClassName);
+    const type = appearing ? 'appear' : 'enter'
+    this.removeClasses(node, type);
+    this.addClass(node, type, 'done');
 
     if (this.props.onEntered) {
       this.props.onEntered(node, appearing)
     }
   }
 
   onExit = (node) => {
-    const { className } = this.getClassNames('exit')
-
     this.removeClasses(node, 'appear');
     this.removeClasses(node, 'enter');
-    addClass(node, className)
+    this.addClass(node, 'exit', 'base')
 
     if (this.props.onExit) {
       this.props.onExit(node)
     }
   }
 
   onExiting = (node) => {
-    const { activeClassName } = this.getClassNames('exit')
-
-    this.reflowAndAddClass(node, activeClassName)
+    this.addClass(node, 'exit', 'active')
 
     if (this.props.onExiting) {
       this.props.onExiting(node)
     }
   }
 
   onExited = (node) => {
-    const { doneClassName } = this.getClassNames('exit');
-
     this.removeClasses(node, 'exit');
-    addClass(node, doneClassName);
+    this.addClass(node, 'exit', 'done');
 
     if (this.props.onExited) {
       this.props.onExited(node)
@@ -148,46 +139,69 @@ class CSSTransition extends React.Component {
   getClassNames = (type) => {
     const { classNames } = this.props;
     const isStringClassNames = typeof classNames === 'string';
-    const prefix = isStringClassNames && classNames ? classNames + '-' : '';
+    const prefix = isStringClassNames && classNames
+      ? `${classNames}-`
+      : '';
 
-    let className = isStringClassNames ?
-      prefix + type : classNames[type]
+    let baseClassName = isStringClassNames
+      ? `${prefix}${type}`
+      : classNames[type]
 
-    let activeClassName = isStringClassNames ?
-      className + '-active' : classNames[type + 'Active'];
+    let activeClassName = isStringClassNames
+      ? `${baseClassName}-active`
+      : classNames[`${type}Active`];
 
-    let doneClassName = isStringClassNames ?
-      className + '-done' : classNames[type + 'Done'];
+    let doneClassName = isStringClassNames
+      ? `${baseClassName}-done`
+      : classNames[`${type}Done`];
 
     return {
-      className,
+      baseClassName,
       activeClassName,
       doneClassName
     };
   }
 
-  removeClasses(node, type) {
-    const { className, activeClassName, doneClassName } = this.getClassNames(type)
-    className && removeClass(node, className);
-    activeClassName && removeClass(node, activeClassName);
-    doneClassName && removeClass(node, doneClassName);
-  }
+  addClass(node, type, phase) {
+    let className = this.getClassNames(type)[`${phase}ClassName`];
+
+    if (type === 'appear' && phase === 'done') {
+      className += ` ${this.getClassNames('enter').doneClassName}`;
+    }
 
-  reflowAndAddClass(node, className) {
     // This is for to force a repaint,
     // which is necessary in order to transition styles when adding a class name.
-    if (className) {
+    if (phase === 'active') {
       /* eslint-disable no-unused-expressions */
       node && node.scrollTop;
-      /* eslint-enable no-unused-expressions */
-      addClass(node, className);
     }
+
+    this.appliedClasses[type][phase] = className
+    addClass(node, className)
   }
 
-  render() {
-    const props = { ...this.props };
+  removeClasses(node, type) {
+    const {
+      base: baseClassName,
+      active: activeClassName,
+      done: doneClassName
+    } = this.appliedClasses[type]
+
+    this.appliedClasses[type] = {};
 
-    delete props.classNames;
+    if (baseClassName) {
+      removeClass(node, baseClassName);
+    }
+    if (activeClassName) {
+      removeClass(node, activeClassName);
+    }
+    if (doneClassName) {
+      removeClass(node, doneClassName);
+    }
+  }
+
+  render() {
+    const { classNames: _, ...props } = this.props;
 
     return (
       <Transition

test/.eslintrc.yml

@@ -1,6 +1,7 @@
 
 env:
   jest: true
+  es6: true
 rules:
   no-require: off
   global-require: off

test/CSSTransition-test.js

@@ -2,6 +2,7 @@ import React from 'react';
 import { mount } from 'enzyme';
 
 import CSSTransition from '../src/CSSTransition';
+import TransitionGroup from '../src/TransitionGroup';
 
 describe('CSSTransition', () => {
 
@@ -334,4 +335,83 @@ describe('CSSTransition', () => {
       });
     });
   });
+
+  describe('reentering', () => {
+    it('should remove dynamically applied classes', done => {
+      let count = 0;
+      class Test extends React.Component {
+        render() {
+          const { direction, text, ...props } = this.props;
+
+          return (
+            <TransitionGroup
+              component={null}
+              childFactory={child =>
+                React.cloneElement(child, {
+                  classNames: direction
+                })
+              }
+            >
+              <CSSTransition
+                key={text}
+                timeout={100}
+                {...props}
+              >
+                <span>{text}</span>
+              </CSSTransition>
+            </TransitionGroup>
+          )
+        }
+      }
+
+      const instance = mount(<Test direction="down" text="foo" />)
+
+      const rerender = getProps => new Promise(resolve =>
+        instance.setProps({
+          onEnter: undefined,
+          onEntering: undefined,
+          onEntered: undefined,
+          onExit: undefined,
+          onExiting: undefined,
+          onExited: undefined,
+          ...getProps(resolve)
+        })
+      );
+
+      Promise.resolve().then(() =>
+        rerender(resolve => ({
+          direction: 'up',
+          text: 'bar',
+
+          onEnter(node) {
+            count++;
+            expect(node.className).toEqual('up-enter');
+          },
+          onEntering(node) {
+            count++;
+            expect(node.className).toEqual('up-enter up-enter-active');
+            resolve()
+          }
+        }))
+      ).then(() => {
+        return rerender(resolve => ({
+          direction: 'down',
+          text: 'foo',
+
+          onEntering(node) {
+            count++;
+            expect(node.className).toEqual('down-enter down-enter-active');
+          },
+          onEntered(node) {
+            count++;
+            expect(node.className).toEqual('down-enter-done');
+            resolve();
+          }
+        }))
+      }).then(() => {
+        expect(count).toEqual(4);
+        done();
+      });
+    });
+  });
 });