Skip to content
This repository has been archived by the owner on Dec 30, 2022. It is now read-only.

Commit

Permalink
fix(createConnector): new life cycles (#2357)
Browse files Browse the repository at this point in the history
* fix(createConnector): new life cycles

- move willMount -> constructor
- move willReceiveProps -> didUpdate

This didn't have an impact on more requests, but did cause one more render if you update a prop

IFW-671

* remove comment

* test(sCU): shouldComponentUpdate is called two separate times now
  • Loading branch information
Haroenv committed Jun 27, 2019
1 parent addbb01 commit fc10640
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,7 @@ describe('createConnector', () => {

const context = createFakeContext();

const wrapper = shallow(<Connected {...props} contextValue={context} />, {
disableLifecycleMethods: true,
});
const wrapper = shallow(<Connected {...props} contextValue={context} />);

// Simulate props change before mount
wrapper.setProps({ hello: 'again' });
Expand Down Expand Up @@ -337,13 +335,13 @@ describe('createConnector', () => {

const props = { hello: 'there' };
const context = createFakeContext();
const wrapper = shallow(<Connected {...props} contextValue={context} />);
const wrapper = mount(<Connected {...props} contextValue={context} />);

expect(shouldComponentUpdate).toHaveBeenCalledTimes(0);

wrapper.setProps({ hello: 'here' });

expect(shouldComponentUpdate).toHaveBeenCalledTimes(1);
expect(shouldComponentUpdate).toHaveBeenCalledTimes(2);
expect(shouldComponentUpdate).toHaveBeenCalledWith(
{
hello: 'there',
Expand All @@ -359,6 +357,29 @@ describe('createConnector', () => {
contextValue: context,
},
},
{
providedProps: {
hello: 'there',
contextValue: context,
},
}
);

expect(shouldComponentUpdate).toHaveBeenCalledWith(
{
hello: 'here',
contextValue: context,
},
{
hello: 'here',
contextValue: context,
},
{
providedProps: {
hello: 'there',
contextValue: context,
},
},
{
providedProps: {
hello: 'here',
Expand Down Expand Up @@ -686,7 +707,7 @@ describe('createConnector', () => {
expect(update).toHaveBeenCalledTimes(0);
});

it('triggers an onSearchStateChange on props change with transitationState', () => {
it('triggers an onSearchStateChange on props change with transitionState', () => {
const transitionState = jest.fn(function() {
return this.props;
});
Expand Down Expand Up @@ -723,7 +744,7 @@ describe('createConnector', () => {

expect(onSearchStateChange).toHaveBeenCalledTimes(1);
expect(onSearchStateChange).toHaveBeenCalledWith({
hello: 'there', // the instance didn't have the next props yet
hello: 'again',
contextValue: context,
});

Expand All @@ -735,7 +756,7 @@ describe('createConnector', () => {
);
});

it('does not trigger an onSearchStateChange on props change wihtout transitionState', () => {
it('does not trigger an onSearchStateChange on props change without transitionState', () => {
const Connected = createConnectorWithoutContext({
displayName: 'Connector',
getProvidedProps: () => {},
Expand Down
50 changes: 26 additions & 24 deletions packages/react-instantsearch-core/src/core/createConnector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,9 @@ export function createConnectorWithoutContext(
providedProps: this.getProvidedProps(this.props),
};

componentWillMount() {
constructor(props: ConnectorProps) {
super(props);

if (connectorDesc.getSearchParameters) {
this.props.contextValue.onSearchParameters(
connectorDesc.getSearchParameters.bind(this),
Expand Down Expand Up @@ -129,29 +131,6 @@ export function createConnectorWithoutContext(
}
}

componentWillReceiveProps(nextProps) {
if (!isEqual(this.props, nextProps)) {
this.setState({
providedProps: this.getProvidedProps(nextProps),
});

if (isWidget) {
this.props.contextValue.widgetsManager.update();

if (typeof connectorDesc.transitionState === 'function') {
this.props.contextValue.onSearchStateChange(
connectorDesc.transitionState.call(
this,
nextProps,
this.props.contextValue.store.getState().widgets,
this.props.contextValue.store.getState().widgets
)
);
}
}
}
}

shouldComponentUpdate(nextProps, nextState) {
if (typeof connectorDesc.shouldComponentUpdate === 'function') {
return connectorDesc.shouldComponentUpdate.call(
Expand Down Expand Up @@ -181,6 +160,29 @@ export function createConnectorWithoutContext(
);
}

componentDidUpdate(prevProps) {
if (!isEqual(prevProps, this.props)) {
this.setState({
providedProps: this.getProvidedProps(this.props),
});

if (isWidget) {
this.props.contextValue.widgetsManager.update();

if (typeof connectorDesc.transitionState === 'function') {
this.props.contextValue.onSearchStateChange(
connectorDesc.transitionState.call(
this,
this.props,
this.props.contextValue.store.getState().widgets,
this.props.contextValue.store.getState().widgets
)
);
}
}
}
}

componentWillUnmount() {
this.isUnmounting = true;

Expand Down

0 comments on commit fc10640

Please sign in to comment.