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

shouldUpdate does not work if parent triggers child render #729

Open
TrevorHinesley opened this issue Aug 21, 2018 · 3 comments
Open

shouldUpdate does not work if parent triggers child render #729

TrevorHinesley opened this issue Aug 21, 2018 · 3 comments

Comments

@TrevorHinesley
Copy link

TrevorHinesley commented Aug 21, 2018

In React's shouldComponentUpdate callback, if false is returned, the component will not re-render, even if its parent's props change (i.e. when a parent forces its tree of children to re-render because its own props changed, those children will not render if their shouldComponentUpdate returns false).

After much debugging today, I realized that shouldUpdate not does behave the same way. If the child is re-rendering because its props changed and not because of a parent's props changing (which in turn re-renders its tree), it works fine. But if the child is re-rendering because its parent's props changed (re-rendering the parent's tree of children), it overlooks shouldUpdate and the children re-render despite the return value of shouldUpdate. The same is true for lifecycle's componentShouldUpdate from what I can tell.

I am not sure if onlyUpdateForKeys or onlyUpdateForProps have the same issue.

@andrewgreenh
Copy link

I'm having this issue in my work application aswell. I tried to reproduce the issue in a codesandbox but couldn't O.o
I'm using the exact same versions of recompose and react.

https://codesandbox.io/s/zk345znwx4

When this bug ocurrs, every click on the button should send the message to the console again.

@mufasa71
Copy link

I can't reproduce it.

@rscotten
Copy link

rscotten commented May 9, 2019

This happened to me too and the same behavior happens for onlyUpdateForKeys. I'm using shouldUpdate or onlyUpdateForKeys in an HOC that is a component in another HOC. When the parent HOC updates, the child HOC updates regardless of the return value of shouldUpdate or onlyUpdateForKeys;

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants