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 too early unmount #425

Closed
wants to merge 1 commit into from
Closed

fix too early unmount #425

wants to merge 1 commit into from

Conversation

szyablitsky
Copy link
Contributor

@szyablitsky szyablitsky commented May 18, 2016

This change is Reviewable

@justin808
Copy link
Member

@szyablitsky This is super awesome! Would you like to create a failing without, passing with test in the spec/dummy app? If not, I can try. I will merge your commit into a branch on my machine and then I'll push up a new PR. If you can write a test, that would be AMAZING! Thanks so much!

@szyablitsky
Copy link
Contributor Author

szyablitsky commented May 19, 2016

@justin808 testing this can be very tricky because of Capybara's waiting behavior. https://github.com/jnicklas/capybara#asynchronous-javascript-ajax-and-friends
Also rerendering actually happens every time. But depending on cache state, time between turbolinks:before-cache and turbolinks:before-render can be zero (in which case rerendering is not visible) or it can be big enough to see the empty space in place of unmounted component.

@justin808
Copy link
Member

@szyablitsky At the least, we can put in an example that confirms this manually, in spec/dummy

I'm also going to push an RC with this fix and test it in my branch for the https://github.com/shakacode/react-webpack-rails-tutorial/.

@justin808
Copy link
Member

I merged this change here:

40553fc

We should still have an example of this in spec/dummy.

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

Successfully merging this pull request may close these issues.

2 participants