Blocks: Deprecate isValidBlockContent (#38794)

The `isValidBlockContent` function attempts to validate a block by
re-generating its output but does so by ignoring any `innerBlocks`
that might exist. Since blocks can change their output based on the
presence or content of its inner blocks this will lead to incorrect
results from the validator.

`validateBlock` also makes this mistake however it is fixable since
it requires passing in the block itself (fixable in another patch
once there are no longer two interfaces for what appears to be the
same purpose). `isValidBlockContent` doesn't pass in the block and
so is fundamentally limited in being able to validate properly.
This limitation also means that we can't rely on `isValidBlockContent`
once we start threading source information into blocks; that is, as
we improve our ability to catch and fix or preserve issues with the
block markup, this function provides no way forward to incorporate
those updates.

After scouring through the commit logs I was unable to find a clear
purpose for this second validation method and in Core it only seems
to be used as a convenience function for tests. We're deprecating it
in this patch because it's an exposed public interface and it's not
clear if anyone relies on it.

docs/contributors/code/deprecations.md

@@ -2,6 +2,10 @@
 
 For features included in the Gutenberg plugin, the deprecation policy is intended to support backward compatibility for two minor plugin releases, when possible. Features and code included in a stable release of WordPress are not included in this deprecation timeline, and are instead subject to the [versioning policies of the WordPress project](https://make.wordpress.org/core/handbook/about/release-cycle/version-numbering/). The current deprecations are listed below and are grouped by _the version at which they will be removed completely_. If your plugin depends on these behaviors, you must update to the recommended alternative before the noted version.
 
+## Unreleased
+
+-   `wp.blocks.isValidBlockContent` has been removed. Please use `wp.blocks.validateBlock` instead.
+
 ## 11.0.0
 
 -   `wp.blocks.registerBlockTypeFromMetadata` method has been removed. Use `wp.blocks.registerBlockType` method instead.

packages/block-editor/src/components/block-list/block-html.js

@@ -12,8 +12,8 @@ import {
 	getBlockAttributes,
 	getBlockContent,
 	getBlockType,
-	isValidBlockContent,
 	getSaveContent,
+	validateBlock,
 } from '@wordpress/blocks';
 
 /**
@@ -43,9 +43,13 @@ function BlockHTML( { clientId } ) {
 
 		// If html is empty  we reset the block to the default HTML and mark it as valid to avoid triggering an error
 		const content = html ? html : getSaveContent( blockType, attributes );
-		const isValid = html
-			? isValidBlockContent( blockType, attributes, content )
-			: true;
+		const [ isValid ] = html
+			? validateBlock( {
+					...block,
+					attributes,
+					originalContent: content,
+			  } )
+			: [ true ];
 
 		updateBlock( clientId, {
 			attributes,

packages/blocks/README.md

@@ -566,6 +566,8 @@ _Returns_
 
 ### isValidBlockContent
 
+> **Deprecated** Use validateBlock instead to avoid data loss.
+
 Returns true if the parsed block is valid given the input content. A block
 is considered valid if, when serialized with assumed attributes, the content
 matches the original value.
@@ -881,6 +883,22 @@ _Parameters_
 -   _slug_ `string`: Block category slug.
 -   _category_ `WPBlockCategory`: Object containing the category properties that should be updated.
 
+### validateBlock
+
+Returns an object with `isValid` property set to `true` if the parsed block
+is valid given the input content. A block is considered valid if, when serialized
+with assumed attributes, the content matches the original value. If block is
+invalid, this function returns all validations issues as well.
+
+_Parameters_
+
+-   _block_ `WPBlock`: block object.
+-   _blockTypeOrName_ `[WPBlockType|string]`: Block type or name, inferred from block if not given.
+
+_Returns_
+
+-   `[boolean,Array<LoggerItem>]`: validation results.
+
 ### withBlockContentContext
 
 A Higher Order Component used to inject BlockContent using context to the

packages/blocks/src/api/index.js

@@ -89,7 +89,7 @@ export {
 // they will be run for all valid and invalid blocks alike. However, once a
 // block is detected as invalid -- failing the three first steps -- it is
 // adequate to spend more time determining validity before throwing a conflict.
-export { isValidBlockContent } from './validation';
+export { isValidBlockContent, validateBlock } from './validation';
 export { getCategories, setCategories, updateCategory } from './categories';
 
 // Blocks are inherently indifferent about where the data they operate with ends

packages/blocks/src/api/test/validation.js

@@ -16,15 +16,14 @@ import {
 	isEqualTokensOfType,
 	getNextNonWhitespaceToken,
 	isEquivalentHTML,
-	isValidBlockContent,
+	validateBlock,
 	isClosedByToken,
 } from '../validation';
 import { createLogger } from '../validation/logger';
 import {
 	registerBlockType,
 	unregisterBlockType,
 	getBlockTypes,
-	getBlockType,
 } from '../registration';
 
 describe( 'validation', () => {
@@ -717,15 +716,17 @@ describe( 'validation', () => {
 		} );
 	} );
 
-	describe( 'isValidBlockContent()', () => {
+	describe( 'validateBlock()', () => {
 		it( 'returns false if block is not valid', () => {
 			registerBlockType( 'core/test-block', defaultBlockSettings );
 
-			const isValid = isValidBlockContent(
-				'core/test-block',
-				{ fruit: 'Bananas' },
-				'Apples'
-			);
+			const [ isValid ] = validateBlock( {
+				name: 'core/test-block',
+				attrs: {
+					fruit: 'Bananas',
+				},
+				originalContent: 'Apples',
+			} );
 
 			expect( isValid ).toBe( false );
 		} );
@@ -738,35 +739,25 @@ describe( 'validation', () => {
 				},
 			} );
 
-			const isValid = isValidBlockContent(
-				'core/test-block',
-				{ fruit: 'Bananas' },
-				'Bananas'
-			);
+			const [ isValid ] = validateBlock( {
+				name: 'core/test-block',
+				attrs: {
+					fruit: 'Bananas',
+				},
+				originalContent: 'Bananas',
+			} );
 
 			expect( isValid ).toBe( false );
 		} );
 
 		it( 'returns true is block is valid', () => {
 			registerBlockType( 'core/test-block', defaultBlockSettings );
 
-			const isValid = isValidBlockContent(
-				'core/test-block',
-				{ fruit: 'Bananas' },
-				'Bananas'
-			);
-
-			expect( isValid ).toBe( true );
-		} );
-
-		it( 'works also when block type object is passed as object', () => {
-			registerBlockType( 'core/test-block', defaultBlockSettings );
-
-			const isValid = isValidBlockContent(
-				getBlockType( 'core/test-block' ),
-				{ fruit: 'Bananas' },
-				'Bananas'
-			);
+			const [ isValid ] = validateBlock( {
+				name: 'core/test-block',
+				attributes: { fruit: 'Bananas' },
+				originalContent: 'Bananas',
+			} );
 
 			expect( isValid ).toBe( true );
 		} );

packages/blocks/src/api/validation/index.js

@@ -7,6 +7,7 @@ import { identity, xor, fromPairs, isEqual, includes, stubTrue } from 'lodash';
 /**
  * WordPress dependencies
  */
+import deprecated from '@wordpress/deprecated';
 import { decodeEntities } from '@wordpress/html-entities';
 
 /**
@@ -20,6 +21,10 @@ import {
 } from '../registration';
 import { normalizeBlockType } from '../utils';
 
+/** @typedef {import('../parser').WPBlock} WPBlock */
+/** @typedef {import('../registration').WPBlockType} WPBlockType */
+/** @typedef {import('./logger').LoggerItem} LoggerItem */
+
 /**
  * Globally matches any consecutive whitespace
  *
@@ -700,19 +705,19 @@ export function isEquivalentHTML( actual, expected, logger = createLogger() ) {
  * with assumed attributes, the content matches the original value. If block is
  * invalid, this function returns all validations issues as well.
  *
- * @param {import('../parser').WPBlock}           block           block object.
- * @param {import('../registration').WPBlockType} blockTypeOrName Block type or name.
+ * @param {WPBlock}            block                          block object.
+ * @param {WPBlockType|string} [blockTypeOrName = block.name] Block type or name, inferred from block if not given.
  *
- * @return {[boolean,Object]} validation results.
+ * @return {[boolean,Array<LoggerItem>]} validation results.
  */
-export function validateBlock( block, blockTypeOrName ) {
+export function validateBlock( block, blockTypeOrName = block.name ) {
 	const isFallbackBlock =
 		block.name === getFreeformContentHandlerName() ||
 		block.name === getUnregisteredTypeHandlerName();
 
 	// Shortcut to avoid costly validation.
 	if ( isFallbackBlock ) {
-		return [ true ];
+		return [ true, [] ];
 	}
 
 	const logger = createQueuedLogger();
@@ -755,6 +760,8 @@ export function validateBlock( block, blockTypeOrName ) {
  *
  * Logs to console in development environments when invalid.
  *
+ * @deprecated Use validateBlock instead to avoid data loss.
+ *
  * @param {string|Object} blockTypeOrName      Block type.
  * @param {Object}        attributes           Parsed block attributes.
  * @param {string}        originalBlockContent Original block content.
@@ -766,6 +773,12 @@ export function isValidBlockContent(
 	attributes,
 	originalBlockContent
 ) {
+	deprecated( 'isValidBlockContent introduces opportunity for data loss', {
+		since: '12.6',
+		plugin: 'Gutenberg',
+		alternative: 'validateBlock',
+	} );
+
 	const blockType = normalizeBlockType( blockTypeOrName );
 	const block = {
 		name: blockType.name,

packages/blocks/src/api/validation/logger.js

@@ -1,3 +1,9 @@
+/**
+ * @typedef LoggerItem
+ * @property {Function}   log  Which logger recorded the message
+ * @property {Array<any>} args White arguments were supplied to the logger
+ */
+
 export function createLogger() {
 	/**
 	 * Creates a log handler with block validation prefix.
@@ -36,7 +42,7 @@ export function createQueuedLogger() {
 	/**
 	 * The list of enqueued log actions to print.
 	 *
-	 * @type {Array}
+	 * @type {Array<LoggerItem>}
 	 */
 	const queue = [];
 

test/integration/is-valid-block.test.js

@@ -1,65 +1,89 @@
 /**
  * WordPress dependencies
  */
-import { isValidBlockContent } from '@wordpress/blocks';
 import { createElement } from '@wordpress/element';
+import {
+	getBlockTypes,
+	registerBlockType,
+	unregisterBlockType,
+	validateBlock,
+} from '@wordpress/blocks';
 
-describe( 'isValidBlockContent', () => {
+describe( 'validateBlock', () => {
 	beforeAll( () => {
 		// Load all hooks that modify blocks.
 		require( '../../packages/editor/src/hooks' );
 	} );
 
+	afterEach( () => {
+		getBlockTypes().forEach( ( block ) => {
+			unregisterBlockType( block.name );
+		} );
+	} );
+
 	it( 'should use the namespace in the classname for non-core blocks', () => {
-		const valid = isValidBlockContent(
-			{
-				save: ( { attributes } ) =>
-					createElement( 'div', null, attributes.fruit ),
-				name: 'myplugin/fruit',
+		registerBlockType( 'myplugin/fruit', {
+			save: ( { attributes } ) =>
+				createElement( 'div', null, attributes.fruit ),
+			name: 'myplugin/fruit',
+			category: 'text',
+			title: 'Fruit block',
+		} );
+
+		const [ valid ] = validateBlock( {
+			name: 'myplugin/fruit',
+			attributes: {
+				fruit: 'Bananas',
 			},
-			{ fruit: 'Bananas' },
-			'<div class="wp-block-myplugin-fruit">Bananas</div>'
-		);
+			originalContent:
+				'<div class="wp-block-myplugin-fruit">Bananas</div>',
+		} );
 
 		expect( valid ).toBe( true );
 	} );
 
 	it( 'should include additional classes in block attributes', () => {
-		const valid = isValidBlockContent(
-			{
-				save: ( { attributes } ) =>
-					createElement(
-						'div',
-						{
-							className: 'fruit',
-						},
-						attributes.fruit
-					),
-				name: 'myplugin/fruit',
-			},
-			{
-				fruit: 'Bananas',
-				className: 'fresh',
-			},
-			'<div class="wp-block-myplugin-fruit fruit fresh">Bananas</div>'
-		);
+		registerBlockType( 'muplugin/fruit', {
+			save: ( { attributes } ) =>
+				createElement(
+					'div',
+					{
+						className: 'fruit',
+					},
+					attributes.fruit
+				),
+			name: 'myplugin/fruit',
+			category: 'text',
+			title: 'Fruit block',
+		} );
+
+		const [ valid ] = validateBlock( {
+			name: 'myplugin/fruit',
+			attributes: { fruit: 'Bananas', className: 'fresh' },
+			originalContent:
+				'<div class="wp-block-myplugin-fruit fruit fresh">Bananas</div>',
+		} );
 
 		expect( valid ).toBe( true );
 	} );
 
 	it( 'should not add a className if falsy', () => {
-		const valid = isValidBlockContent(
-			{
-				save: ( { attributes } ) =>
-					createElement( 'div', null, attributes.fruit ),
-				name: 'myplugin/fruit',
-				supports: {
-					className: false,
-				},
+		registerBlockType( 'myplugin/fruit', {
+			save: ( { attributes } ) =>
+				createElement( 'div', null, attributes.fruit ),
+			name: 'myplugin/fruit',
+			category: 'text',
+			title: 'Fruit block',
+			supports: {
+				className: false,
 			},
-			{ fruit: 'Bananas' },
-			'<div>Bananas</div>'
-		);
+		} );
+
+		const [ valid ] = validateBlock( {
+			name: 'myplugin/fruit',
+			attributes: { fruit: 'Bananas' },
+			originalContent: '<div>Bananas</div>',
+		} );
 
 		expect( valid ).toBe( true );
 	} );