From ddab4ff6bf631b15315d36c7b41d642ece563998 Mon Sep 17 00:00:00 2001 From: Roxanne Date: Sun, 5 Jan 2025 15:58:41 +0800 Subject: [PATCH 1/5] feat: Remove findDOMNode from rc-css-motion --- src/CSSMotion.tsx | 16 ++-------- tests/CSSMotion.spec.tsx | 67 ++++++---------------------------------- 2 files changed, 13 insertions(+), 70 deletions(-) diff --git a/src/CSSMotion.tsx b/src/CSSMotion.tsx index 5660e20..625b8b7 100644 --- a/src/CSSMotion.tsx +++ b/src/CSSMotion.tsx @@ -1,6 +1,5 @@ /* eslint-disable react/default-props-match-prop-types, react/no-multi-comp, react/prop-types */ import classNames from 'classnames'; -import findDOMNode from 'rc-util/lib/Dom/findDOMNode'; import { fillRef, getNodeRef, supportRef } from 'rc-util/lib/ref'; import * as React from 'react'; import { useRef } from 'react'; @@ -141,18 +140,9 @@ export function genCSSMotion(config: CSSMotionConfig) { const wrapperNodeRef = useRef(); function getDomElement() { - try { - // 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(wrapperNodeRef.current); - } catch (e) { - // Only happen when `motionDeadline` trigger but element removed. - return null; - } + return nodeRef.current instanceof HTMLElement + ? nodeRef.current + : wrapperNodeRef.current; } const [status, statusStep, statusStyle, mergedVisible] = useStatus( diff --git a/tests/CSSMotion.spec.tsx b/tests/CSSMotion.spec.tsx index b5c71a9..d09cf3c 100644 --- a/tests/CSSMotion.spec.tsx +++ b/tests/CSSMotion.spec.tsx @@ -5,7 +5,6 @@ import { act, fireEvent, render } from '@testing-library/react'; import classNames from 'classnames'; import React from 'react'; -import ReactDOM from 'react-dom'; import type { CSSMotionProps } from '../src'; import { Provider } from '../src'; import RefCSSMotion, { genCSSMotion } from '../src/CSSMotion'; @@ -831,74 +830,28 @@ describe('CSSMotion', () => { }); describe('strict mode', () => { - beforeEach(() => { - jest.spyOn(ReactDOM, 'findDOMNode'); - }); - - afterEach(() => { - jest.resetAllMocks(); - }); - - it('calls findDOMNode when no refs are passed', () => { + it('renders correctly when no ref is passed', () => { const Div = () =>
; - render( - - {() =>
} - , - ); - act(() => { - jest.runAllTimers(); - }); - - expect(ReactDOM.findDOMNode).toHaveBeenCalled(); - }); - - it('does not call findDOMNode when ref is passed internally', () => { - render( + const { container } = render( - {(props, ref) =>
} + {(props, ref) =>
} , ); - act(() => { - jest.runAllTimers(); - }); - - expect(ReactDOM.findDOMNode).not.toHaveBeenCalled(); + // 检查 DOM 是否渲染出来 + expect(container.querySelector('div')).toBeInTheDocument(); }); - it('calls findDOMNode when refs are forwarded but not assigned', () => { - const domRef = React.createRef(); - const Div = () =>
; - - render( - - {() =>
} - , - ); - - act(() => { - jest.runAllTimers(); - }); - - expect(ReactDOM.findDOMNode).toHaveBeenCalled(); - }); - - it('does not call findDOMNode when refs are forwarded and assigned', () => { - const domRef = React.createRef(); - - render( - + it('renders correctly when ref is passed internally', () => { + const { container } = render( + {(props, ref) =>
} , ); - act(() => { - jest.runAllTimers(); - }); - - expect(ReactDOM.findDOMNode).not.toHaveBeenCalled(); + // 检查 DOM 是否渲染出来 + expect(container.querySelector('div')).toBeInTheDocument(); }); }); From 00c8a48ebc2dbebc92f22a8a9951891ea492132f Mon Sep 17 00:00:00 2001 From: Roxanne Date: Sun, 5 Jan 2025 21:10:06 +0800 Subject: [PATCH 2/5] feat: Remove findDOMNode from rc-css-motion --- tests/CSSMotion.spec.tsx | 48 +++++++++++++++++++++++++++++----------- 1 file changed, 35 insertions(+), 13 deletions(-) diff --git a/tests/CSSMotion.spec.tsx b/tests/CSSMotion.spec.tsx index d09cf3c..2419a97 100644 --- a/tests/CSSMotion.spec.tsx +++ b/tests/CSSMotion.spec.tsx @@ -9,6 +9,15 @@ import type { CSSMotionProps } from '../src'; import { Provider } from '../src'; import RefCSSMotion, { genCSSMotion } from '../src/CSSMotion'; +const ForwardedComponent = React.forwardRef((props, ref) => { + const { visible, ...rest } = props; // 过滤掉 visible 属性 + return ( +
+ Hello +
+ ); +}); + describe('CSSMotion', () => { const CSSMotion = genCSSMotion({ transitionSupport: true, @@ -286,6 +295,10 @@ describe('CSSMotion', () => { unmount(); }); + beforeAll(() => { + jest.spyOn(document, 'addEventListener').mockImplementation(() => {}); + }); + describe('deadline should work', () => { function test(name: string, Component: React.ComponentType) { it(name, () => { @@ -322,24 +335,33 @@ describe('CSSMotion', () => { test( 'without ref', - // eslint-disable-next-line @typescript-eslint/no-unused-vars React.forwardRef((props, ref) => { - return
; + return
; // 使用 forwardRef 正确转发 ref }), ); - test( - 'FC with ref', - React.forwardRef((props, ref: any) =>
), - ); + it('FC with ref', () => { + const ref = React.createRef(); - test( - 'FC but not dom ref', - React.forwardRef((props, ref) => { - React.useImperativeHandle(ref, () => ({})); - return
; - }), - ); + // 使用 act 包裹渲染过程，确保状态更新 + let container; + act(() => { + // 仅在 act 内进行渲染，以确保是同步的 + const { container: renderedContainer } = render( + , // visible 为布尔值 + ); + container = renderedContainer; // 获取容器 + }); + + // 获取 div 元素，确保其正确渲染 + const div = container.querySelector('div'); + + // 确保 div 元素渲染 + expect(div).toBeTruthy(); + + // 确保 ref 被正确绑定到 div 元素 + expect(ref.current).toBe(div); + }); it('not warning on StrictMode', () => { const onLeaveEnd = jest.fn(); From e159dda981ba0145674e56a33e2eddb91599439c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=BA=8C=E8=B4=A7=E6=9C=BA=E5=99=A8=E4=BA=BA?= Date: Thu, 16 Jan 2025 17:41:36 +0800 Subject: [PATCH 3/5] chore: clean up wrapper --- package.json | 4 +- src/CSSMotion.tsx | 32 +++--- src/DomWrapper.tsx | 13 --- src/hooks/useIsomorphicLayoutEffect.ts | 2 +- src/hooks/useNextFrame.ts | 2 +- src/hooks/useStatus.ts | 6 +- src/hooks/useStepQueue.ts | 2 +- src/util/motion.ts | 4 +- tests/CSSMotion.spec.tsx | 130 ++++++++++++++++--------- 9 files changed, 107 insertions(+), 88 deletions(-) delete mode 100644 src/DomWrapper.tsx diff --git a/package.json b/package.json index 3d1ff4d..5d32194 100644 --- a/package.json +++ b/package.json @@ -47,8 +47,8 @@ }, "dependencies": { "@babel/runtime": "^7.11.1", - "classnames": "^2.2.1", - "rc-util": "^5.44.0" + "@rc-component/util": "^1.2.0", + "classnames": "^2.2.1" }, "devDependencies": { "@rc-component/father-plugin": "^1.0.1", diff --git a/src/CSSMotion.tsx b/src/CSSMotion.tsx index 625b8b7..7836a18 100644 --- a/src/CSSMotion.tsx +++ b/src/CSSMotion.tsx @@ -1,10 +1,10 @@ /* eslint-disable react/default-props-match-prop-types, react/no-multi-comp, react/prop-types */ +import { getDOM } from '@rc-component/util/lib/Dom/findDOMNode'; +import { getNodeRef, supportRef } from '@rc-component/util/lib/ref'; import classNames from 'classnames'; -import { fillRef, getNodeRef, supportRef } from 'rc-util/lib/ref'; import * as React from 'react'; import { useRef } from 'react'; import { Context } from './context'; -import DomWrapper from './DomWrapper'; import useStatus from './hooks/useStatus'; import { isActive } from './hooks/useStepQueue'; import type { @@ -90,7 +90,7 @@ export interface CSSMotionProps { style?: React.CSSProperties; [key: string]: any; }, - ref: (node: any) => void, + ref: React.Ref, ) => React.ReactElement; } @@ -136,13 +136,9 @@ export function genCSSMotion(config: CSSMotionConfig) { // Ref to the react node, it may be a HTMLElement const nodeRef = useRef(); - // Ref to the dom wrapper in case ref can not pass to HTMLElement - const wrapperNodeRef = useRef(); function getDomElement() { - return nodeRef.current instanceof HTMLElement - ? nodeRef.current - : wrapperNodeRef.current; + return getDOM(nodeRef.current) as HTMLElement; } const [status, statusStep, statusStyle, mergedVisible] = useStatus( @@ -160,13 +156,7 @@ export function genCSSMotion(config: CSSMotionConfig) { } // ====================== Refs ====================== - const setNodeRef = React.useCallback( - (node: any) => { - nodeRef.current = node; - fillRef(ref, node); - }, - [ref], - ); + React.useImperativeHandle(ref, () => getDomElement()); // ===================== Render ===================== let motionChildren: React.ReactNode; @@ -178,16 +168,16 @@ export function genCSSMotion(config: CSSMotionConfig) { } else if (status === STATUS_NONE) { // Stable children if (mergedVisible) { - motionChildren = children({ ...mergedProps }, setNodeRef); + motionChildren = children({ ...mergedProps }, nodeRef); } else if (!removeOnLeave && renderedRef.current && leavedClassName) { motionChildren = children( { ...mergedProps, className: leavedClassName }, - setNodeRef, + nodeRef, ); } else if (forceRender || (!removeOnLeave && !leavedClassName)) { motionChildren = children( { ...mergedProps, style: { display: 'none' } }, - setNodeRef, + nodeRef, ); } else { motionChildren = null; @@ -217,7 +207,7 @@ export function genCSSMotion(config: CSSMotionConfig) { }), style: statusStyle, }, - setNodeRef, + nodeRef, ); } @@ -229,13 +219,13 @@ export function genCSSMotion(config: CSSMotionConfig) { motionChildren = React.cloneElement( motionChildren as React.ReactElement, { - ref: setNodeRef, + ref: nodeRef, }, ); } } - return {motionChildren}; + return motionChildren; }); CSSMotion.displayName = 'CSSMotion'; diff --git a/src/DomWrapper.tsx b/src/DomWrapper.tsx deleted file mode 100644 index e53c9e5..0000000 --- a/src/DomWrapper.tsx +++ /dev/null @@ -1,13 +0,0 @@ -import * as React from 'react'; - -export interface DomWrapperProps { - children: React.ReactNode; -} - -class DomWrapper extends React.Component { - render() { - return this.props.children; - } -} - -export default DomWrapper; diff --git a/src/hooks/useIsomorphicLayoutEffect.ts b/src/hooks/useIsomorphicLayoutEffect.ts index 7dd0ab0..e2a6211 100644 --- a/src/hooks/useIsomorphicLayoutEffect.ts +++ b/src/hooks/useIsomorphicLayoutEffect.ts @@ -1,4 +1,4 @@ -import canUseDom from 'rc-util/lib/Dom/canUseDom'; +import canUseDom from '@rc-component/util/lib/Dom/canUseDom'; import { useEffect, useLayoutEffect } from 'react'; // It's safe to use `useLayoutEffect` but the warning is annoying diff --git a/src/hooks/useNextFrame.ts b/src/hooks/useNextFrame.ts index 9f06173..0ee9604 100644 --- a/src/hooks/useNextFrame.ts +++ b/src/hooks/useNextFrame.ts @@ -1,4 +1,4 @@ -import raf from 'rc-util/lib/raf'; +import raf from '@rc-component/util/lib/raf'; import * as React from 'react'; export default (): [ diff --git a/src/hooks/useStatus.ts b/src/hooks/useStatus.ts index 73014ef..da39f0c 100644 --- a/src/hooks/useStatus.ts +++ b/src/hooks/useStatus.ts @@ -1,6 +1,6 @@ -import { useEvent } from 'rc-util'; -import useState from 'rc-util/lib/hooks/useState'; -import useSyncState from 'rc-util/lib/hooks/useSyncState'; +import { useEvent } from '@rc-component/util'; +import useState from '@rc-component/util/lib/hooks/useState'; +import useSyncState from '@rc-component/util/lib/hooks/useSyncState'; import * as React from 'react'; import { useEffect, useRef } from 'react'; import type { CSSMotionProps } from '../CSSMotion'; diff --git a/src/hooks/useStepQueue.ts b/src/hooks/useStepQueue.ts index 48a4605..05362c0 100644 --- a/src/hooks/useStepQueue.ts +++ b/src/hooks/useStepQueue.ts @@ -1,4 +1,4 @@ -import useState from 'rc-util/lib/hooks/useState'; +import useState from '@rc-component/util/lib/hooks/useState'; import * as React from 'react'; import type { MotionStatus, StepStatus } from '../interface'; import { diff --git a/src/util/motion.ts b/src/util/motion.ts index a9d3752..d2a60d9 100644 --- a/src/util/motion.ts +++ b/src/util/motion.ts @@ -1,5 +1,5 @@ -import canUseDOM from 'rc-util/lib/Dom/canUseDom'; -import { MotionName } from '../CSSMotion'; +import canUseDOM from '@rc-component/util/lib/Dom/canUseDom'; +import type { MotionName } from '../CSSMotion'; // ================= Transition ================= // Event wrapper. Copy from react source code diff --git a/tests/CSSMotion.spec.tsx b/tests/CSSMotion.spec.tsx index 2419a97..10e53be 100644 --- a/tests/CSSMotion.spec.tsx +++ b/tests/CSSMotion.spec.tsx @@ -5,19 +5,11 @@ import { act, fireEvent, render } from '@testing-library/react'; import classNames from 'classnames'; import React from 'react'; +import ReactDOM from 'react-dom'; import type { CSSMotionProps } from '../src'; import { Provider } from '../src'; import RefCSSMotion, { genCSSMotion } from '../src/CSSMotion'; -const ForwardedComponent = React.forwardRef((props, ref) => { - const { visible, ...rest } = props; // 过滤掉 visible 属性 - return ( -
- Hello -
- ); -}); - describe('CSSMotion', () => { const CSSMotion = genCSSMotion({ transitionSupport: true, @@ -295,11 +287,10 @@ describe('CSSMotion', () => { unmount(); }); - beforeAll(() => { - jest.spyOn(document, 'addEventListener').mockImplementation(() => {}); - }); - describe('deadline should work', () => { + // NOTE: only test for not crash here + // Since React 19 not support `findDOMNode` anymore + // the func call will not get real DOM node function test(name: string, Component: React.ComponentType) { it(name, () => { const onAppearEnd = jest.fn(); @@ -335,33 +326,24 @@ describe('CSSMotion', () => { test( 'without ref', + // eslint-disable-next-line @typescript-eslint/no-unused-vars React.forwardRef((props, ref) => { - return
; // 使用 forwardRef 正确转发 ref + return
; }), ); - it('FC with ref', () => { - const ref = React.createRef(); - - // 使用 act 包裹渲染过程，确保状态更新 - let container; - act(() => { - // 仅在 act 内进行渲染，以确保是同步的 - const { container: renderedContainer } = render( - , // visible 为布尔值 - ); - container = renderedContainer; // 获取容器 - }); - - // 获取 div 元素，确保其正确渲染 - const div = container.querySelector('div'); - - // 确保 div 元素渲染 - expect(div).toBeTruthy(); + test( + 'FC with ref', + React.forwardRef((props, ref: any) =>
), + ); - // 确保 ref 被正确绑定到 div 元素 - expect(ref.current).toBe(div); - }); + test( + 'FC but not dom ref', + React.forwardRef((props, ref) => { + React.useImperativeHandle(ref, () => ({})); + return
; + }), + ); it('not warning on StrictMode', () => { const onLeaveEnd = jest.fn(); @@ -852,28 +834,88 @@ describe('CSSMotion', () => { }); describe('strict mode', () => { - it('renders correctly when no ref is passed', () => { + beforeEach(() => { + jest.spyOn(ReactDOM, 'findDOMNode'); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + it('calls findDOMNode when no refs are passed', () => { const Div = () =>
; + render( + + {() =>
} + , + ); - const { container } = render( + act(() => { + jest.runAllTimers(); + }); + + expect(ReactDOM.findDOMNode).toHaveBeenCalled(); + }); + + it('does not call findDOMNode when ref is passed internally', () => { + render( - {(props, ref) =>
} + {(props, ref) =>
} , ); - // 检查 DOM 是否渲染出来 - expect(container.querySelector('div')).toBeInTheDocument(); + act(() => { + jest.runAllTimers(); + }); + + expect(ReactDOM.findDOMNode).not.toHaveBeenCalled(); }); - it('renders correctly when ref is passed internally', () => { + it('support nativeElement of ref', () => { + const domRef = React.createRef(); + const Div = React.forwardRef< + { + nativeElement: HTMLDivElement; + }, + object + >((props, ref) => { + const divRef = React.useRef(null); + + React.useImperativeHandle(ref, () => ({ + nativeElement: divRef.current!, + })); + + return
; + }); + const { container } = render( - + + {() =>
} + , + ); + + act(() => { + jest.runAllTimers(); + }); + + expect(domRef.current).toBe(container.querySelector('.bamboo')); + expect(ReactDOM.findDOMNode).not.toHaveBeenCalled(); + }); + + it('does not call findDOMNode when refs are forwarded and assigned', () => { + const domRef = React.createRef(); + + render( + {(props, ref) =>
} , ); - // 检查 DOM 是否渲染出来 - expect(container.querySelector('div')).toBeInTheDocument(); + act(() => { + jest.runAllTimers(); + }); + + expect(ReactDOM.findDOMNode).not.toHaveBeenCalled(); }); }); From 22849e70913854970b9a8c461a215131b6dc9218 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=BA=8C=E8=B4=A7=E6=9C=BA=E5=99=A8=E4=BA=BA?= Date: Thu, 16 Jan 2025 17:46:37 +0800 Subject: [PATCH 4/5] test: fix test case --- tests/CSSMotion.spec.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/CSSMotion.spec.tsx b/tests/CSSMotion.spec.tsx index 10e53be..d724fdd 100644 --- a/tests/CSSMotion.spec.tsx +++ b/tests/CSSMotion.spec.tsx @@ -842,10 +842,11 @@ describe('CSSMotion', () => { jest.resetAllMocks(); }); - it('calls findDOMNode when no refs are passed', () => { + it('not crash when no refs are passed', () => { const Div = () =>
; + const cssMotionRef = React.createRef(); render( - + {() =>
} , ); @@ -854,7 +855,8 @@ describe('CSSMotion', () => { jest.runAllTimers(); }); - expect(ReactDOM.findDOMNode).toHaveBeenCalled(); + expect(cssMotionRef.current).toBeFalsy(); + expect(ReactDOM.findDOMNode).not.toHaveBeenCalled(); }); it('does not call findDOMNode when ref is passed internally', () => { From cc9a87777e14fe0517a0ccaaf91e34926886e4e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=BA=8C=E8=B4=A7=E6=9C=BA=E5=99=A8=E4=BA=BA?= Date: Fri, 17 Jan 2025 11:15:44 +0800 Subject: [PATCH 5/5] chore: fix lint --- package.json | 1 + src/CSSMotion.tsx | 2 +- src/CSSMotionList.tsx | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 5d32194..88e14cb 100644 --- a/package.json +++ b/package.json @@ -31,6 +31,7 @@ "docs:build": "dumi build", "docs:deploy": "gh-pages -d .doc", "lint": "eslint src/ --ext .tsx,.ts", + "lint:tsc": "tsc --noEmit", "now-build": "npm run docs:build", "prepare": "husky install", "prepublishOnly": "npm run compile && np --yolo --no-publish", diff --git a/src/CSSMotion.tsx b/src/CSSMotion.tsx index 7836a18..8757c0e 100644 --- a/src/CSSMotion.tsx +++ b/src/CSSMotion.tsx @@ -225,7 +225,7 @@ export function genCSSMotion(config: CSSMotionConfig) { } } - return motionChildren; + return motionChildren as React.ReactElement; }); CSSMotion.displayName = 'CSSMotion'; diff --git a/src/CSSMotionList.tsx b/src/CSSMotionList.tsx index 2a3d478..6eedfd1 100644 --- a/src/CSSMotionList.tsx +++ b/src/CSSMotionList.tsx @@ -55,7 +55,7 @@ export interface CSSMotionListProps index?: number; [key: string]: any; }, - ref: (node: any) => void, + ref: React.Ref, ) => React.ReactElement; }