From a52980bd0f8dad5dad66f20715bd1b2c5aa9a434 Mon Sep 17 00:00:00 2001
From: Andrew Duthie <andrew@andrewduthie.com>
Date: Thu, 1 Jun 2017 16:18:57 -0400
Subject: [PATCH] Change attribute serialization to explicit opt-in

---
 blocks/api/serializer.js       | 43 +++++++++++++++-------------------
 blocks/api/test/serializer.js  | 39 ++++++++++++++++--------------
 blocks/library/button/index.js |  5 ++++
 blocks/library/image/index.js  |  5 ++++
 blocks/library/quote/index.js  |  5 ++++
 5 files changed, 56 insertions(+), 41 deletions(-)

diff --git a/blocks/api/serializer.js b/blocks/api/serializer.js
index 3d114e4aa3183..7c025e59eab35 100644
--- a/blocks/api/serializer.js
+++ b/blocks/api/serializer.js
@@ -1,14 +1,13 @@
 /**
  * External dependencies
  */
-import { difference } from 'lodash';
+import { reduce } from 'lodash';
 import { html as beautifyHtml } from 'js-beautify';
 
 /**
  * Internal dependencies
  */
 import { getBlockType } from './registration';
-import { parseBlockAttributes } from './parser';
 
 /**
  * Given a block's save render implementation and attributes, returns the
@@ -37,30 +36,28 @@ export function getSaveContent( save, attributes ) {
 }
 
 /**
- * Returns comment attributes as serialized string, determined by subset of
- * difference between actual attributes of a block and those expected based
- * on its settings.
+ * Returns comment attributes as serialized string, determined by the return
+ * value object of the block's `encodeAttributes` implementation. If a block
+ * does not provide an implementation, an empty string is returned.
  *
- * @param  {Object} realAttributes     Actual block attributes
- * @param  {Object} expectedAttributes Expected block attributes
- * @return {string}                    Comment attributes
+ * @param  {?Function} encodeAttributes Attribute encoding implementation
+ * @param  {Object}    attributes       Block attributes
+ * @return {String}                     Comment attributes
  */
-export function getCommentAttributes( realAttributes, expectedAttributes ) {
-	// Find difference and build into object subset of attributes.
-	const keys = difference(
-		Object.keys( realAttributes ),
-		Object.keys( expectedAttributes )
-	);
+export function getCommentAttributes( encodeAttributes, attributes ) {
+	let encodedAttributes;
+	if ( encodeAttributes ) {
+		encodedAttributes = encodeAttributes( attributes );
+	}
 
 	// Serialize the comment attributes as `key="value"`.
-	return keys.reduce( ( memo, key ) => {
-		const value = realAttributes[ key ];
-		if ( undefined === value ) {
-			return memo;
+	return reduce( encodedAttributes, ( result, value, key ) => {
+		if ( undefined !== value ) {
+			result.push( `${ key }="${ value }"` );
 		}
 
-		return memo + `${ key }="${ value }" `;
-	}, '' );
+		return result;
+	}, [] ).join( ' ' );
 }
 
 /**
@@ -74,6 +71,7 @@ export default function serialize( blocks ) {
 		const blockName = block.name;
 		const blockType = getBlockType( blockName );
 		const saveContent = getSaveContent( blockType.save, block.attributes );
+		const commentAttributes = getCommentAttributes( blockType.encodeAttributes, block.attributes );
 		const beautifyOptions = {
 			indent_inner_html: true,
 			wrap_line_length: 0,
@@ -83,10 +81,7 @@ export default function serialize( blocks ) {
 			'<!-- wp:' +
 			blockName +
 			' ' +
-			getCommentAttributes(
-				block.attributes,
-				parseBlockAttributes( saveContent, blockType )
-			) +
+			( commentAttributes ? commentAttributes + ' ' : '' ) +
 			'-->' +
 			( saveContent ? '\n' + beautifyHtml( saveContent, beautifyOptions ) + '\n' : '' ) +
 			'<!-- /wp:' +
diff --git a/blocks/api/test/serializer.js b/blocks/api/test/serializer.js
index 75621e39e31a0..a8868dd11bdf8 100644
--- a/blocks/api/test/serializer.js
+++ b/blocks/api/test/serializer.js
@@ -2,6 +2,7 @@
  * External dependencies
  */
 import { expect } from 'chai';
+import { pick, identity } from 'lodash';
 
 /**
  * Internal dependencies
@@ -56,34 +57,34 @@ describe( 'block serializer', () => {
 	} );
 
 	describe( 'getCommentAttributes()', () => {
-		it( 'should return empty string if no difference', () => {
-			const attributes = getCommentAttributes( {}, {} );
+		it( 'should return empty string if no encodeAttributes implementation', () => {
+			const commentAttributes = getCommentAttributes( undefined, {
+				fruit: 'bananas',
+			} );
 
-			expect( attributes ).to.equal( '' );
+			expect( commentAttributes ).to.equal( '' );
 		} );
 
-		it( 'should return joined string of key:value pairs by difference subset', () => {
-			const attributes = getCommentAttributes( {
-				fruit: 'bananas',
-				category: 'food',
-				ripeness: 'ripe',
-			}, {
-				fruit: 'bananas',
-			} );
+		it( 'should return joined string of `key="value"` pairs by difference subset', () => {
+			const commentAttributes = getCommentAttributes(
+				( attributes ) => pick( attributes, 'category', 'ripeness' ),
+				{
+					fruit: 'bananas',
+					category: 'food',
+					ripeness: 'ripe',
+				}
+			);
 
-			expect( attributes ).to.equal( 'category="food" ripeness="ripe" ' );
+			expect( commentAttributes ).to.equal( 'category="food" ripeness="ripe"' );
 		} );
 
 		it( 'should not append an undefined attribute value', () => {
-			const attributes = getCommentAttributes( {
-				fruit: 'bananas',
+			const commentAttributes = getCommentAttributes( identity, {
 				category: 'food',
 				ripeness: undefined,
-			}, {
-				fruit: 'bananas',
 			} );
 
-			expect( attributes ).to.equal( 'category="food" ' );
+			expect( commentAttributes ).to.equal( 'category="food"' );
 		} );
 	} );
 
@@ -95,6 +96,10 @@ describe( 'block serializer', () => {
 						content: rawContent,
 					};
 				},
+				encodeAttributes( attributes ) {
+					const { align } = attributes;
+					return { align };
+				},
 				save( { attributes } ) {
 					return <p dangerouslySetInnerHTML={ { __html: attributes.content } } />;
 				},
diff --git a/blocks/library/button/index.js b/blocks/library/button/index.js
index f391e6d963451..f83ac94d4b634 100644
--- a/blocks/library/button/index.js
+++ b/blocks/library/button/index.js
@@ -39,6 +39,11 @@ registerBlockType( 'core/button', {
 		text: children( 'a' ),
 	},
 
+	encodeAttributes( attributes ) {
+		const { align } = attributes;
+		return { align };
+	},
+
 	controls: [
 		{
 			icon: 'align-left',
diff --git a/blocks/library/image/index.js b/blocks/library/image/index.js
index 2b7d06ab90810..57b44a2f76f2c 100644
--- a/blocks/library/image/index.js
+++ b/blocks/library/image/index.js
@@ -40,6 +40,11 @@ registerBlockType( 'core/image', {
 		caption: children( 'figcaption' ),
 	},
 
+	encodeAttributes( attributes ) {
+		const { align } = attributes;
+		return { align };
+	},
+
 	controls: [
 		{
 			icon: 'align-left',
diff --git a/blocks/library/quote/index.js b/blocks/library/quote/index.js
index e75a009503d7f..abb2d9eb9b67b 100644
--- a/blocks/library/quote/index.js
+++ b/blocks/library/quote/index.js
@@ -22,6 +22,11 @@ registerBlockType( 'core/quote', {
 		citation: children( 'footer' ),
 	},
 
+	encodeAttributes( attributes ) {
+		const { style } = attributes;
+		return { style };
+	},
+
 	controls: [ 1, 2 ].map( ( variation ) => ( {
 		icon: 'format-quote',
 		title: wp.i18n.sprintf( wp.i18n.__( 'Quote style %d' ), variation ),