Skip to content

Conversation

@Thom1729
Copy link
Member

Before this change, the saved value of a setting wasn't updated until after the callback was completed, so if you changed that settings object inside the callback, it would incorrectly detect that the first setting had changed again.

@FichteFoll
Copy link
Member

This fixes an error case, but I still see potential for confusion when a "complex" object is constructed by the selector and modified in place, since the callback is called by reference, but that is a slightly different issue. To tackle that, we'd probably need a mutex and a second call of the selector to ensure we get two copies of the same state without relying on the data to be copyable, which is definitely not ideal.

Besides, I don't think it is currently documented that the selector's result must be equatable.

@Thom1729
Copy link
Member Author

That's a good point. I think it could be handled separately from this bug, though.

@FichteFoll FichteFoll added this to the 1.4.1 milestone Apr 1, 2021
@Thom1729 Thom1729 merged commit f173d87 into master Apr 1, 2021
@Thom1729 Thom1729 deleted the fix-settings-changes-in-subscribe-callback branch April 1, 2021 14:16
@Thom1729 Thom1729 mentioned this pull request Apr 10, 2021
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.

3 participants