Enhance attributes to specify types and defaults

.eslintrc.json

@@ -94,7 +94,7 @@
 		"one-var": "off",
 		"padded-blocks": [ "error", "never" ],
 		"prefer-const": "error",
-		"quote-props": [ "error", "as-needed", { "keywords": true } ],
+		"quote-props": [ "error", "as-needed" ],
 		"react/display-name": "off",
 		"react/jsx-curly-spacing": [ "error", "always" ],
 		"react/jsx-equals-spacing": "error",

blocks/README.md

@@ -136,7 +136,10 @@ add_action( 'enqueue_block_editor_assets', 'random_image_enqueue_block_editor_as
 		category: 'media',
 
 		attributes: {
-			category: query.attr( 'img', 'alt' )
+			category: {
+				type: 'string',
+				source: query.attr( 'img', 'alt' )
+			}
 		},
 
 		edit: function( props ) {
@@ -231,11 +234,13 @@ editor interface where blocks are implemented.
   [Dashicon](https://developer.wordpress.org/resource/dashicons/#awards)
   to be shown in the control's button, or an element (or function returning an
   element) if you choose to render your own SVG.
-- `attributes: Object | Function` - An object of
-  [matchers](http://github.com/aduth/hpq) or a function which, when passed the
-  raw content of the block, returns block attributes as an object. When defined
-  as an object of matchers, the attributes object is generated with values
-  corresponding to the shape of the matcher object keys.
+- `attributes: Object | Function` - An object of attribute schemas, where the
+  keys of the object define the shape of attributes, and each value an object
+  schema describing the `type`, `default` (optional), and
+  [`source`](http://gutenberg-devdoc.surge.sh/reference/attribute-matchers/)
+  (optional) of the attribute. If `source` is omitted, the attribute is 
+  serialized into the block's comment delimiters. Alternatively, define 
+  `attributes` as a function which returns the attributes object.
 - `category: string` - Slug of the block's category. The category is used to
   organize the blocks in the block inserter.
 - `edit( { attributes: Object, setAttributes: Function } ): WPElement` -

blocks/api/factory.js

@@ -2,7 +2,14 @@
  * External dependencies
  */
 import uuid from 'uuid/v4';
-import { get, castArray, findIndex, isObjectLike, find } from 'lodash';
+import {
+	get,
+	reduce,
+	castArray,
+	findIndex,
+	isObjectLike,
+	find,
+} from 'lodash';
 
 /**
  * Internal dependencies
@@ -20,22 +27,26 @@ export function createBlock( name, attributes = {} ) {
 	// Get the type definition associated with a registered block.
 	const blockType = getBlockType( name );
 
-	// Do we need this? What purpose does it have?
-	let defaultAttributes;
-	if ( blockType ) {
-		defaultAttributes = blockType.defaultAttributes;
-	}
+	// Ensure attributes contains only values defined by block type, and merge
+	// default values for missing attributes.
+	attributes = reduce( blockType.attributes, ( result, source, key ) => {
+		const value = attributes[ key ];
+		if ( undefined !== value ) {
+			result[ key ] = value;
+		} else if ( source.default ) {
+			result[ key ] = source.default;
+		}
+
+		return result;
+	}, {} );
 
 	// Blocks are stored with a unique ID, the assigned type name,
 	// and the block attributes.
 	return {
 		uid: uuid(),
 		name,
 		isValid: true,
-		attributes: {
-			...defaultAttributes,
-			...attributes,
-		},
+		attributes,
 	};
 }
 

blocks/api/parser.js

@@ -2,7 +2,7 @@
  * External dependencies
  */
 import { parse as hpqParse } from 'hpq';
-import { pickBy } from 'lodash';
+import { mapValues, reduce, pickBy } from 'lodash';
 
 /**
  * WordPress dependencies
@@ -17,29 +17,78 @@ import { getBlockType, getUnknownTypeHandlerName } from './registration';
 import { createBlock } from './factory';
 import { isValidBlock } from './validation';
 
+/**
+ * Returns true if the provided function is a valid attribute source, or false
+ * otherwise.
+ *
+ * Sources are implemented as functions receiving a DOM node to select data
+ * from. Using the DOM is incidental and we shouldn't guarantee a contract that
+ * this be provided, else block implementers may feel inclined to use the node.
+ * Instead, sources are intended as a generic interface to query data from any
+ * tree shape. Here we pick only sources which include an internal flag.
+ *
+ * @param  {Function} source Function to test
+ * @return {Boolean}         Whether function is an attribute source
+ */
+export function isValidSource( source ) {
+	return !! source && '_wpBlocksKnownMatcher' in source;
+}
+
 /**
  * Returns the block attributes parsed from raw content.
  *
- * @param  {String} rawContent    Raw block content
- * @param  {Object} attributes    Block attribute matchers
- * @return {Object}               Block attributes
+ * @param  {String} rawContent Raw block content
+ * @param  {Object} schema     Block attribute schema
+ * @return {Object}            Block attribute values
  */
-export function parseBlockAttributes( rawContent, attributes ) {
-	if ( 'function' === typeof attributes ) {
-		return attributes( rawContent );
-	} else if ( attributes ) {
-		// Matchers are implemented as functions that receive a DOM node from
-		// which to select data. Use of the DOM is incidental and we shouldn't
-		// guarantee a contract that this be provided, else block implementers
-		// may feel compelled to use the node. Instead, matchers are intended
-		// as a generic interface to query data from any tree shape. Here we
-		// pick only matchers which include an internal flag.
-		const knownMatchers = pickBy( attributes, '_wpBlocksKnownMatcher' );
-
-		return hpqParse( rawContent, knownMatchers );
+export function getSourcedAttributes( rawContent, schema ) {
+	const sources = mapValues(
+		// Parse only sources with source defined
+		pickBy( schema, ( attributeSchema ) => isValidSource( attributeSchema.source ) ),
+
+		// Transform to object where source is value
+		( attributeSchema ) => attributeSchema.source
+	);
+
+	return hpqParse( rawContent, sources );
+}
+
+/**
+ * Returns value coerced to the specified JSON schema type string
+ *
+ * @see http://json-schema.org/latest/json-schema-validation.html#rfc.section.6.25
+ *
+ * @param  {*}      value Original value
+ * @param  {String} type  Type to coerce
+ * @return {*}            Coerced value
+ */
+export function asType( value, type ) {
+	switch ( type ) {
+		case 'string':
+			return String( value );
+
+		case 'boolean':
+			return Boolean( value );
+
+		case 'object':
+			return Object( value );
+
+		case 'null':
+			return null;
+
+		case 'array':
+			if ( Array.isArray( value ) ) {
+				return value;
+			}
+
+			return Array.from( value );
+
+		case 'integer':
+		case 'number':
+			return Number( value );
 	}
 
-	return {};
+	return value;
 }
 
 /**
@@ -51,18 +100,57 @@ export function parseBlockAttributes( rawContent, attributes ) {
  * @return {Object}                All block attributes
  */
 export function getBlockAttributes( blockType, rawContent, attributes ) {
-	// Merge any attributes present in comment delimiters with those
-	// that are specified in the block implementation.
-	attributes = attributes || {};
-	if ( blockType ) {
-		attributes = {
-			...blockType.defaultAttributes,
-			...attributes,
-			...parseBlockAttributes( rawContent, blockType.attributes ),
-		};
-	}
+	// Retrieve additional attributes sourced from content
+	const sourcedAttributes = getSourcedAttributes(
+		rawContent,
+		blockType.attributes
+	);
+
+	return reduce( blockType.attributes, ( result, source, key ) => {
+		let value;
+		if ( sourcedAttributes.hasOwnProperty( key ) ) {
+			value = sourcedAttributes[ key ];
+		} else if ( attributes ) {
+			value = attributes[ key ];
+		}
+
+		// Return default if attribute value not assigned
+		if ( undefined === value ) {
+			// Nest the condition so that constructor coercion never occurs if
+			// value is undefined and block type doesn't specify default value
+			if ( 'default' in source ) {
+				value = source.default;
+			} else {
+				return result;
+			}
+		}
+
+		// Coerce value to specified type
+		const coercedValue = asType( value, source.type );
+
+		if ( 'development' === process.env.NODE_ENV &&
+				! sourcedAttributes.hasOwnProperty( key ) &&
+				value !== coercedValue ) {
+			// Only in case of sourcing attribute from content do we want to
+			// allow coercion, as comment attributes are serialized respecting
+			// original data type. In development environments, log if value
+			// coerced to specified type is not strictly equal. We still allow
+			// coerced value to be assigned into attributes to avoid errors.
+			//
+			// Example:
+			//   Number( 5 ) === 5
+			//   Number( '5' ) !== '5'
+
+			// eslint-disable-next-line no-console
+			console.error(
+				`Expected attribute "${ key }" of type ${ source.type } for ` +
+				`block type "${ blockType.name }" but received ${ typeof value }.`
+			);
+		}
 
-	return attributes;
+		result[ key ] = coercedValue;
+		return result;
+	}, {} );
 }
 
 /**

blocks/api/paste.js

@@ -13,7 +13,7 @@ import { nodetypes } from '@wordpress/utils';
  */
 import { createBlock } from './factory';
 import { getBlockTypes, getUnknownTypeHandlerName } from './registration';
-import { parseBlockAttributes } from './parser';
+import { getSourcedAttributes } from './parser';
 import stripAttributes from './paste/strip-attributes';
 import removeSpans from './paste/remove-spans';
 
@@ -94,10 +94,12 @@ export default function( nodes ) {
 				return acc;
 			}
 
-			const { name, defaultAttributes = [] } = blockType;
-			const attributes = parseBlockAttributes( node.outerHTML, transform.attributes );
+			const attributes = getSourcedAttributes(
+				node.outerHTML,
+				transform.attributes,
+			);
 
-			return createBlock( name, { ...defaultAttributes, ...attributes } );
+			return createBlock( blockType.name, attributes );
 		}, null );
 
 		if ( block ) {

blocks/api/serializer.js

@@ -1,7 +1,7 @@
 /**
  * External dependencies
  */
-import { isEmpty, reduce, isObject, isEqual, pickBy } from 'lodash';
+import { isEmpty, reduce, isObject } from 'lodash';
 import { html as beautifyHtml } from 'js-beautify';
 import classnames from 'classnames';
 
@@ -14,7 +14,6 @@ import { Component, createElement, renderToString, cloneElement, Children } from
  * Internal dependencies
  */
 import { getBlockType } from './registration';
-import { parseBlockAttributes } from './parser';
 
 /**
  * Returns the block's default classname from its name
@@ -71,36 +70,43 @@ export function getSaveContent( blockType, attributes ) {
 }
 
 /**
- * Returns attributes which ought to be saved
- * and serialized into the block comment header
+ * Returns attributes which are to be saved and serialized into the block
+ * comment delimiter.
  *
- * When a block exists in memory it contains as its attributes
- * both those which come from the block comment header _and_
- * those which come from parsing the contents of the block.
+ * When a block exists in memory it contains as its attributes both those
+ * parsed the block comment delimiter _and_ those which matched from the
+ * contents of the block.
  *
- * This function returns only those attributes which are
- * needed to persist and which cannot already be inferred
- * from the block content.
+ * This function returns only those attributes which are needed to persist and
+ * which cannot be matched from the block content.
  *
- * @param {Object<String,*>}   allAttributes         Attributes from in-memory block data
- * @param {Object<String,*>}   attributesFromContent Attributes which are inferred from block content
- * @returns {Object<String,*>} filtered set of attributes for minimum save/serialization
+ * @param   {Object<String,*>} allAttributes Attributes from in-memory block data
+ * @param   {Object<String,*>} schema        Block type schema
+ * @returns {Object<String,*>}               Subset of attributes for comment serialization
  */
-export function getCommentAttributes( allAttributes, attributesFromContent ) {
-	// Iterate over attributes and produce the set to save
-	return reduce(
-		Object.keys( allAttributes ),
-		( toSave, key ) => {
-			const allValue = allAttributes[ key ];
-			const contentValue = attributesFromContent[ key ];
-
-			// save only if attribute if not inferred from the content and if valued
-			return ! ( contentValue !== undefined || allValue === undefined )
-				? Object.assign( toSave, { [ key ]: allValue } )
-				: toSave;
-		},
-		{},
-	);
+export function getCommentAttributes( allAttributes, schema ) {
+	return reduce( schema, ( result, attributeSchema, key ) => {
+		const value = allAttributes[ key ];
+
+		// Ignore undefined values
+		if ( undefined === value ) {
+			return result;
+		}
+
+		// Ignore values sources from content
+		if ( attributeSchema.source ) {
+			return result;
+		}
+
+		// Ignore default value
+		if ( 'default' in attributeSchema && attributeSchema.default === value ) {
+			return result;
+		}
+
+		// Otherwise, include in comment set
+		result[ key ] = value;
+		return result;
+	}, {} );
 }
 
 export function serializeAttributes( attrs ) {
@@ -138,12 +144,7 @@ export function serializeBlock( block ) {
 		saveContent = block.originalContent;
 	}
 
-	let saveAttributes = getCommentAttributes( block.attributes, parseBlockAttributes( saveContent, blockType.attributes ) );
-
-	// Remove attributes that are the same as the defaults.
-	if ( blockType.defaultAttributes ) {
-		saveAttributes = pickBy( saveAttributes, ( value, key ) => ! isEqual( value, blockType.defaultAttributes[ key ] ) );
-	}
+	const saveAttributes = getCommentAttributes( block.attributes, blockType.attributes );
 
 	if ( 'core/more' === blockName ) {
 		return `<!--more${ saveAttributes.text ? ` ${ saveAttributes.text }` : '' }-->${ saveAttributes.noTeaser ? '\n<!--noteaser-->' : '' }`;