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 binding out of sync on reactive update #3886

Merged
merged 2 commits into from
Nov 10, 2019

Conversation

AlbertLucianto
Copy link
Contributor

Should fix #3382

The bug is caused by updating_count in App being set to true on bound after RefactoredButton click . When count is updated reactively in App, updating_count is still true (because it has not been flushed yet), hence setting the final value of count back to RefactoredButton is skipped.

This solution is to move setting updating_count (and flush callback) from binding to p. The reason is, I believe, updating in binding does not entirely represent updating of a component i.e. the way parent updates and passes the props to its children is only through p and it should also be considered updating.

I hope my understanding is correct and this change does not create any unexpected behaviours. I have also added a new test case for this.

Thanks and appreciate for your review!

PS: Sorry, I closed the previous PR when I thought it didn't pass the CI 😓, but I think it's unrelated from my changes 🤔.

@Rich-Harris
Copy link
Member

Thank you — yep, the CI failures are unrelated, I fixed them by merging master into this branch.

I'll admit my brain gets a little tied in knots when trying to think about data flow where component bindings are concerned, but your explanation makes sense, and there are loads of tests around this stuff already so I'd be very surprised if there are unexpected behaviours from this.

@Rich-Harris Rich-Harris merged commit 9500282 into sveltejs:master Nov 10, 2019
@AlbertLucianto AlbertLucianto deleted the fix/bind-out-of-sync branch November 11, 2019 06:36
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.

Prop binding goes out of sync when reactively updated
2 participants