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

early break in AutoObservable s.hasChanged check can trigger multiple recalculations #78

Open
nadako opened this issue Nov 21, 2022 · 2 comments

Comments

@nadako
Copy link
Contributor

nadako commented Nov 21, 2022

I've found an interesting thing while porting your excellent library to C#. When AutoObservable checks if any of the subscriptions has actually changed it breaks earlier and skips checking other dependencies, which sounds logical... However hasChanged in the current revision also updates the revision and value for a Subscription, and if we only do that for the first changed dependency and skip all other, the subsValid will return false and we'll need a second loop iteration.

At first I was thinking that the correct fix would be to simply remove break here, however after some investigation, I think it's even better to update Subscription.lastRev on reuse. What do you think?

@back2dos
Copy link
Member

Ok. I think I sorta understand what you're saying. But you wouldn't happen to have a failing test for the current behavior? ^^

@nadako
Copy link
Contributor Author

nadako commented Apr 26, 2023

If I remember correctly it doesn't exactly fail in a sense of broken state or invalid values, but it does unnecessary loops.

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

No branches or pull requests

2 participants