Skip to content

Commit

Permalink
fix: Invalid memoized context value in LocaleProvider (#33723)
Browse files Browse the repository at this point in the history
* fix: Invalid memoized context value in LocaleProvider

* Add button type in test case.

* fix: Invalid memoized context value in Anchor
  • Loading branch information
mrwd2009 committed Jan 14, 2022
1 parent f81e392 commit f53e615
Show file tree
Hide file tree
Showing 4 changed files with 121 additions and 11 deletions.
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>;
}
}

0 comments on commit f53e615

Please sign in to comment.