Skip to content
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

shouldComponentUpdate returns true when the item is visible #97

Open
MatanBobi opened this issue Jun 7, 2017 · 1 comment
Open

shouldComponentUpdate returns true when the item is visible #97

MatanBobi opened this issue Jun 7, 2017 · 1 comment

Comments

@MatanBobi
Copy link

MatanBobi commented Jun 7, 2017

Hey there!
Im trying to understand whats the reason for the shouldcomponentupdate logic that you are applying.

  shouldComponentUpdate(_nextProps, nextState) {
    return nextState.visible;
  }

If the current state is visible and the next one is also visible then why should i update the component?
i would think that the wanted behavior should be:

  shouldComponentUpdate(_nextProps, nextState) {
    return !this.state.visible && nextState.visible;
  }

What am i missing here?
Thanks!

@MatanBobi MatanBobi changed the title shouldComponentUpdate returns true when the image is visible shouldComponentUpdate returns true when the item is visible Jun 7, 2017
@sfrieson-tm
Copy link

I'm inclined to agree. It isn't quite as simple as !this.state.visible && nextState.visible since props could have changed including props.children aka whatever is being lazy loaded. Adding a shallow equal of the props should work since all the props are primitives except props.children, leaving it up to the developer using the package to put in a component that will only cause the update when necessary.

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

No branches or pull requests

2 participants