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

Fix motion warning of findDOMNode #51

Merged
merged 6 commits into from
May 16, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,4 @@ coverage/
.dumi/tmp
.dumi/tmp-production
.dumi/tmp-test
.node
12 changes: 6 additions & 6 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,16 @@
"dependencies": {
"@babel/runtime": "^7.11.1",
"classnames": "^2.2.1",
"rc-util": "^5.21.0"
"rc-util": "^5.39.3"
},
"devDependencies": {
"@rc-component/father-plugin": "^1.0.1",
"@testing-library/jest-dom": "^5.16.4",
"@testing-library/react": "^13.0.0",
"@testing-library/react": "^15.0.7",
"@types/classnames": "^2.2.9",
"@types/jest": "^26.0.8",
"@types/react": "^16.9.2",
"@types/react-dom": "^16.9.0",
"@types/react": "^18.0.0",
"@types/react-dom": "^18.0.0",
"@umijs/fabric": "^2.0.8",
"cross-env": "^7.0.2",
"dumi": "^2.0.18",
Expand All @@ -69,8 +69,8 @@
"np": "^6.2.4",
"prettier": "^2.1.1",
"rc-test": "^7.0.14",
"react": "^18.0.0",
"react-dom": "^18.0.0",
"react": "^18.3.0",
"react-dom": "^18.3.0",
"typescript": "^4.0.3"
},
"peerDependencies": {
Expand Down
4 changes: 1 addition & 3 deletions src/CSSMotion.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,7 @@ export interface CSSMotionState {
* `transitionSupport` is used for none transition test case.
* Default we use browser transition event support check.
*/
export function genCSSMotion(
config: CSSMotionConfig,
): React.ForwardRefExoticComponent<CSSMotionProps & { ref?: React.Ref<any> }> {
export function genCSSMotion(config: CSSMotionConfig) {
let transitionSupport = config;

if (typeof config === 'object') {
Expand Down
11 changes: 1 addition & 10 deletions src/hooks/useDomMotionEvents.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,10 @@ import type { MotionEvent } from '../interface';
import { animationEndName, transitionEndName } from '../util/motion';

export default (
callback: (event: MotionEvent) => void,
onInternalMotionEnd: (event: MotionEvent) => void,
): [(element: HTMLElement) => void, (element: HTMLElement) => void] => {
const cacheElementRef = useRef<HTMLElement>();

// Cache callback
const callbackRef = useRef(callback);
callbackRef.current = callback;

// Internal motion event handler
const onInternalMotionEnd = React.useCallback((event: MotionEvent) => {
callbackRef.current(event);
}, []);

// Remove events
function removeMotionEvents(element: HTMLElement) {
if (element) {
Expand Down
15 changes: 11 additions & 4 deletions src/hooks/useStatus.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { useEvent } from 'rc-util';
import useState from 'rc-util/lib/hooks/useState';
import * as React from 'react';
import { useEffect, useRef } from 'react';
Expand Down Expand Up @@ -72,7 +73,13 @@ export default function useStatus(
setStyle(null, true);
}

function onInternalMotionEnd(event: MotionEvent) {
const onInternalMotionEnd = useEvent((event: MotionEvent) => {
// Do nothing since not in any transition status.
// This may happen when `motionDeadline` trigger.
if (status === STATUS_NONE) {
return;
}

const element = getDomElement();
if (event && !event.deadline && event.target !== element) {
// event exists
Expand All @@ -93,10 +100,10 @@ export default function useStatus(
}

// Only update status when `canEnd` and not destroyed
if (status !== STATUS_NONE && currentActive && canEnd !== false) {
if (currentActive && canEnd !== false) {
updateMotionEndStatus();
}
}
});

const [patchMotionEvents] = useDomMotionEvents(onInternalMotionEnd);

Expand Down Expand Up @@ -151,7 +158,7 @@ export default function useStatus(
setStyle(eventHandlers[step]?.(getDomElement(), null) || null);
}

if (step === STEP_ACTIVE) {
if (step === STEP_ACTIVE && status !== STATUS_NONE) {
// Patch events when motion needed
patchMotionEvents(getDomElement());

Expand Down
8 changes: 5 additions & 3 deletions src/util/diff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ export type DiffStatus =
| typeof STATUS_REMOVE
| typeof STATUS_REMOVED;

type RawKeyType = string | number;

export interface KeyObject {
key: React.Key;
key: RawKeyType;
status?: DiffStatus;
}

Expand All @@ -18,7 +20,7 @@ export function wrapKeyToObject(key: React.Key | KeyObject) {
if (key && typeof key === 'object' && 'key' in key) {
keyObj = key;
} else {
keyObj = { key: key as React.Key };
keyObj = { key: key as RawKeyType };
}
return {
...keyObj,
Expand Down Expand Up @@ -90,7 +92,7 @@ export function diffKeys(
* Merge same key when it remove and add again:
* [1 - add, 2 - keep, 1 - remove] -> [1 - keep, 2 - keep]
*/
const keys = {};
const keys: Record<RawKeyType, number> = {};
list.forEach(({ key }) => {
keys[key] = (keys[key] || 0) + 1;
});
Expand Down
57 changes: 55 additions & 2 deletions tests/CSSMotion.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,10 @@
react/no-render-return-value, max-classes-per-file,
react/prefer-stateless-function, react/no-multi-comp
*/
import { fireEvent, render } from '@testing-library/react';
import { act, fireEvent, render } from '@testing-library/react';
import classNames from 'classnames';
import React from 'react';
import ReactDOM from 'react-dom';
import { act } from 'react-dom/test-utils';
import type { CSSMotionProps } from '../src';
import { Provider } from '../src';
import RefCSSMotion, { genCSSMotion } from '../src/CSSMotion';
Expand Down Expand Up @@ -342,6 +341,60 @@ describe('CSSMotion', () => {
return <div {...props} />;
}),
);

it('not warning on StrictMode', () => {
const onLeaveEnd = jest.fn();
const errorSpy = jest.spyOn(console, 'error');

const renderDemo = (visible: boolean) => (
<React.StrictMode>
<CSSMotion
motionName="transition"
motionDeadline={1000}
onLeaveEnd={onLeaveEnd}
visible={visible}
motionAppear={false}
motionLeave={true}
>
{({ style, className }) => (
<div
style={style}
className={classNames('motion-box', className)}
/>
)}
</CSSMotion>
</React.StrictMode>
);

const { rerender, container } = render(renderDemo(true));
act(() => {
jest.advanceTimersByTime(100000);
});

// Leave
rerender(renderDemo(false));
act(() => {
jest.advanceTimersByTime(500);
});

// Motion end
fireEvent.transitionEnd(
container.querySelector('.transition-leave-active'),
);
act(() => {
jest.advanceTimersByTime(100);
});

// Another timeout
act(() => {
jest.advanceTimersByTime(1000);
});

expect(onLeaveEnd).toHaveBeenCalledTimes(1);
expect(errorSpy).not.toHaveBeenCalled();

errorSpy.mockRestore();
});
});

it('not crash when no children', () => {
Expand Down
Loading