Do not separately sanitize attributes in cloneSanitizedBlock

The only reason we still had this was to fill in default values for
any internal attrs that got stripped on duplication; since there is
no obvious use case for an internal attribute with defaults I'm
removing this for now.

I leave __experimentalSanitizeBlockAttributes as it is still used in
createBlock, and keeping it pulled out like this allows for ease of
unit testing.

packages/blocks/src/api/factory.js

@@ -95,8 +95,9 @@ export function createBlocksFromInnerBlocksTemplate(
 }
 
 /**
- * Given a block object, returns a copy of the block object while sanitizing its attributes,
+ * Given a block object, returns a copy of the block object,
  * optionally merging new attributes and/or replacing its inner blocks.
+ * Attributes with the `internal` role are not copied into the new object.
  *
  * @param {Object} block           Block instance.
  * @param {Object} mergeAttributes Block attributes.
@@ -122,17 +123,10 @@ export function __experimentalCloneSanitizedBlock(
 		'internal'
 	);
 
-	// Remove any attributes not defined in the block type, and fill in default values for
-	// misisng attributes.
-	const sanitizedAttributes = __experimentalSanitizeBlockAttributes(
-		block.name,
-		retainedAttributes
-	);
-
 	return {
 		...block,
 		clientId,
-		attributes: sanitizedAttributes,
+		attributes: retainedAttributes,
 		innerBlocks:
 			newInnerBlocks ||
 			block.innerBlocks.map( ( innerBlock ) =>

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

@@ -428,52 +428,182 @@ describe( 'block factory', () => {
 	} );
 
 	describe( '__experimentalCloneSanitizedBlock', () => {
-		it( 'should sanitize attributes not defined in the block type', () => {
+		it( 'should not duplicate internal attributes', () => {
 			registerBlockType( 'core/test-block', {
 				...defaultBlockSettings,
 				attributes: {
-					align: {
+					internalAttribute: {
+						type: 'string',
+						__experimentalRole: 'internal',
+					},
+					contentAttribute: {
+						type: 'string',
+						__experimentalRole: 'content',
+					},
+					attributeWithNoRole: {
 						type: 'string',
 					},
 				},
 			} );
 
 			const block = createBlock( 'core/test-block', {
-				notDefined: 'not-defined',
+				internalAttribute: 'this-should-not-be-copied',
+				contentAttribute: 'some content',
+				attributeWithNoRole: 'another attribute',
 			} );
 
+			const clonedBlock = __experimentalCloneSanitizedBlock( block, {} );
+
+			expect( clonedBlock.attributes ).toEqual( {
+				contentAttribute: 'some content',
+				attributeWithNoRole: 'another attribute',
+			} );
+		} );
+
+		it( 'should merge attributes into the existing block', () => {
+			registerBlockType( 'core/test-block', {
+				attributes: {
+					align: {
+						type: 'string',
+					},
+					isDifferent: {
+						type: 'boolean',
+						default: false,
+					},
+					includesDefault: {
+						type: 'boolean',
+						default: true,
+					},
+					includesFalseyDefault: {
+						type: 'number',
+						default: 0,
+					},
+					content: {
+						type: 'array',
+						source: 'children',
+					},
+					defaultContent: {
+						type: 'array',
+						source: 'children',
+						default: 'test',
+					},
+					unknownDefaultContent: {
+						type: 'array',
+						source: 'children',
+						default: 1,
+					},
+					htmlContent: {
+						source: 'html',
+					},
+				},
+				save: noop,
+				category: 'text',
+				title: 'test block',
+			} );
+			const block = deepFreeze(
+				createBlock( 'core/test-block', { align: 'left' }, [
+					createBlock( 'core/test-block' ),
+				] )
+			);
+
 			const clonedBlock = __experimentalCloneSanitizedBlock( block, {
-				notDefined2: 'not-defined-2',
+				isDifferent: true,
+				htmlContent: 'test',
 			} );
 
-			expect( clonedBlock.attributes ).toEqual( {} );
+			expect( clonedBlock.name ).toEqual( block.name );
+			expect( clonedBlock.attributes ).toEqual( {
+				includesDefault: true,
+				includesFalseyDefault: 0,
+				align: 'left',
+				isDifferent: true,
+				content: [],
+				defaultContent: [ 'test' ],
+				unknownDefaultContent: [],
+				htmlContent: 'test',
+			} );
+			expect( clonedBlock.innerBlocks ).toHaveLength( 1 );
+			expect( typeof clonedBlock.clientId ).toBe( 'string' );
+			expect( clonedBlock.clientId ).not.toBe( block.clientId );
 		} );
-		it( 'should not duplicate internal attributes, but fallback to available defaults', () => {
+
+		it( 'should replace inner blocks of the existing block', () => {
 			registerBlockType( 'core/test-block', {
-				...defaultBlockSettings,
 				attributes: {
-					internalAttribute: {
+					align: {
 						type: 'string',
-						__experimentalRole: 'internal',
 					},
-					internalAttributeWithDefault: {
-						type: 'string',
-						__experimentalRole: 'internal',
-						default: 'default-value',
+					isDifferent: {
+						type: 'boolean',
+						default: false,
 					},
 				},
+				save: noop,
+				category: 'text',
+				title: 'test block',
 			} );
+			const block = deepFreeze(
+				createBlock( 'core/test-block', { align: 'left' }, [
+					createBlock( 'core/test-block', { align: 'right' } ),
+					createBlock( 'core/test-block', { align: 'left' } ),
+				] )
+			);
 
-			const block = createBlock( 'core/test-block', {
-				internalAttribute: 'unique-value',
-				internalAttributeWithDefault: 'unique-non-default-value',
-			} );
+			const clonedBlock = __experimentalCloneSanitizedBlock(
+				block,
+				undefined,
+				[ createBlock( 'core/test-block' ) ]
+			);
 
-			const clonedBlock = __experimentalCloneSanitizedBlock( block, {} );
+			expect( clonedBlock.innerBlocks ).toHaveLength( 1 );
+			expect(
+				clonedBlock.innerBlocks[ 0 ].attributes
+			).not.toHaveProperty( 'align' );
+		} );
 
-			expect( clonedBlock.attributes ).toEqual( {
-				internalAttributeWithDefault: 'default-value',
+		it( 'should clone innerBlocks if innerBlocks are not passed', () => {
+			registerBlockType( 'core/test-block', {
+				attributes: {
+					align: {
+						type: 'string',
+					},
+					isDifferent: {
+						type: 'boolean',
+						default: false,
+					},
+				},
+				save: noop,
+				category: 'text',
+				title: 'test block',
 			} );
+			const block = deepFreeze(
+				createBlock( 'core/test-block', { align: 'left' }, [
+					createBlock( 'core/test-block', { align: 'right' } ),
+					createBlock( 'core/test-block', { align: 'left' } ),
+				] )
+			);
+
+			const clonedBlock = __experimentalCloneSanitizedBlock( block );
+
+			expect( clonedBlock.innerBlocks ).toHaveLength( 2 );
+			expect( clonedBlock.innerBlocks[ 0 ].clientId ).not.toBe(
+				block.innerBlocks[ 0 ].clientId
+			);
+			expect( clonedBlock.innerBlocks[ 0 ].attributes ).not.toBe(
+				block.innerBlocks[ 0 ].attributes
+			);
+			expect( clonedBlock.innerBlocks[ 0 ].attributes ).toEqual(
+				block.innerBlocks[ 0 ].attributes
+			);
+			expect( clonedBlock.innerBlocks[ 1 ].clientId ).not.toBe(
+				block.innerBlocks[ 1 ].clientId
+			);
+			expect( clonedBlock.innerBlocks[ 1 ].attributes ).not.toBe(
+				block.innerBlocks[ 1 ].attributes
+			);
+			expect( clonedBlock.innerBlocks[ 1 ].attributes ).toEqual(
+				block.innerBlocks[ 1 ].attributes
+			);
 		} );
 	} );