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: Invalid memoized context value in LocaleProvider #33723

Merged
merged 3 commits into from Jan 14, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 11 additions & 7 deletions components/anchor/Anchor.tsx
Expand Up @@ -257,6 +257,16 @@ export default class Anchor extends React.Component<AnchorProps, AnchorState, Co
}
};

getMemoizedContextValue = memoizeOne(
(link: AntAnchor['activeLink'], onClickFn: AnchorProps['onClick']): AntAnchor => ({
registerLink: this.registerLink,
unregisterLink: this.unregisterLink,
scrollTo: this.handleScrollTo,
activeLink: link,
onClick: onClickFn,
}),
);

render() {
const { getPrefixCls, direction } = this.context;
const {
Expand Down Expand Up @@ -310,13 +320,7 @@ export default class Anchor extends React.Component<AnchorProps, AnchorState, Co
</div>
);

const contextValue = memoizeOne((link, onClickFn) => ({
registerLink: this.registerLink,
unregisterLink: this.unregisterLink,
scrollTo: this.handleScrollTo,
activeLink: link,
onClick: onClickFn,
}))(activeLink, onClick);
const contextValue = this.getMemoizedContextValue(activeLink, onClick);

return (
<AnchorContext.Provider value={contextValue}>
Expand Down
51 changes: 51 additions & 0 deletions components/anchor/__tests__/cached-context.test.tsx
@@ -0,0 +1,51 @@
import React, { memo, useState, useRef, useContext } from 'react';
import { mount } from 'enzyme';
import Anchor from '../Anchor';
import AnchorContext from '../context';

// we use'memo' here in order to only render inner component while context changed.
const CacheInner = memo(() => {
const countRef = useRef(0);
countRef.current++;
// subscribe anchor context
useContext(AnchorContext);
return (
<div>
Child Rendering Count: <span id="child_count">{countRef.current}</span>
</div>
);
});

const CacheOuter = () => {
// We use 'useState' here in order to trigger parent component rendering.
const [count, setCount] = useState(1);
const handleClick = () => {
setCount(count + 1);
};
// During each rendering phase, the cached context value returned from method 'Anchor#getMemoizedContextValue' will take effect.
// So 'CacheInner' component won't rerender.
return (
<div>
<button type="button" onClick={handleClick} id="parent_btn">
Click
</button>
Parent Rendering Count: <span id="parent_count">{count}</span>
<Anchor affix={false}>
<CacheInner />
</Anchor>
</div>
);
};

it("Rendering on Anchor without changed AnchorContext won't trigger rendering on child component.", () => {
const wrapper = mount(<CacheOuter />);
const childCount = wrapper.find('#child_count').text();
wrapper.find('#parent_btn').at(0).simulate('click');
expect(wrapper.find('#parent_count').text()).toBe('2');
// child component won't rerender
expect(wrapper.find('#child_count').text()).toBe(childCount);
wrapper.find('#parent_btn').at(0).simulate('click');
expect(wrapper.find('#parent_count').text()).toBe('3');
// child component won't rerender
expect(wrapper.find('#child_count').text()).toBe(childCount);
});
53 changes: 53 additions & 0 deletions components/locale-provider/__tests__/cached-context.test.tsx
@@ -0,0 +1,53 @@
import React, { memo, useState, useRef, useContext } from 'react';
import { mount } from 'enzyme';
import LocaleProvider, { ANT_MARK } from '..';
import LocaleContext from '../context';

const defaultLocale = {
locale: 'locale',
};
// we use'memo' here in order to only render inner component while context changed.
const CacheInner = memo(() => {
const countRef = useRef(0);
countRef.current++;
// subscribe locale context
useContext(LocaleContext);
return (
<div>
Child Rendering Count: <span id="child_count">{countRef.current}</span>
</div>
);
});

const CacheOuter = () => {
// We use 'useState' here in order to trigger parent component rendering.
const [count, setCount] = useState(1);
const handleClick = () => {
setCount(count + 1);
};
// During each rendering phase, the cached context value returned from method 'LocaleProvider#getMemoizedContextValue' will take effect.
// So 'CacheInner' component won't rerender.
return (
<div>
<button type="button" onClick={handleClick} id="parent_btn">
Click
</button>
Parent Rendering Count: <span id="parent_count">{count}</span>
<LocaleProvider locale={defaultLocale} _ANT_MARK__={ANT_MARK}>
<CacheInner />
</LocaleProvider>
</div>
);
};

it("Rendering on LocaleProvider won't trigger rendering on child component.", () => {
const wrapper = mount(<CacheOuter />);
wrapper.find('#parent_btn').at(0).simulate('click');
expect(wrapper.find('#parent_count').text()).toBe('2');
// child component won't rerender
expect(wrapper.find('#child_count').text()).toBe('1');
wrapper.find('#parent_btn').at(0).simulate('click');
expect(wrapper.find('#parent_count').text()).toBe('3');
// child component won't rerender
expect(wrapper.find('#child_count').text()).toBe('1');
});
10 changes: 6 additions & 4 deletions components/locale-provider/index.tsx
Expand Up @@ -78,12 +78,14 @@ export default class LocaleProvider extends React.Component<LocaleProviderProps,
changeConfirmLocale();
}

getMemoizedContextValue = memoizeOne((localeValue: Locale): Locale & { exist?: boolean } => ({
...localeValue,
exist: true,
}));

render() {
const { locale, children } = this.props;
const contextValue = memoizeOne(localeValue => ({
...localeValue,
exist: true,
}))(locale);
const contextValue = this.getMemoizedContextValue(locale);
return <LocaleContext.Provider value={contextValue}>{children}</LocaleContext.Provider>;
}
}