Core Data: Use WeakMap keyed by content object for parsed blocks cache#78254
Open
ellatrix wants to merge 1 commit into
Open
Core Data: Use WeakMap keyed by content object for parsed blocks cache#78254ellatrix wants to merge 1 commit into
ellatrix wants to merge 1 commit into
Conversation
Switches the parsed blocks cache from a string-keyed `Map` with a content validator back to a `WeakMap` keyed by the record's `content` object. The queried-data reducer preserves the content reference across receives when content is unchanged (via `conservativeMapItem`), and replaces it when content changes — so the WeakMap evicts naturally and survives unrelated record updates. The hook only writes to the cache in the unedited path (where `content === contentObject.raw`), so edited-content parses can't pollute the entry that later unedited renders depend on. The resolver writes after the receive batch so the stored content reference is the one used as the WeakMap key. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
This was referenced May 13, 2026
|
Flaky tests detected in 477430a. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/25822814545
|
|
Size Change: +13 B (0%) Total Size: 7.95 MB 📦 View Changed
ℹ️ View Unchanged
|
Contributor
|
My suspicion was that this would break "selected block restoration" after navigating between entities. Now to my surprise, I discovered that it's broken in trunk already as well, so I can't tell whether this PR will be a compounding effect on that breakage or not. Basically the idea is
This seems to be broken right now in trunk though, so I can't tell if this PR makes it worse. |
Contributor
|
Conceptually though, I don't think this should be a weakmap, I think any entity that gets loaded into core-data should have its blocks parsed and kept there, instead of reparsing them every time we need them. Now I do realize using a Map is a hacky solution to that. The problem is that implementing that "parse on fetch" in core-data is a bit hard. I tried it a long time ago by defining and get/set on the transient properties of entities but never managed to do the refactor fully. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What?
Follow-up to #78026. Switches the parsed blocks cache from a string-keyed
Map+ content validator to aWeakMapkeyed by the record'scontentobject.Why?
conservativeMapItem) preserves thecontentobject reference when content is unchanged and replaces it when content changes. So the WeakMap key changes iff content changes — no validator needed, no stale entries.content(not the whole record) means title/status/meta updates don't invalidate the cache.content === contentObject.raw), so an edited-content render can't overwrite the resolver-populated entry. Addresses jsnajdr's concern on #78026.How?
parsedBlocksCacheis now aWeakMap.rawRecord.contentviagetEntityRecordand uses it as the key. Edited-content renders parse uncached.registry.batch, usingselect.getEntityRecord(...).contentso the key matches what the hook will later see.Testing Instructions
Testing Instructions for Keyboard
No UI changes.
Use of AI Tools
Authored with Claude Code (Opus 4.7). I reviewed the analysis, the keying choice, and every line of the diff before committing.
🤖 Generated with Claude Code