Core Data: Validate the parsed-blocks cache against registered block types

packages/core-data/CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+### Bug Fixes
+
+-   Validate the shared parsed-blocks cache against the registered block types as well as the content, so a record resolved before the block types register — as happens when the editor's assets load lazily — is re-parsed instead of rendering, and one save later persisting, an empty block list ([#81809](https://github.com/WordPress/gutenberg/pull/81809)).
+
 ### Internal
 
 -   Split tsconfig into a build project and a default dev project so dev files are type checked without publishing their declarations. ([#81514](https://github.com/WordPress/gutenberg/pull/81514))

packages/core-data/src/hooks/use-entity-block-editor.js

@@ -4,7 +4,7 @@ import { parse, __unstableSerializeAndClean } from '@wordpress/blocks';
 import { STORE_NAME } from '../name';
 import useEntityId from './use-entity-id';
 import { updateFootnotesFromMeta } from '../footnotes';
-import { parsedBlocksCache, getCacheKey } from '../parsed-blocks-cache';
+import { getCachedBlocks, setCachedBlocks } from '../parsed-blocks-cache';
 
 const EMPTY_ARRAY = [];
 
@@ -60,17 +60,15 @@ export default function useEntityBlockEditor( kind, name, { id: _id } = {} ) {
 			return EMPTY_ARRAY;
 		}
 
-		// Cache parsed blocks by entity identity. Store the content
-		// alongside the blocks so we can validate it hasn't changed.
-		const cacheKey = getCacheKey( kind, name, id );
-		const cached = parsedBlocksCache.get( cacheKey );
-		let _blocks;
+		// The cache validates the content and the registered block types, so
+		// an entry parsed from other content — or before the block types were
+		// registered, as happens when a record resolves while the editor's
+		// assets are still loading — is re-parsed instead of adopted.
+		let _blocks = getCachedBlocks( kind, name, id, content );
 
-		if ( cached && cached.content === content ) {
-			_blocks = cached.blocks;
-		} else {
+		if ( ! _blocks ) {
 			_blocks = parse( content );
-			parsedBlocksCache.set( cacheKey, { content, blocks: _blocks } );
+			setCachedBlocks( kind, name, id, content, _blocks );
 		}
 
 		return _blocks;

packages/core-data/src/parsed-blocks-cache.js

@@ -1,12 +1,85 @@
+import { select } from '@wordpress/data';
+import { store as blocksStore } from '@wordpress/blocks';
+
 /**
- * Shared cache of blocks parsed from an entity's `content` string, keyed by
- * `kind:name:id`. Populated both eagerly by the `getEntityRecord` resolver
- * (when the sync manager parses content for transient edits) and lazily by
- * `useEntityBlockEditor`. The stored `content` string acts as a validator so
- * stale entries are discarded when the underlying record changes.
+ * Shared cache of blocks parsed from an entity's `content` string. Populated
+ * both eagerly by the `getEntityRecord` resolver (when the sync manager
+ * parses content for transient edits) and lazily by `useEntityBlockEditor`.
+ *
+ * A parse result is a function of the content and of the block types
+ * registered at the time: a block whose type is not registered yet is
+ * dropped or replaced, not kept. The cache is therefore two levels — the
+ * outer one keyed by the registered block types list, the inner one by
+ * `kind:name:id`, holding the blocks with the `content` string they were
+ * parsed from as the remaining validator. A record resolved before the
+ * editor's assets have registered the block types — a resolver can run
+ * that early — lands under a list that registration then replaces.
+ *
+ * The outer level is a WeakMap keyed by the list itself: the blocks store
+ * memoizes it until a type is added or removed, then produces a new one and
+ * drops the old, taking every entry parsed against it with it. Nothing is
+ * invalidated by hand and nothing stale is retained.
  */
-export const parsedBlocksCache = new Map();
+const caches = new WeakMap();
 
-export function getCacheKey( kind, name, id ) {
+function getCacheKey( kind, name, id ) {
 	return `${ kind }:${ name }:${ id }`;
 }
+
+/**
+ * Returns the registered block types, from the registry `parse` reads.
+ *
+ * Block types are registered on the default registry, so the cache is
+ * partitioned by that registry's list rather than one from the registry
+ * hosting the entities.
+ *
+ * @return {Array} Registered block types.
+ */
+function getBlockTypes() {
+	return select( blocksStore ).getBlockTypes();
+}
+
+/**
+ * Returns the blocks cached for an entity, if they were parsed from the
+ * given content with the block types registered right now.
+ *
+ * @param {string}        kind    Entity kind.
+ * @param {string}        name    Entity name.
+ * @param {string|number} id      Record ID.
+ * @param {string}        content The record's current `content` string.
+ *
+ * @return {Array|undefined} The cached blocks, or undefined without a valid
+ *                           entry.
+ */
+export function getCachedBlocks( kind, name, id, content ) {
+	const cached = caches
+		.get( getBlockTypes() )
+		?.get( getCacheKey( kind, name, id ) );
+
+	if ( cached && cached.content === content ) {
+		return cached.blocks;
+	}
+}
+
+/**
+ * Caches the blocks parsed for an entity, under the block types they were
+ * parsed with.
+ *
+ * @param {string}        kind    Entity kind.
+ * @param {string}        name    Entity name.
+ * @param {string|number} id      Record ID.
+ * @param {string}        content The `content` string the blocks were parsed
+ *                                from.
+ * @param {Array}         blocks  The parsed blocks.
+ */
+export function setCachedBlocks( kind, name, id, content, blocks ) {
+	const blockTypes = getBlockTypes();
+	let cache = caches.get( blockTypes );
+
+	if ( ! cache ) {
+		cache = new Map();
+		caches.set( blockTypes, cache );
+	}
+
+	cache.set( getCacheKey( kind, name, id ), { content, blocks } );
+}

packages/core-data/src/resolvers.js

@@ -18,7 +18,7 @@ import {
 } from './utils';
 import { fetchBlockPatterns } from './fetch';
 import { restoreSelection, getSelectionHistory } from './utils/crdt-selection';
-import { parsedBlocksCache, getCacheKey } from './parsed-blocks-cache';
+import { setCachedBlocks } from './parsed-blocks-cache';
 
 /**
  * Requests authors from the REST API.
@@ -172,15 +172,20 @@ export const getEntityRecord =
 					} );
 
 				// Share the parsed blocks with `useEntityBlockEditor` so the
-				// editor doesn't re-parse the same `content` string.
+				// editor doesn't re-parse the same `content` string. The cache
+				// remembers which block types the parse ran against, so an
+				// entry computed before they register is discarded, not served.
 				if (
 					recordWithTransients.blocks &&
 					typeof recordWithTransients.content?.raw === 'string'
 				) {
-					parsedBlocksCache.set( getCacheKey( kind, name, key ), {
-						content: recordWithTransients.content.raw,
-						blocks: recordWithTransients.blocks,
-					} );
+					setCachedBlocks(
+						kind,
+						name,
+						key,
+						recordWithTransients.content.raw,
+						recordWithTransients.blocks
+					);
 				}
 
 				const syncManager =
@@ -189,6 +194,11 @@ export const getEntityRecord =
 						: getSyncManager();
 
 				// Load the entity record for syncing. Do not await promise.
+				// NOTE: when this resolver runs before block types register,
+				// `recordWithTransients.blocks` was parsed as empty. The cache
+				// above discards such an entry; the sync manager receives it
+				// as-is, and seeding a collaborative document from it is an
+				// open problem of the collaboration path.
 				void syncManager?.load(
 					entityConfig.syncConfig,
 					objectType,

packages/core-data/src/test/parsed-blocks-cache.js

@@ -0,0 +1,43 @@
+import { registerBlockType, unregisterBlockType } from '@wordpress/blocks';
+import { getCachedBlocks, setCachedBlocks } from '../parsed-blocks-cache';
+
+describe( 'parsed blocks cache', () => {
+	const content = '<!-- wp:test/block /-->';
+	const blocks = [];
+
+	it( 'returns an entry parsed from the same content with the same block types', () => {
+		setCachedBlocks( 'postType', 'page', 1, content, blocks );
+
+		expect( getCachedBlocks( 'postType', 'page', 1, content ) ).toBe(
+			blocks
+		);
+	} );
+
+	it( 'discards an entry when the content changed', () => {
+		setCachedBlocks( 'postType', 'page', 2, content, blocks );
+
+		expect(
+			getCachedBlocks( 'postType', 'page', 2, '<!-- wp:test/other /-->' )
+		).toBeUndefined();
+	} );
+
+	it( 'discards an entry once block types register, so blocks parsed too early are not served', () => {
+		// Parsed and cached while `test/block` was not registered — as happens
+		// when a record resolves before the editor's assets have loaded.
+		setCachedBlocks( 'postType', 'page', 3, content, blocks );
+
+		registerBlockType( 'test/block', {
+			apiVersion: 3,
+			title: 'Test Block',
+			category: 'text',
+		} );
+
+		try {
+			expect(
+				getCachedBlocks( 'postType', 'page', 3, content )
+			).toBeUndefined();
+		} finally {
+			unregisterBlockType( 'test/block' );
+		}
+	} );
+} );