Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Remove findDOMNode from rc-css-motion #59

Merged
merged 5 commits into from
Jan 17, 2025
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
zombieJ marked this conversation as resolved.
Show resolved Hide resolved
},
"devDependencies": {
"@rc-component/father-plugin": "^1.0.1",
Expand Down
42 changes: 11 additions & 31 deletions src/CSSMotion.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +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 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';
import { Context } from './context';
import DomWrapper from './DomWrapper';
import useStatus from './hooks/useStatus';
import { isActive } from './hooks/useStepQueue';
import type {
Expand Down Expand Up @@ -91,7 +90,7 @@ export interface CSSMotionProps {
style?: React.CSSProperties;
[key: string]: any;
},
ref: (node: any) => void,
ref: React.Ref<any>,
) => React.ReactElement;
}

Expand Down Expand Up @@ -137,22 +136,9 @@ export function genCSSMotion(config: CSSMotionConfig) {

// Ref to the react node, it may be a HTMLElement
const nodeRef = useRef<any>();
// Ref to the dom wrapper in case ref can not pass to HTMLElement
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<HTMLElement>(wrapperNodeRef.current);
} catch (e) {
// Only happen when `motionDeadline` trigger but element removed.
return null;
}
return getDOM(nodeRef.current) as HTMLElement;
}

const [status, statusStep, statusStyle, mergedVisible] = useStatus(
Expand All @@ -170,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;
Expand All @@ -188,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;
Expand Down Expand Up @@ -227,7 +207,7 @@ export function genCSSMotion(config: CSSMotionConfig) {
}),
style: statusStyle,
},
setNodeRef,
nodeRef,
);
}

Expand All @@ -239,13 +219,13 @@ export function genCSSMotion(config: CSSMotionConfig) {
motionChildren = React.cloneElement(
motionChildren as React.ReactElement,
{
ref: setNodeRef,
ref: nodeRef,
},
);
}
}

return <DomWrapper ref={wrapperNodeRef}>{motionChildren}</DomWrapper>;
return motionChildren;
});

CSSMotion.displayName = 'CSSMotion';
Expand Down
13 changes: 0 additions & 13 deletions src/DomWrapper.tsx

This file was deleted.

2 changes: 1 addition & 1 deletion src/hooks/useIsomorphicLayoutEffect.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/hooks/useNextFrame.ts
Original file line number Diff line number Diff line change
@@ -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 (): [
Expand Down
6 changes: 3 additions & 3 deletions src/hooks/useStatus.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down
2 changes: 1 addition & 1 deletion src/hooks/useStepQueue.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions src/util/motion.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down
33 changes: 26 additions & 7 deletions tests/CSSMotion.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,9 @@ describe('CSSMotion', () => {
});

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<any>) {
it(name, () => {
const onAppearEnd = jest.fn();
Expand Down Expand Up @@ -839,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 = () => <div />;
const cssMotionRef = React.createRef();
render(
<CSSMotion motionName="transition" visible>
<CSSMotion motionName="transition" visible ref={cssMotionRef}>
{() => <Div />}
</CSSMotion>,
);
Expand All @@ -851,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', () => {
Expand All @@ -868,11 +873,24 @@ describe('CSSMotion', () => {
expect(ReactDOM.findDOMNode).not.toHaveBeenCalled();
});

it('calls findDOMNode when refs are forwarded but not assigned', () => {
it('support nativeElement of ref', () => {
const domRef = React.createRef();
const Div = () => <div />;
const Div = React.forwardRef<
{
nativeElement: HTMLDivElement;
},
object
>((props, ref) => {
const divRef = React.useRef<HTMLDivElement>(null);

render(
React.useImperativeHandle(ref, () => ({
nativeElement: divRef.current!,
}));

return <div {...props} ref={divRef} className="bamboo" />;
});

const { container } = render(
<CSSMotion motionName="transition" visible ref={domRef}>
{() => <Div />}
</CSSMotion>,
Expand All @@ -882,7 +900,8 @@ describe('CSSMotion', () => {
jest.runAllTimers();
});

expect(ReactDOM.findDOMNode).toHaveBeenCalled();
expect(domRef.current).toBe(container.querySelector('.bamboo'));
expect(ReactDOM.findDOMNode).not.toHaveBeenCalled();
});

it('does not call findDOMNode when refs are forwarded and assigned', () => {
Expand Down
Loading