Skip to content

Commit

Permalink
fix: SubMenu in React18 sync problem (#537)
Browse files Browse the repository at this point in the history
Co-authored-by: huangkairan <wb-hkr877030@alibaba-inc.com>
  • Loading branch information
huangkairan and huangkairan authored Oct 27, 2022
1 parent d98ed07 commit 0cf3d90
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 8 deletions.
19 changes: 12 additions & 7 deletions src/Menu.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as React from 'react';
import { flushSync } from 'react-dom';
import type { CSSMotionProps } from 'rc-motion';
import classNames from 'classnames';
import shallowEqual from 'shallowequal';
Expand Down Expand Up @@ -271,13 +272,18 @@ const Menu = React.forwardRef<MenuRef, MenuProps>((props, ref) => {
});

const triggerOpenKeys = (keys: string[]) => {
setMergedOpenKeys(keys);
// Prevent React18 auto batch since trigger openKeys on same time
// which makes mergedOpenKeys closure problem
flushSync(() => {
setMergedOpenKeys(keys);
});
onOpenChange?.(keys);
};

// >>>>> Cache & Reset open keys when inlineCollapsed changed
const [inlineCacheOpenKeys, setInlineCacheOpenKeys] =
React.useState(mergedOpenKeys);
const [inlineCacheOpenKeys, setInlineCacheOpenKeys] = React.useState(
mergedOpenKeys,
);

const isInlineMode = mergedMode === 'inline';

Expand Down Expand Up @@ -329,10 +335,9 @@ const Menu = React.forwardRef<MenuRef, MenuProps>((props, ref) => {
[registerPath, unregisterPath],
);

const pathUserContext = React.useMemo(
() => ({ isSubPathKey }),
[isSubPathKey],
);
const pathUserContext = React.useMemo(() => ({ isSubPathKey }), [
isSubPathKey,
]);

React.useEffect(() => {
refreshOverflowKeys(
Expand Down
37 changes: 36 additions & 1 deletion tests/React18.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* eslint-disable no-undef */
import React from 'react';
import { act, render } from '@testing-library/react';
import { act, fireEvent, render } from '@testing-library/react';
import { sleep } from './util';
import Menu, { MenuItem, SubMenu } from '../src';
import type { MenuProps } from '../src';

Expand Down Expand Up @@ -55,5 +56,39 @@ describe('React18', () => {
.querySelector('.rc-menu-submenu-title').textContent,
).toEqual('submenu1');
});

it('prevent React 18 auto batch', async () => {
const handleOpenChange = jest.fn();
const { container } = render(
<Menu onOpenChange={handleOpenChange}>
<SubMenu title="s1">
<MenuItem key="1">1</MenuItem>
</SubMenu>
<SubMenu title="s2">
<MenuItem key="2">2</MenuItem>
</SubMenu>
</Menu>,
);

// Enter
fireEvent.mouseEnter(container.querySelector('.rc-menu-submenu-title'));
runAllTimer();
expect(container.querySelector('.rc-menu-submenu-open')).toBeTruthy();
// Leave
fireEvent.mouseLeave(container.querySelector('.rc-menu-submenu-title'));
act(() => {
jest.runAllTimers();
});
expect(container.querySelector('.rc-menu-submenu-open')).toBeFalsy();
await act(async () => {
await sleep();
});
// Enter
fireEvent.mouseEnter(
container.querySelectorAll('.rc-menu-submenu-title')[1],
);
jest.runAllTimers();
expect(container.querySelector('.rc-menu-submenu-open')).toBeTruthy();
});
});
/* eslint-enable */
11 changes: 11 additions & 0 deletions tests/util.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { act } from '@testing-library/react';
export function isActive(container: HTMLElement, index: number, active = true) {
const checker = expect(container.querySelectorAll('li.rc-menu-item')[index]);

Expand All @@ -11,3 +12,13 @@ export function isActive(container: HTMLElement, index: number, active = true) {
export function last(elements: NodeListOf<Element>) {
return elements[elements.length - 1];
}

const globalTimeout = global.setTimeout;

export const sleep = async (timeout = 0) => {
await act(async () => {
await new Promise(resolve => {
globalTimeout(resolve, timeout);
});
});
};

0 comments on commit 0cf3d90

Please sign in to comment.