Skip to content

Commit

Permalink
Fix motion warning of findDOMNode (#51)
Browse files Browse the repository at this point in the history
* chore: tmp of it

* chore: update deps

* chore: useEvent

* fix: status

* chore: fix ci

* chore: fix ts
  • Loading branch information
zombieJ authored May 16, 2024
1 parent f18d5f4 commit 9f60865
Show file tree
Hide file tree
Showing 7 changed files with 80 additions and 28 deletions.
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

0 comments on commit 9f60865

Please sign in to comment.