Test for concurrency issue with use find client#428
Conversation
Breaks Verify reference stability between rerenders
radekmie
left a comment
There was a problem hiding this comment.
A few comments:
mergeRefreshDatadoes not respect order, so it'll ignoresortchanges. That should not be ignored.mergeRefreshDatadoes only shallow comparisons, and that's not enough for more complex documents (nested objects are not that popular, but arrays are). On the other hand, doing deep comparisons will be very expensive.- The way
initialDatais now calculated triggersfetchDataon every render which is not ideal performance-wise.
|
You are right, the solution is not good after looking closely! |
|
I'm fine with adding this test as is. But we need to find a fix for this as well. |
radekmie
left a comment
There was a problem hiding this comment.
I agree it's a valuable test to merge.
|
It looks like the test here meant to highlight the issue described in #418, but it is passing green when it should have failed. I had assumed the test would fail until #419 was applied, after which it would pass to confirm the fix. Currently, this test is on master already, and passing properly. So supposedly is fixed without the need of any other PR to be merged. Does anyone know what happens and have guidance on verifying the fix using this test? I’m looking to finalize the changes for the upcoming beta release of react-packages. |
Add a test that catches the issue #418
I am skipping the test because it is failing with the current code
So this is not a fix attempt anymore, but just add the test for someone who will try to fix it