Skip to content

Core Data: Add invalidateCache option to saveEntityRecord - #81321

Open
Mamaduka wants to merge 1 commit into
trunkfrom
update/save-entity-record-cache-invalidation
Open

Mamaduka wants to merge 1 commit into
trunkfrom
update/save-entity-record-cache-invalidation

Conversation

@Mamaduka

@Mamaduka Mamaduka commented Aug 7, 2026

Copy link
Copy Markdown
Member

What?

Note: Mostly wanted to run the idea with folks.

PR adds an invalidateCache option to saveEntityRecord, with a default of true. Existing behavior unchanged. This will allow consumers to opt out of cache invalidation if needed.

Possible use case

  • Creating an entity in the editor can invalidate the cache earlier than needed. Example: Creating a tag invalidates the cache, fetches for old term IDs, then fetches again for new term IDs. Ideally, it should only do the latter. Related Editor: Avoid unnecessary term re-fetches in FlatTermSelector #80623.
  • Bulk saves. Saving N records fires N invalidation storms. Opt out on all but the last.
  • Quick Edit. Editing a field in a data view via Quick Edit invalidates the whole list; depending on the case, the consumer might want to opt out.

Testing Instructions

No behavior change by default; existing core-data action tests cover both paths.

Use of AI Tools

Assisted by Claude.

@Mamaduka Mamaduka self-assigned this Aug 7, 2026
@Mamaduka Mamaduka added the [Type] Enhancement A suggestion for improvement. label Aug 7, 2026
@Mamaduka
Mamaduka requested a review from fabiankaegy as a code owner August 7, 2026 09:48
@Mamaduka Mamaduka added the [Package] Core data /packages/core-data label Aug 7, 2026
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

Size Change: +4 B (0%)

Total Size: 7.82 MB

📦 View Changed
Filename Size Change
build/scripts/core-data/index.min.js 36.9 kB +4 B (+0.01%)

compressed-size-action

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

Flaky tests detected in d05616f.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/31167450860
📝 Reported tests:

preserves content in the Custom HTML block in /test/e2e/specs/widgets/customizing-widgets.spec.js, passed after 1 failed attempt.
Error: apiRequestContext.fetch: socket hang up
Call log:
  - → POST http://localhost:8889/wp-json/wp/v2/sidebars/sidebar-2
    - user-agent: Playwright/1.62.1 (x64; ubuntu 24.04) node/20.20 CI/1
    - accept: */*
    - accept-encoding: gzip,deflate,br
    - X-WP-Nonce: f31694edbf
    - content-type: application/json
    - content-length: 31
    - cookie: wordpress_test_cookie=WP%20Cookie%20check; wordpress_logged_in_23778236db82f19306f247e20a353a99=admin%7C1786269220%7CU1VQNXAzp4nearil9rdLXl1kqHFD1vWSJLgfxc3zJK7%7C0ec168c01479e1b69dcfcee124c58ca69f765d2f1ed8f7bedff71efb1286a254; wp-settings-time-1=1786097532

    at RequestUtils.rest (/home/runner/work/gutenberg/gutenberg/packages/e2e-test-utils-playwright/src/request-utils/rest.ts:119:39)
    at map (/home/runner/work/gutenberg/gutenberg/packages/e2e-test-utils-playwright/src/request-utils/widgets.js:22:9)
    at RequestUtils.deleteAllWidgets (/home/runner/work/gutenberg/gutenberg/packages/e2e-test-utils-playwright/src/request-utils/widgets.js:21:12)
    at /home/runner/work/gutenberg/gutenberg/test/e2e/specs/widgets/customizing-widgets.spec.js:37:3

@youknowriad

Copy link
Copy Markdown
Contributor

I would like to understand the reasoning more. from a core-data perspective, saving a tag should invalidate the cache because the current lists are going to be outdated anyway, so in that sense the current behavior is correct.

Maybe what you need here is to wrap multiple action calls in "registry.batch" and have the registry only do the cache invalidation after the batch (don't know if that behavior is built-in or not).

@Mamaduka

Mamaduka commented Aug 7, 2026

Copy link
Copy Markdown
Member Author

Thanks for the comment, Riad!

The term example isn't really a good one; I've only mentioned it because that's what led me to this exploration. At the moment, I'm not sure whether I should use that option. Depends on refactoring in #80967.

The current default behavior is correct for most cases, and that's why it remains. I think this is more about giving consumers the freedom to opt out of cache invalidation.

I need to experiment with Quick Edit in DataViews and see if there's an actual win.

@tyxla tyxla left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing to consider is that this option moves cache consistency from core-data to each consumer. That way any consumer can disable invalidation and accidentally leave lists, ordering, or pagination out of date. I guess if we choose to keep this option, we should clearly define how callers invalidate the cache later and clearly document their responsibility.

@Mamaduka

Mamaduka commented Aug 7, 2026

Copy link
Copy Markdown
Member Author

One thing to consider is that this option moves cache consistency from core-data to each consumer.

That should be expected since the consumer intentionally opts out.

We could provide an action to easily invalidate entity collections; currently, you have to loop through the getCachedResolvers value. I think it could be helpful without this change as well.

P.S. It was interesting to learn that @tanstack/react-query doesn't have automatic invalidation on mutations. Though I think core-data default makes sense for WP.

@tyxla

tyxla commented Aug 7, 2026

Copy link
Copy Markdown
Member

One thing to consider is that this option moves cache consistency from core-data to each consumer.

That should be expected since the consumer intentionally opts out.

I can see how that can be missed regardless, and that's my biggest worry. Especially considering that everyone shares this data and plugin X can harm plugin Y without realizing.

@jsnajdr

jsnajdr commented Aug 10, 2026

Copy link
Copy Markdown
Member

Creating a tag invalidates the cache, fetches for old term IDs, then fetches again for new term IDs

We should be using the stale-while-revalidate (SWR) pattern here. Creating a tag invalidates the old list, but most of the time, it's desirable that the selectors still return the old list instead of [] or null. The old list is still very good. There should be a way to detect stale results with some isStale flag.

If we don't do this SWR pattern correctly, that's almost a bug, we should fix that.

Let's use TanStack Query as an inspiration whenever possible. They have all this solved very elegantly, hard to come up with something better. When TanStack Query does a mutation, it doesn't know automatically what to invalidate, you need to do it manually:

useMutation( {
  onSuccess: () => {
    queryClient.invalidateQueries( key );
  }
} );

Our invalidateCache flag is very similar to that, just much less flexible.

@Mamaduka

Mamaduka commented Oct 9, 2026

Copy link
Copy Markdown
Member Author

A similar issue came up with #76767.

The comments lack a proper custom-type API and are currently treated as a single entity. Adding a reaction (another comment type) invalidates the entire notes list. While we ended up using this to our advantage, I think it still highlights a lack of consumer-side control in more complex cases.

What do you think we should provide alongside the invalidateCache option to make it less "foot-gun-y" for consumers?

P.S. Lack of a custom comment API is a whole different can of worms, so let's ignore that for the sake of the argument 😅

@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown

🤖 PR meta 🤖

🎉 Props

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
Co-authored-by: tyxla <tyxla@git.wordpress.org>
Co-authored-by: youknowriad <youknowriad@git.wordpress.org>
Co-authored-by: jsnajdr <jsnajdr@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Updated as activity occurs, without notifying anyone named here. Add the props-bot label to refresh.

@jsnajdr

jsnajdr commented Oct 14, 2026

Copy link
Copy Markdown
Member

What do you think we should provide alongside the invalidateCache option to make it less "foot-gun-y" for consumers?

The advantage of invalidateCache is that it always works and gives correct results. Anything else will be a custom optimization that doesn't work universally.

If there is a new comment or a minor modification of an existing comment, we could write the new entity record into the existing Redux state (RECEIVE_ITEMS), and keep all resolvers resolved. We just updated the already-resolved query with fresh results. I'm not sure if the existing core-data actions like receiveEntityRecords` fully support this scenario, but that can be expanded if needed.

The part that IMO doesn't work reliably is updating all queries correctly. When there are resolved results for a { foo: 'bar' } query, how can we know if the updated record still matches that query? Only the REST query engine on the server knows that. The JS client library doesn't. That's why we can use this approach only as an optimization on a particular screen, which knows all its relevant queries, and why it's not universal.

@Mamaduka

Copy link
Copy Markdown
Member Author

Thanks, @jsnajdr! That makes sense.

I'll continue working on this and other core-data improvements once I have time.

This branch has not been deployed

No deployments
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 [Type] Enhancement A suggestion for improvement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants