From f94b6edf9d87d9c6a856e22a159616e870db5bb4 Mon Sep 17 00:00:00 2001 From: Haroen Viaene Date: Tue, 2 Apr 2019 17:22:42 +0200 Subject: [PATCH] chore(life cycle): add comments where change is necessary --- examples/next/pages/index.js | 1 + examples/react-native/src/Home.js | 2 -- examples/react-native/src/Price.js | 1 + examples/react-router-v3/src/App.js | 1 + examples/react-router/src/App.js | 1 + packages/react-instantsearch-core/src/components/Index.tsx | 1 + .../react-instantsearch-core/src/components/InstantSearch.tsx | 3 ++- packages/react-instantsearch-core/src/core/createConnector.tsx | 2 ++ .../react-instantsearch-core/src/core/createInstantSearch.js | 2 ++ .../src/components/PanelCallbackHandler.js | 2 ++ packages/react-instantsearch-dom/src/components/RangeInput.js | 2 ++ packages/react-instantsearch-dom/src/components/SearchBox.js | 2 ++ packages/react-instantsearch-dom/src/widgets/RangeSlider.js | 1 + stories/3rdPartyIntegrations.stories.js | 1 + website/examples/tourism/App.js | 1 + 15 files changed, 20 insertions(+), 3 deletions(-) diff --git a/examples/next/pages/index.js b/examples/next/pages/index.js index e9343eedaf..994061d623 100644 --- a/examples/next/pages/index.js +++ b/examples/next/pages/index.js @@ -50,6 +50,7 @@ export default class extends React.Component { } componentWillReceiveProps() { + // @TODO: probably derived state this.setState({ searchState: qs.parse(window.location.search.slice(1)) }); } diff --git a/examples/react-native/src/Home.js b/examples/react-native/src/Home.js index 943b444fe0..ebe213a1d0 100644 --- a/examples/react-native/src/Home.js +++ b/examples/react-native/src/Home.js @@ -121,8 +121,6 @@ class Home extends Component { }; } - componentWillReceiveProps() {} - onSearchStateChange = nextState => { this.setState({ searchState: { ...this.state.searchState, ...nextState } }); }; diff --git a/examples/react-native/src/Price.js b/examples/react-native/src/Price.js index 8804d59e90..acc4eb71e0 100644 --- a/examples/react-native/src/Price.js +++ b/examples/react-native/src/Price.js @@ -81,6 +81,7 @@ class Range extends React.Component { }; } componentWillReceiveProps(sliderState) { + // @TODO: derived state if (sliderState.canRefine) { this.setState({ currentValues: { diff --git a/examples/react-router-v3/src/App.js b/examples/react-router-v3/src/App.js index 931119025e..0c6220db5c 100644 --- a/examples/react-router-v3/src/App.js +++ b/examples/react-router-v3/src/App.js @@ -25,6 +25,7 @@ class App extends Component { } componentWillReceiveProps() { + // @TODO: derived state this.setState({ searchState: qs.parse(this.props.router.location.query) }); } diff --git a/examples/react-router/src/App.js b/examples/react-router/src/App.js index ce0aa9c764..60c6064f76 100644 --- a/examples/react-router/src/App.js +++ b/examples/react-router/src/App.js @@ -32,6 +32,7 @@ class App extends Component { } componentWillReceiveProps(props) { + // @TODO: derived state if (props.location !== this.props.location) { this.setState({ searchState: urlToSearchState(props.location) }); } diff --git a/packages/react-instantsearch-core/src/components/Index.tsx b/packages/react-instantsearch-core/src/components/Index.tsx index 1e581c74a1..9adecc225d 100644 --- a/packages/react-instantsearch-core/src/components/Index.tsx +++ b/packages/react-instantsearch-core/src/components/Index.tsx @@ -95,6 +95,7 @@ class Index extends Component { } componentWillReceiveProps(nextProps: InnerProps) { + // @TODO: DidUpdate if (this.props.indexName !== nextProps.indexName) { this.props.contextValue.widgetsManager.update(); } diff --git a/packages/react-instantsearch-core/src/components/InstantSearch.tsx b/packages/react-instantsearch-core/src/components/InstantSearch.tsx index 3ce7767647..9340f86b10 100644 --- a/packages/react-instantsearch-core/src/components/InstantSearch.tsx +++ b/packages/react-instantsearch-core/src/components/InstantSearch.tsx @@ -175,7 +175,8 @@ class InstantSearch extends Component { }; } - componentWillReceiveProps(nextProps: Props) { + componentWillReceiveProps(nextProps) { + // @TODO: DidUpdate validateNextProps(this.props, nextProps); if (this.props.indexName !== nextProps.indexName) { diff --git a/packages/react-instantsearch-core/src/core/createConnector.tsx b/packages/react-instantsearch-core/src/core/createConnector.tsx index 58d22f7743..82bb3b9456 100644 --- a/packages/react-instantsearch-core/src/core/createConnector.tsx +++ b/packages/react-instantsearch-core/src/core/createConnector.tsx @@ -101,6 +101,7 @@ export function createConnectorWithoutContext( }; componentWillMount() { + // @TODO: constructor if (connectorDesc.getSearchParameters) { this.props.contextValue.onSearchParameters( connectorDesc.getSearchParameters.bind(this), @@ -130,6 +131,7 @@ export function createConnectorWithoutContext( } componentWillReceiveProps(nextProps) { + // @TODO: split in Derived for state & DidUpdate if (!isEqual(this.props, nextProps)) { this.setState({ providedProps: this.getProvidedProps(nextProps), diff --git a/packages/react-instantsearch-core/src/core/createInstantSearch.js b/packages/react-instantsearch-core/src/core/createInstantSearch.js index b0769c0056..0c72e0608f 100644 --- a/packages/react-instantsearch-core/src/core/createInstantSearch.js +++ b/packages/react-instantsearch-core/src/core/createInstantSearch.js @@ -75,6 +75,7 @@ export default function createInstantSearch(defaultAlgoliaClient, root) { } componentWillReceiveProps(nextProps) { + // @TODO: DidUpdate, but might be too late? If needs to be before rendering: derived State for client const props = this.props; if (nextProps.searchClient) { @@ -89,6 +90,7 @@ export default function createInstantSearch(defaultAlgoliaClient, root) { } if (typeof this.client.addAlgoliaAgent === 'function') { + // @TODO: maybe DidUpdate if that's not too late, otherwise side effect in derived state this.client.addAlgoliaAgent(`react (${React.version})`); this.client.addAlgoliaAgent(`react-instantsearch (${version})`); } diff --git a/packages/react-instantsearch-dom/src/components/PanelCallbackHandler.js b/packages/react-instantsearch-dom/src/components/PanelCallbackHandler.js index 29d66f596d..308915cb6c 100644 --- a/packages/react-instantsearch-dom/src/components/PanelCallbackHandler.js +++ b/packages/react-instantsearch-dom/src/components/PanelCallbackHandler.js @@ -12,12 +12,14 @@ class PanelCallbackHandler extends Component { }; componentWillMount() { + // @TODO: didMount? -> really test this if (this.context.setCanRefine) { this.context.setCanRefine(this.props.canRefine); } } componentWillReceiveProps(nextProps) { + // @TODO: DidUpdate if ( this.context.setCanRefine && this.props.canRefine !== nextProps.canRefine diff --git a/packages/react-instantsearch-dom/src/components/RangeInput.js b/packages/react-instantsearch-dom/src/components/RangeInput.js index 60543ab1ab..09cc718238 100644 --- a/packages/react-instantsearch-dom/src/components/RangeInput.js +++ b/packages/react-instantsearch-dom/src/components/RangeInput.js @@ -33,6 +33,8 @@ export class RawRangeInput extends Component { } componentWillReceiveProps(nextProps) { + // @TODO: Render, worst case in Derived State + // In react@16.0.0 the call to setState on the inputs trigger this lifecycle hook // because the context has changed (for react). I don't think that the bug is related // to react because I failed to reproduce it with a simple hierarchy of components. diff --git a/packages/react-instantsearch-dom/src/components/SearchBox.js b/packages/react-instantsearch-dom/src/components/SearchBox.js index 52329a254e..08f66f7baf 100644 --- a/packages/react-instantsearch-dom/src/components/SearchBox.js +++ b/packages/react-instantsearch-dom/src/components/SearchBox.js @@ -116,6 +116,8 @@ class SearchBox extends Component { } componentWillReceiveProps(nextProps) { + // @TODO: should component maybe be controlled, otherwise Derived + // Reset query when the searchParameters query has changed. // This is kind of an anti-pattern (props in state), but it works here // since we know for sure that searchParameters having changed means a diff --git a/packages/react-instantsearch-dom/src/widgets/RangeSlider.js b/packages/react-instantsearch-dom/src/widgets/RangeSlider.js index c8fa2be4e2..3ea3f0d926 100644 --- a/packages/react-instantsearch-dom/src/widgets/RangeSlider.js +++ b/packages/react-instantsearch-dom/src/widgets/RangeSlider.js @@ -30,6 +30,7 @@ class Range extends React.Component { state = { currentValues: { min: this.props.min, max: this.props.max } }; componentWillReceiveProps(sliderState) { + // @TODO: Derived State, maybe render if (sliderState.canRefine) { this.setState({ currentValues: { diff --git a/stories/3rdPartyIntegrations.stories.js b/stories/3rdPartyIntegrations.stories.js index e6a45fc2b8..0a4219dfaa 100644 --- a/stories/3rdPartyIntegrations.stories.js +++ b/stories/3rdPartyIntegrations.stories.js @@ -30,6 +30,7 @@ class Range extends Component { state = { currentValues: { min: this.props.min, max: this.props.max } }; componentWillReceiveProps(sliderState) { + // @TODO: Derived State, maybe render if (sliderState.canRefine) { this.setState({ currentValues: { diff --git a/website/examples/tourism/App.js b/website/examples/tourism/App.js index d650391e29..a3b8c3c76f 100644 --- a/website/examples/tourism/App.js +++ b/website/examples/tourism/App.js @@ -292,6 +292,7 @@ class Range extends Component { state = { currentValues: { min: this.props.min, max: this.props.max } }; componentWillReceiveProps(sliderState) { + // @TODO: derived state if (sliderState.canRefine) { this.setState({ currentValues: {