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

Fix lazy loading #462

Merged
merged 3 commits into from
Sep 22, 2018
Merged

Fix lazy loading #462

merged 3 commits into from
Sep 22, 2018

Conversation

Bobgy
Copy link
Contributor

@Bobgy Bobgy commented Sep 2, 2018

I've been using this fix in production for a few weeks. It worked well and I didn't need to do any other fix after the change.

Closes #453

To be honest, I am not familiar with enzyme, especially how lifecycle and first render/second render works in it.
I am confused by how the test 'prop: disableLazyLoading' was written. Does it mean mounting the same component twice in consecutive tests will count as first render and then second render?

I'd really like some suggestions on that, I tried some fixes just to pass the test.

@Bobgy
Copy link
Contributor Author

Bobgy commented Sep 2, 2018

There seems to be a network error when installing dependencies on circle ci. Can someone with write access retry it?

@oliviertassinari oliviertassinari self-assigned this Sep 22, 2018
@oliviertassinari oliviertassinari changed the title lazy loading by setTimeout Fix lazy loading Sep 22, 2018
@oliviertassinari oliviertassinari merged commit 248fec1 into oliviertassinari:master Sep 22, 2018
@oliviertassinari
Copy link
Owner

@Bobgy It's a great first pull request 👌🏻. Thank you for working on it!

@Bobgy
Copy link
Contributor Author

Bobgy commented Sep 23, 2018

@oliviertassinari I checked your new changes. They really made this more readable. Awesome work!
Thank you for the follow-up fix.

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.

lazy loading is useless right now
2 participants