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

Bailout if the element and the context is unchanged properly #7819

Closed
wants to merge 2 commits into from

Conversation

wuct
Copy link
Contributor

@wuct wuct commented Sep 28, 2016

Currently, We use === to check if a context is changed before bailing out, but we create a new context everytime a compnent is updated, regardless the origin context provided by the consumer is unchanged.

As a result, === would always return false unless the context is undefined. That is, when there is a context, a component would never be bailed out. Specifically, If we use react-redux, react-router, react-intl or any other lib which provides a context, we would get no bailout optimization.

To fix this, I suggest we use shallowEqual instead of ===. But I am not sure it would introduce a breaking change or not. I have made this change in this PR along with a test.

@wuct
Copy link
Contributor Author

wuct commented Sep 28, 2016

I also create a jsfiddle to compare the performance with and without bailing out (with a context and without a context) in production. It seems like bailing out decrease around 30% of updating time on my laptop.

Please open the console to see the result.

@nkt
Copy link

nkt commented Sep 28, 2016

@wuct this is breaking change, you can define shouldComponentUpdate which has third argument nextContext.

@wuct
Copy link
Contributor Author

wuct commented Sep 29, 2016

@nkt Thanks for replying. I know I can always use shouldComponentUpdate, but which is not I am looking for.
However, I found there is already a discussion in #4344. I apologize for not finding it earlier. I am going to close this PR.

@wuct wuct closed this Sep 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants