Blocks: Deprecate isValidBlockContent

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.

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.
@@ -857,6 +859,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_ `import('../parser').WPBlock`: block object.
+-   _blockTypeOrName_ `[import('../registration').WPBlockType]`: Block type or name, inferred from block if not given.
+
+_Returns_
+
+-   `[boolean,Object]`: validation results.
+
 ### withBlockContentContext
 
 A Higher Order Component used to inject BlockContent using context to the

packages/blocks/src/api/index.js

@@ -88,7 +88,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';
 
 /**
@@ -700,12 +701,12 @@ 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 {import('../parser').WPBlock}           block             block object.
+ * @param {import('../registration').WPBlockType} [blockTypeOrName] Block type or name, inferred from block if not given.
  *
  * @return {[boolean,Object]} validation results.
  */
-export function validateBlock( block, blockTypeOrName ) {
+export function validateBlock( block, blockTypeOrName = block.name ) {
 	const isFallbackBlock =
 		block.name === getFreeformContentHandlerName() ||
 		block.name === getUnregisteredTypeHandlerName();
@@ -755,6 +756,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 +769,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,

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 );
 	} );