calling deprecated findDOMNode only when it is necessary (#17)

* avoiding call for deprecated findDOMNode as much as possible; removed some excessive refs

* added tests for strict mode deprecated findDOMNode call checks

Co-authored-by: samat <samat@dereknet.com>

Author: adaladam

src/CSSMotion.tsx

@@ -9,12 +9,9 @@ import type {
   MotionStatus,
   MotionEventHandler,
   MotionEndEventHandler,
-  MotionPrepareEventHandler} from './interface';
-import {
-  STATUS_NONE,
-  STEP_PREPARE,
-  STEP_START,
+  MotionPrepareEventHandler,
 } from './interface';
+import { STATUS_NONE, STEP_PREPARE, STEP_START } from './interface';
 import useStatus from './hooks/useStatus';
 import DomWrapper from './DomWrapper';
 import { isActive } from './hooks/useStepQueue';
@@ -135,15 +132,19 @@ export function genCSSMotion(
     const supportMotion = isSupportTransition(props);
 
     // Ref to the react node, it may be a HTMLElement
-    const nodeRef = useRef();
+    const nodeRef = useRef<any>();
     // Ref to the dom wrapper in case ref can not pass to HTMLElement
     const wrapperNodeRef = useRef();
 
     function getDomElement() {
       try {
-        return findDOMNode<HTMLElement>(
-          nodeRef.current || wrapperNodeRef.current,
-        );
+        // Here we're avoiding call for findDOMNode since it's deprecated
+        // in strict mode. We're calling it only when node ref is not
+        // an instance of DOM HTMLElement. Otherwise use
+        // findDOMNode as a final resort
+        return nodeRef.current instanceof HTMLElement
+          ? nodeRef.current
+          : findDOMNode<HTMLElement>(wrapperNodeRef.current);
       } catch (e) {
         // Only happen when `motionDeadline` trigger but element removed.
         return null;
@@ -157,21 +158,17 @@ export function genCSSMotion(
       props,
     );
 
-    // Record whether content has rended
+    // Record whether content has rendered
     // Will return null for un-rendered even when `removeOnLeave={false}`
     const renderedRef = React.useRef(mergedVisible);
     if (mergedVisible) {
       renderedRef.current = true;
     }
 
     // ====================== Refs ======================
-    const originRef = useRef(ref);
-    originRef.current = ref;
-
     const setNodeRef = React.useCallback((node: any) => {
       nodeRef.current = node;
-
-      fillRef(originRef.current, node);
+      fillRef(ref, node);
     }, []);
 
     // ===================== Render =====================
@@ -213,10 +210,8 @@ export function genCSSMotion(
         {
           ...mergedProps,
           className: classNames(getTransitionName(motionName, status), {
-            [getTransitionName(
-              motionName,
-              `${status}-${statusSuffix}`,
-            )]: statusSuffix,
+            [getTransitionName(motionName, `${status}-${statusSuffix}`)]:
+              statusSuffix,
             [motionName as string]: typeof motionName === 'string',
           }),
           style: statusStyle,

tests/CSSMotion.spec.tsx

@@ -8,6 +8,7 @@ import classNames from 'classnames';
 import { mount } from './wrapper';
 import type { CSSMotionProps } from '../src/CSSMotion';
 import RefCSSMotion, { genCSSMotion } from '../src/CSSMotion';
+import ReactDOM from 'react-dom';
 
 describe('CSSMotion', () => {
   const CSSMotion = genCSSMotion({
@@ -638,9 +639,9 @@ describe('CSSMotion', () => {
       jest.runAllTimers();
 
       const transitionEndEvent = new Event('transitionend');
-      ((wrapper
-        .find('.motion-box')
-        .instance() as any) as HTMLElement).dispatchEvent(transitionEndEvent);
+      (
+        wrapper.find('.motion-box').instance() as any as HTMLElement
+      ).dispatchEvent(transitionEndEvent);
 
       jest.runAllTimers();
 
@@ -649,4 +650,78 @@ describe('CSSMotion', () => {
     expect(wrapper.find('.motion-box')).toHaveLength(1);
     expect(wrapper.find('.motion-box').hasClass('removed')).toBeTruthy();
   });
+
+  describe('strict mode', () => {
+    beforeEach(() => {
+      jest.spyOn(ReactDOM, 'findDOMNode');
+    });
+
+    afterEach(() => {
+      jest.resetAllMocks();
+    });
+
+    it('calls findDOMNode when no refs are passed', () => {
+      const wrapper = mount(
+        <CSSMotion motionName="transition" visible>
+          {() => <div />}
+        </CSSMotion>,
+      );
+
+      act(() => {
+        jest.runAllTimers();
+        wrapper.update();
+      });
+
+      expect(ReactDOM.findDOMNode).toHaveBeenCalled();
+    });
+
+    it('does not call findDOMNode when ref is passed internally', () => {
+      const wrapper = mount(
+        <CSSMotion motionName="transition" visible>
+          {(props, ref) => <div ref={ref} />}
+        </CSSMotion>,
+      );
+
+      act(() => {
+        jest.runAllTimers();
+        wrapper.update();
+      });
+
+      expect(ReactDOM.findDOMNode).not.toHaveBeenCalled();
+    });
+
+    it('calls findDOMNode when refs are forwarded but not assigned', () => {
+      const domRef = React.createRef();
+
+      const wrapper = mount(
+        <CSSMotion motionName="transition" visible ref={domRef}>
+          {() => <div />}
+        </CSSMotion>,
+      );
+
+      act(() => {
+        jest.runAllTimers();
+        wrapper.update();
+      });
+
+      expect(ReactDOM.findDOMNode).toHaveBeenCalled();
+    });
+
+    it('does not call findDOMNode when refs are forwarded and assigned', () => {
+      const domRef = React.createRef();
+
+      const wrapper = mount(
+        <CSSMotion motionName="transition" visible ref={domRef}>
+          {(props, ref) => <div ref={ref} />}
+        </CSSMotion>,
+      );
+
+      act(() => {
+        jest.runAllTimers();
+        wrapper.update();
+      });
+
+      expect(ReactDOM.findDOMNode).not.toHaveBeenCalled();
+    });
+  });
 });