Skip to content

Commit 9f60865

Browse files
authored
Fix motion warning of findDOMNode (#51)
* chore: tmp of it * chore: update deps * chore: useEvent * fix: status * chore: fix ci * chore: fix ts
1 parent f18d5f4 commit 9f60865

File tree

7 files changed

+80
-28
lines changed

7 files changed

+80
-28
lines changed

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,3 +34,4 @@ coverage/
3434
.dumi/tmp
3535
.dumi/tmp-production
3636
.dumi/tmp-test
37+
.node

package.json

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,16 +48,16 @@
4848
"dependencies": {
4949
"@babel/runtime": "^7.11.1",
5050
"classnames": "^2.2.1",
51-
"rc-util": "^5.21.0"
51+
"rc-util": "^5.39.3"
5252
},
5353
"devDependencies": {
5454
"@rc-component/father-plugin": "^1.0.1",
5555
"@testing-library/jest-dom": "^5.16.4",
56-
"@testing-library/react": "^13.0.0",
56+
"@testing-library/react": "^15.0.7",
5757
"@types/classnames": "^2.2.9",
5858
"@types/jest": "^26.0.8",
59-
"@types/react": "^16.9.2",
60-
"@types/react-dom": "^16.9.0",
59+
"@types/react": "^18.0.0",
60+
"@types/react-dom": "^18.0.0",
6161
"@umijs/fabric": "^2.0.8",
6262
"cross-env": "^7.0.2",
6363
"dumi": "^2.0.18",
@@ -69,8 +69,8 @@
6969
"np": "^6.2.4",
7070
"prettier": "^2.1.1",
7171
"rc-test": "^7.0.14",
72-
"react": "^18.0.0",
73-
"react-dom": "^18.0.0",
72+
"react": "^18.3.0",
73+
"react-dom": "^18.3.0",
7474
"typescript": "^4.0.3"
7575
},
7676
"peerDependencies": {

src/CSSMotion.tsx

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,7 @@ export interface CSSMotionState {
107107
* `transitionSupport` is used for none transition test case.
108108
* Default we use browser transition event support check.
109109
*/
110-
export function genCSSMotion(
111-
config: CSSMotionConfig,
112-
): React.ForwardRefExoticComponent<CSSMotionProps & { ref?: React.Ref<any> }> {
110+
export function genCSSMotion(config: CSSMotionConfig) {
113111
let transitionSupport = config;
114112

115113
if (typeof config === 'object') {

src/hooks/useDomMotionEvents.ts

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,10 @@ import type { MotionEvent } from '../interface';
55
import { animationEndName, transitionEndName } from '../util/motion';
66

77
export default (
8-
callback: (event: MotionEvent) => void,
8+
onInternalMotionEnd: (event: MotionEvent) => void,
99
): [(element: HTMLElement) => void, (element: HTMLElement) => void] => {
1010
const cacheElementRef = useRef<HTMLElement>();
1111

12-
// Cache callback
13-
const callbackRef = useRef(callback);
14-
callbackRef.current = callback;
15-
16-
// Internal motion event handler
17-
const onInternalMotionEnd = React.useCallback((event: MotionEvent) => {
18-
callbackRef.current(event);
19-
}, []);
20-
2112
// Remove events
2213
function removeMotionEvents(element: HTMLElement) {
2314
if (element) {

src/hooks/useStatus.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { useEvent } from 'rc-util';
12
import useState from 'rc-util/lib/hooks/useState';
23
import * as React from 'react';
34
import { useEffect, useRef } from 'react';
@@ -72,7 +73,13 @@ export default function useStatus(
7273
setStyle(null, true);
7374
}
7475

75-
function onInternalMotionEnd(event: MotionEvent) {
76+
const onInternalMotionEnd = useEvent((event: MotionEvent) => {
77+
// Do nothing since not in any transition status.
78+
// This may happen when `motionDeadline` trigger.
79+
if (status === STATUS_NONE) {
80+
return;
81+
}
82+
7683
const element = getDomElement();
7784
if (event && !event.deadline && event.target !== element) {
7885
// event exists
@@ -93,10 +100,10 @@ export default function useStatus(
93100
}
94101

95102
// Only update status when `canEnd` and not destroyed
96-
if (status !== STATUS_NONE && currentActive && canEnd !== false) {
103+
if (currentActive && canEnd !== false) {
97104
updateMotionEndStatus();
98105
}
99-
}
106+
});
100107

101108
const [patchMotionEvents] = useDomMotionEvents(onInternalMotionEnd);
102109

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

154-
if (step === STEP_ACTIVE) {
161+
if (step === STEP_ACTIVE && status !== STATUS_NONE) {
155162
// Patch events when motion needed
156163
patchMotionEvents(getDomElement());
157164

src/util/diff.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,10 @@ export type DiffStatus =
88
| typeof STATUS_REMOVE
99
| typeof STATUS_REMOVED;
1010

11+
type RawKeyType = string | number;
12+
1113
export interface KeyObject {
12-
key: React.Key;
14+
key: RawKeyType;
1315
status?: DiffStatus;
1416
}
1517

@@ -18,7 +20,7 @@ export function wrapKeyToObject(key: React.Key | KeyObject) {
1820
if (key && typeof key === 'object' && 'key' in key) {
1921
keyObj = key;
2022
} else {
21-
keyObj = { key: key as React.Key };
23+
keyObj = { key: key as RawKeyType };
2224
}
2325
return {
2426
...keyObj,
@@ -90,7 +92,7 @@ export function diffKeys(
9092
* Merge same key when it remove and add again:
9193
* [1 - add, 2 - keep, 1 - remove] -> [1 - keep, 2 - keep]
9294
*/
93-
const keys = {};
95+
const keys: Record<RawKeyType, number> = {};
9496
list.forEach(({ key }) => {
9597
keys[key] = (keys[key] || 0) + 1;
9698
});

tests/CSSMotion.spec.tsx

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,10 @@
22
react/no-render-return-value, max-classes-per-file,
33
react/prefer-stateless-function, react/no-multi-comp
44
*/
5-
import { fireEvent, render } from '@testing-library/react';
5+
import { act, fireEvent, render } from '@testing-library/react';
66
import classNames from 'classnames';
77
import React from 'react';
88
import ReactDOM from 'react-dom';
9-
import { act } from 'react-dom/test-utils';
109
import type { CSSMotionProps } from '../src';
1110
import { Provider } from '../src';
1211
import RefCSSMotion, { genCSSMotion } from '../src/CSSMotion';
@@ -342,6 +341,60 @@ describe('CSSMotion', () => {
342341
return <div {...props} />;
343342
}),
344343
);
344+
345+
it('not warning on StrictMode', () => {
346+
const onLeaveEnd = jest.fn();
347+
const errorSpy = jest.spyOn(console, 'error');
348+
349+
const renderDemo = (visible: boolean) => (
350+
<React.StrictMode>
351+
<CSSMotion
352+
motionName="transition"
353+
motionDeadline={1000}
354+
onLeaveEnd={onLeaveEnd}
355+
visible={visible}
356+
motionAppear={false}
357+
motionLeave={true}
358+
>
359+
{({ style, className }) => (
360+
<div
361+
style={style}
362+
className={classNames('motion-box', className)}
363+
/>
364+
)}
365+
</CSSMotion>
366+
</React.StrictMode>
367+
);
368+
369+
const { rerender, container } = render(renderDemo(true));
370+
act(() => {
371+
jest.advanceTimersByTime(100000);
372+
});
373+
374+
// Leave
375+
rerender(renderDemo(false));
376+
act(() => {
377+
jest.advanceTimersByTime(500);
378+
});
379+
380+
// Motion end
381+
fireEvent.transitionEnd(
382+
container.querySelector('.transition-leave-active'),
383+
);
384+
act(() => {
385+
jest.advanceTimersByTime(100);
386+
});
387+
388+
// Another timeout
389+
act(() => {
390+
jest.advanceTimersByTime(1000);
391+
});
392+
393+
expect(onLeaveEnd).toHaveBeenCalledTimes(1);
394+
expect(errorSpy).not.toHaveBeenCalled();
395+
396+
errorSpy.mockRestore();
397+
});
345398
});
346399

347400
it('not crash when no children', () => {

0 commit comments

Comments
 (0)