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

Strange behaviour with alpha version #6348

Closed
kafein opened this issue Sep 23, 2018 · 7 comments
Closed

Strange behaviour with alpha version #6348

kafein opened this issue Sep 23, 2018 · 7 comments

Comments

@kafein
Copy link
Contributor

kafein commented Sep 23, 2018

Version

4.4.0-alpha.1

Test Case

https://codesandbox.io/s/qx464vwxlj

Steps to reproduce

Click the links, check console output.

Expected Behavior

Layout component has explicit return false statement, so TestComponent shouldn't re-render when route changes.

Same example with Reach Router works well. https://codesandbox.io/s/6zv4m0wxp3

Actual Behavior

Although layout component works as expected, child component re-renders in route change. SCU doesn't prevent child re-render. (check console output after clicking links)

@timdorr
Copy link
Member

timdorr commented Sep 23, 2018

This is already changed on master.

@timdorr timdorr closed this as completed Sep 23, 2018
@kafein
Copy link
Contributor Author

kafein commented Sep 23, 2018

Hi @timdorr. Just wondering, what causes this behaviour?

@timdorr
Copy link
Member

timdorr commented Sep 23, 2018

setState in the Router at the top level. Causes a render on navigation. That's gone now.

@kafein
Copy link
Contributor Author

kafein commented Sep 23, 2018

Got it. Thank you so much.

@kafein
Copy link
Contributor Author

kafein commented Sep 30, 2018

@timdorr @mjackson

Test sandbox updated with latest beta version. Still same behaviour. Nothing changed.

@mjackson
Copy link
Member

Thanks for bringing this to my attention, @kafein. I'm looking into it. I think you may be right that using the old context API is causing that re-render. I'm going to keep looking into this.

@mjackson
Copy link
Member

Hi @kafein, I just confirmed that beta.3 doesn't have this issue. You can see the codesandbox here: https://codesandbox.io/s/3265w17kjq

Thanks again for bringing it to our attention :)

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

3 participants