-
Notifications
You must be signed in to change notification settings - Fork 516
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
Avoid to rely on shouldComponentUpdate in component #3056
Labels
Comments
samouss
changed the title
Avoid to rely on SCU in widget
Avoid to rely on SCU in component
Aug 8, 2018
What is SCU @samouss ? |
samouss
changed the title
Avoid to rely on SCU in component
Avoid to rely on shouldComponentUpdate in component
Aug 10, 2018
I've update the title and the description. |
I'd go for removing the hook, as it is a very straightforward solution. If we find evidence that this particular widget is causing performance issues, we can work on it then. |
bobylito
pushed a commit
that referenced
this issue
Aug 20, 2018
This removes the implementation of shouldComponentUpdate. This can be seen as a regression but given that it's a pretty simple widget, the impact shouldn't be noticeable. Fix #3056
bobylito
added a commit
that referenced
this issue
Aug 20, 2018
This removes the implementation of shouldComponentUpdate. This can be seen as a regression but given that it's a pretty simple widget, the impact shouldn't be noticeable. Fix #3056
bobylito
pushed a commit
that referenced
this issue
Sep 10, 2018
<a name=2.10.2></a> ## [2.10.2](v2.10.1...v2.10.2) (2018-09-10) ### Bug Fixes * **searchbox:** Add missing color to searchbox input field ([#3086](#3086)) ([62b852a](62b852a)), closes [#3075](#3075) * **Stats:** let the widget render on all values ([#3070](#3070)) ([cd8f17e](cd8f17e)), closes [#3056](#3056)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug 🐛
The stats widget re-renders only when the
nbHits
orprocessingTimeMS
changes. It can lead to issue when you are using the page number in the template for example. When you click on the next page with the Pagination widget only the page value changed not the other ones. The shouldComponentUpdate (sCU) function of the widget will return false and so prevent the re-render.https://github.com/algolia/instantsearch.js/blob/33fa09835df8678085cb7f382b4973eaa1f9b20f/src/components/Stats/Stats.js#L9-L14
To Reproduce 🔍
Expected behavior 💭
We should trigger the re-render. The quick fix is to add more values inside the condition. But do we really need the sCU hook on those components? To have a better idea we can track the effect on the performance with / without the hook.
The text was updated successfully, but these errors were encountered: