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

[@wordpress/core-data] getEntityRecords and saveEntityRecord don't sync data correctly #22127

Closed
samueljseay opened this issue May 6, 2020 · 6 comments
Labels
[Package] Core data /packages/core-data

Comments

@samueljseay
Copy link
Contributor

Describe the bug
When using getEntityRecords and saveEntityRecord within a single component, I expect that when saveEntityRecord is called, we will not see another fetch of the data (getEntityRecords) until the saveEntityRecord data is finished saving.

What I see instead is that on call of saveEntityRecord the cache is invalidated too soon and getEntityRecords fetches stale data. This causes flickers and the data on screen to be out of date. I have tested this behaviour in withSelect and also useSelect and it behaves the same way in both.

When debugging I found it hard to always get consistent behaviour which makes me think a race condition exists, but quite frequently I do see that core-data's saveEntityRecord triggers 2 RECEIVE_ITEMS actions here, the first of which invalidates the cache conditions for getEntityRecords defined here.

I'm not sure if this is a red herring at this stage, but I couldn't find another reason for the immediate fetch behaviour.

Additionally this issue seems similar to others I've seen such as #19132 but this is in the context of core-data so I felt like it should be a unique issue.

To reproduce
I have produced a minimal reproduction in a dummy plugin in a branch here. You should be able to run it as you would normally run gutenberg in development, go to the core data issue plugin and see a list of items, as you check the items they will either flicker back and forth from their checked state or start to get out of sync very quickly.

The code responsible for the fetch/save functionality is here

Expected behavior
The expected behaviour would be that the checkbox would finish saving completely before the new collection is fetched. This would allow the fetched data to be completely up to date.

Screenshots
reproduce-issue

Editor version (please complete the following information):

  • WordPress version: 5.5-alpha-47766 (in development mode) but I've seen it in 5.4 as well.
  • "gutenberg plugin"

Desktop (please complete the following information):

  • OS: MacOS
  • Browser Chrome
  • Version 81.0.4044.129
@talldan talldan added [Package] Core data /packages/core-data Needs Testing Needs further testing to be confirmed. labels May 8, 2020
@chriskmnds
Copy link
Contributor

This seems related: #19752 Similar issue with flickering when updating.

@noisysocks noisysocks removed the Needs Testing Needs further testing to be confirmed. label Oct 13, 2020
@noisysocks
Copy link
Member

It looks like @adamziel ran into this here, so I'm removing Needs Testing: #25859 (comment)

@adamziel
Copy link
Contributor

This bites us a lot in the widgets editor, let's take a stab at it as soon as possible cc @noisysocks @draganescu @TimothyBJacobs @youknowriad.

@adamziel
Copy link
Contributor

adamziel commented Nov 3, 2020

#26627 and #26575 improved the situation here. There is much less flickering, although there is still some of it as seen below:

core data issue

I think locks would be the ultimate solution here. The downside is that they would add a layer of complexity to an already complex core-data so I'm still trying to come up with some alternatives. But for reference, see the same interaction with #26389 applied:

2020-11-03 12-41-39 2020-11-03 12_41_59

cc @youknowriad @samueljseay

@adamziel
Copy link
Contributor

adamziel commented Nov 3, 2020

The flicker on my first gif above is caused by:

  1. I save an entity record book with ID = 11 , no resolution is triggered, we’re good, PUT#1 request is waiting for response.
  2. I save an entity record book with ID = 10 , no resolution is triggered, we’re good, PUT#2 request is waiting for response.
  3. The PUT#1 finishes, invalidates the entity. I have a useSelect call that calls getEntityRecords() . A GET request is issued and awaits response.
  4. Now comes the tricky part. Once the GET is finished, it will issue RECEIVE_ITEMS and update the list of entities, including the ones handled by PUT#2.

Different things could happen depending on the resolution order too.

@adamziel
Copy link
Contributor

Since #26389 is now merged, I'm closing this one. Feel free to comment or reopen if the fix did not fully help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Core data /packages/core-data
Projects
None yet
Development

No branches or pull requests

5 participants