Merge pull request #33432 from MadCcc/dev

fix: not allow setState in Popconfirm if unmounted
This commit is contained in:
MadCcc 2021-12-29 17:49:15 +08:00 committed by GitHub
commit 7df1d52dee
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 90 additions and 4 deletions

View File

@ -1,6 +1,7 @@
import * as React from 'react';
import Button from '../button';
import { LegacyButtonType, ButtonProps, convertLegacyProps } from '../button/button';
import useDestroyed from './hooks/useDestroyed';
export interface ActionButtonProps {
type?: LegacyButtonType;
@ -20,6 +21,7 @@ function isThenable(thing?: PromiseLike<any>): boolean {
const ActionButton: React.FC<ActionButtonProps> = props => {
const clickedRef = React.useRef<boolean>(false);
const ref = React.useRef<any>();
const isDestroyed = useDestroyed();
const [loading, setLoading] = React.useState<ButtonProps['loading']>(false);
React.useEffect(() => {
@ -43,7 +45,9 @@ const ActionButton: React.FC<ActionButtonProps> = props => {
setLoading(true);
returnValueOfOnOk!.then(
(...args: any[]) => {
setLoading(false);
if (!isDestroyed()) {
setLoading(false);
}
close(...args);
clickedRef.current = false;
},
@ -52,7 +56,9 @@ const ActionButton: React.FC<ActionButtonProps> = props => {
// eslint-disable-next-line no-console
console.error(e);
// See: https://github.com/ant-design/ant-design/issues/6183
setLoading(false);
if (!isDestroyed()) {
setLoading(false);
}
clickedRef.current = false;
},
);

View File

@ -0,0 +1,20 @@
import { mount } from 'enzyme';
import React from 'react';
import useDestroyed from '../hooks/useDestroyed';
describe('useMounted', () => {
it('should work properly', () => {
let isDestroyed = null;
const AutoUnmounted = () => {
isDestroyed = useDestroyed();
return <div>Mounted</div>;
};
const wrapper = mount(<AutoUnmounted />);
expect(isDestroyed()).toBeFalsy();
wrapper.unmount();
expect(isDestroyed()).toBeTruthy();
});
});

View File

@ -0,0 +1,14 @@
import * as React from 'react';
export default function useDestroyed() {
const mountedRef = React.useRef<boolean>(true);
React.useEffect(
() => () => {
mountedRef.current = false;
},
[],
);
return () => !mountedRef.current;
}

View File

@ -5,6 +5,7 @@ import Popconfirm from '..';
import mountTest from '../../../tests/shared/mountTest';
import { sleep } from '../../../tests/utils';
import rtlTest from '../../../tests/shared/rtlTest';
import Button from '../../button';
describe('Popconfirm', () => {
mountTest(Popconfirm);
@ -223,4 +224,45 @@ describe('Popconfirm', () => {
triggerNode.simulate('keydown', { key: 'Escape', keyCode: 27 });
expect(onVisibleChange).toHaveBeenLastCalledWith(false, eventObject);
});
it('should not warn memory leaking if setState in async callback', async () => {
const error = jest.spyOn(console, 'error');
const Test = () => {
const [show, setShow] = React.useState(true);
if (show) {
return (
<Popconfirm
title="will unmount"
onConfirm={() =>
new Promise(resolve => {
setTimeout(() => {
setShow(false);
resolve();
}, 300);
})
}
>
<Button className="clickTarget">Test</Button>
</Popconfirm>
);
}
return <Button>Unmounted</Button>;
};
const wrapper = mount(
<div>
<Test />
</div>,
);
expect(wrapper.text()).toEqual('Test');
const triggerNode = wrapper.find('.clickTarget').at(0);
triggerNode.simulate('click');
wrapper.find('.ant-btn-primary').simulate('click');
await sleep(500);
expect(wrapper.text()).toEqual('Unmounted');
expect(error).not.toHaveBeenCalled();
});
});

View File

@ -13,6 +13,7 @@ import { getRenderPropValue, RenderFunction } from '../_util/getRenderPropValue'
import { cloneElement } from '../_util/reactNode';
import { getTransitionName } from '../_util/motion';
import ActionButton from '../_util/ActionButton';
import useDestroyed from '../_util/hooks/useDestroyed';
export interface PopconfirmProps extends AbstractTooltipProps {
title: React.ReactNode | RenderFunction;
@ -48,12 +49,15 @@ const Popconfirm = React.forwardRef<unknown, PopconfirmProps>((props, ref) => {
defaultValue: props.defaultVisible,
});
const isDestroyed = useDestroyed();
const settingVisible = (
value: boolean,
e?: React.MouseEvent<HTMLButtonElement> | React.KeyboardEvent<HTMLDivElement>,
) => {
setVisible(value);
if (!isDestroyed()) {
setVisible(value);
}
props.onVisibleChange?.(value, e);
};