-
Notifications
You must be signed in to change notification settings - Fork 743
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
Update dependent keys after a successful load #1375
Conversation
See #1156 (comment) for details
yes, this fixes the issue, I will think about the test today. Thank you |
@designatednerd here is a test that verifies it's working. I wouldn't add it as it is, because it's messy (In order to make this work I've created a new query with the same structure as If I have the time in the next week or so I will try to come up with something better because the issue we are fixing is a bit hidden and it's very easy to miss.
|
…ge from the cache and once for the change from a related query.
@teodorpenkov Thanks for your help with the idea - I've taken the test and fleshed it out a bit, would love your feedback, but it passes when resetting the dependent keys is there and fails when it's commented out, so I think it should be an effective safety net. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@teodorpenkov thanks for the inspo on the test, it already caught a massive bug with query watchers on the networking rewrite branch 🙃 |
Addresses #1365 - @teodorpenkov, can you confirm this fixes your issue? I'm still futzing around on how to test this.