Add stricter block settings validation (#1345)

WordPress · Jul 3, 2017 · 6c6d682 · 6c6d682
1 parent fa445c0
commit 6c6d682
Show file tree

Hide file tree

Showing 5 changed files with 104 additions and 38 deletions.
diff --git a/blocks/api/registration.js b/blocks/api/registration.js
@@ -1,5 +1,10 @@
 /* eslint no-console: [ 'error', { allow: [ 'error' ] } ] */
 
+/**
+ * External dependencies
+ */
+import { isFunction } from 'lodash';
+
 /**
  * Block settings keyed by block name.
  *
@@ -44,6 +49,18 @@ export function registerBlockType( name, settings ) {
 		);
 		return;
 	}
+	if ( ! settings || ! isFunction( settings.save ) ) {
+		console.error(
+			'The "save" property must be specified and must be a valid function.'
+		);
+		return;
+	}
+	if ( 'edit' in settings && ! isFunction( settings.edit ) ) {
+		console.error(
+			'The "edit" property must be a valid function.'
+		);
+		return;
+	}
 	if ( blocks[ name ] ) {
 		console.error(
 			'Block "' + name + '" is already registered.'

diff --git a/blocks/api/test/factory.js b/blocks/api/test/factory.js
@@ -2,6 +2,7 @@
  * External dependencies
  */
 import { expect } from 'chai';
+import { noop } from 'lodash';
 
 /**
  * Internal dependencies
@@ -10,6 +11,8 @@ import { createBlock, switchToBlockType } from '../factory';
 import { getBlockTypes, unregisterBlockType, setUnknownTypeHandler, registerBlockType } from '../registration';
 
 describe( 'block factory', () => {
+	const defaultBlockSettings = { save: noop };
+
 	afterEach( () => {
 		setUnknownTypeHandler( undefined );
 		getBlockTypes().forEach( ( block ) => {
@@ -23,6 +26,7 @@ describe( 'block factory', () => {
 				defaultAttributes: {
 					includesDefault: true,
 				},
+				save: noop,
 			} );
 			const block = createBlock( 'core/test-block', {
 				align: 'left',
@@ -50,8 +54,9 @@ describe( 'block factory', () => {
 						},
 					} ],
 				},
+				save: noop,
 			} );
-			registerBlockType( 'core/text-block', {} );
+			registerBlockType( 'core/text-block', defaultBlockSettings );
 
 			const block = {
 				uid: 1,
@@ -73,7 +78,7 @@ describe( 'block factory', () => {
 		} );
 
 		it( 'should switch the blockType of a block using the "transform to"', () => {
-			registerBlockType( 'core/updated-text-block', {} );
+			registerBlockType( 'core/updated-text-block', defaultBlockSettings );
 			registerBlockType( 'core/text-block', {
 				transforms: {
 					to: [ {
@@ -85,6 +90,7 @@ describe( 'block factory', () => {
 						},
 					} ],
 				},
+				save: noop,
 			} );
 
 			const block = {
@@ -107,8 +113,8 @@ describe( 'block factory', () => {
 		} );
 
 		it( 'should return null if no transformation is found', () => {
-			registerBlockType( 'core/updated-text-block', {} );
-			registerBlockType( 'core/text-block', {} );
+			registerBlockType( 'core/updated-text-block', defaultBlockSettings );
+			registerBlockType( 'core/text-block', defaultBlockSettings );
 
 			const block = {
 				uid: 1,
@@ -131,8 +137,9 @@ describe( 'block factory', () => {
 						transform: () => null,
 					} ],
 				},
+				save: noop,
 			} );
-			registerBlockType( 'core/text-block', {} );
+			registerBlockType( 'core/text-block', defaultBlockSettings );
 
 			const block = {
 				uid: 1,
@@ -155,8 +162,9 @@ describe( 'block factory', () => {
 						transform: () => [],
 					} ],
 				},
+				save: noop,
 			} );
-			registerBlockType( 'core/text-block', {} );
+			registerBlockType( 'core/text-block', defaultBlockSettings );
 
 			const block = {
 				uid: 1,
@@ -185,8 +193,9 @@ describe( 'block factory', () => {
 						},
 					} ],
 				},
+				save: noop,
 			} );
-			registerBlockType( 'core/text-block', {} );
+			registerBlockType( 'core/text-block', defaultBlockSettings );
 
 			const block = {
 				uid: 1,
@@ -220,8 +229,9 @@ describe( 'block factory', () => {
 						},
 					} ],
 				},
+				save: noop,
 			} );
-			registerBlockType( 'core/text-block', {} );
+			registerBlockType( 'core/text-block', defaultBlockSettings );
 
 			const block = {
 				uid: 1,
@@ -237,7 +247,7 @@ describe( 'block factory', () => {
 		} );
 
 		it( 'should reject single transformations with unexpected block types', () => {
-			registerBlockType( 'core/updated-text-block', {} );
+			registerBlockType( 'core/updated-text-block', defaultBlockSettings );
 			registerBlockType( 'core/text-block', {
 				transforms: {
 					to: [ {
@@ -249,6 +259,7 @@ describe( 'block factory', () => {
 						},
 					} ],
 				},
+				save: noop,
 			} );
 
 			const block = {
@@ -265,7 +276,7 @@ describe( 'block factory', () => {
 		} );
 
 		it( 'should reject array transformations with unexpected block types', () => {
-			registerBlockType( 'core/updated-text-block', {} );
+			registerBlockType( 'core/updated-text-block', defaultBlockSettings );
 			registerBlockType( 'core/text-block', {
 				transforms: {
 					to: [ {
@@ -282,6 +293,7 @@ describe( 'block factory', () => {
 						},
 					} ],
 				},
+				save: noop,
 			} );
 
 			const block = {
@@ -298,7 +310,7 @@ describe( 'block factory', () => {
 		} );
 
 		it( 'should accept valid array transformations', () => {
-			registerBlockType( 'core/updated-text-block', {} );
+			registerBlockType( 'core/updated-text-block', defaultBlockSettings );
 			registerBlockType( 'core/text-block', {
 				transforms: {
 					to: [ {
@@ -315,6 +327,7 @@ describe( 'block factory', () => {
 						},
 					} ],
 				},
+				save: noop,
 			} );
 
 			const block = {

diff --git a/blocks/api/test/parser.js b/blocks/api/test/parser.js
@@ -2,6 +2,7 @@
  * External dependencies
  */
 import { expect } from 'chai';
+import { noop } from 'lodash';
 
 /**
  * Internal dependencies
@@ -21,6 +22,8 @@ import {
 } from '../registration';
 
 describe( 'block parser', () => {
+	const defaultBlockSettings = { save: noop };
+
 	afterEach( () => {
 		setUnknownTypeHandler( undefined );
 		getBlockTypes().forEach( ( block ) => {
@@ -93,7 +96,7 @@ describe( 'block parser', () => {
 
 	describe( 'createBlockWithFallback', () => {
 		it( 'should create the requested block if it exists', () => {
-			registerBlockType( 'core/test-block', {} );
+			registerBlockType( 'core/test-block', defaultBlockSettings );
 
 			const block = createBlockWithFallback(
 				'core/test-block',
@@ -105,15 +108,15 @@ describe( 'block parser', () => {
 		} );
 
 		it( 'should create the requested block with no attributes if it exists', () => {
-			registerBlockType( 'core/test-block', {} );
+			registerBlockType( 'core/test-block', defaultBlockSettings );
 
 			const block = createBlockWithFallback( 'core/test-block', 'content' );
 			expect( block.name ).to.eql( 'core/test-block' );
 			expect( block.attributes ).to.eql( {} );
 		} );
 
 		it( 'should fall back to the unknown type handler for unknown blocks if present', () => {
-			registerBlockType( 'core/unknown-block', {} );
+			registerBlockType( 'core/unknown-block', defaultBlockSettings );
 			setUnknownTypeHandler( 'core/unknown-block' );
 
 			const block = createBlockWithFallback(
@@ -126,7 +129,7 @@ describe( 'block parser', () => {
 		} );
 
 		it( 'should fall back to the unknown type handler if block type not specified', () => {
-			registerBlockType( 'core/unknown-block', {} );
+			registerBlockType( 'core/unknown-block', defaultBlockSettings );
 			setUnknownTypeHandler( 'core/unknown-block' );
 
 			const block = createBlockWithFallback( null, 'content' );
@@ -149,6 +152,7 @@ describe( 'block parser', () => {
 						content: rawContent,
 					};
 				},
+				save: noop,
 			} );
 
 			const parsed = parse(
@@ -181,6 +185,7 @@ describe( 'block parser', () => {
 						content: rawContent + ' & Chicken',
 					};
 				},
+				save: noop,
 			} );
 
 			const parsed = parse(
@@ -198,8 +203,8 @@ describe( 'block parser', () => {
 		} );
 
 		it( 'should parse the post content, using unknown block handler', () => {
-			registerBlockType( 'core/test-block', {} );
-			registerBlockType( 'core/unknown-block', {} );
+			registerBlockType( 'core/test-block', defaultBlockSettings );
+			registerBlockType( 'core/unknown-block', defaultBlockSettings );
 
 			setUnknownTypeHandler( 'core/unknown-block' );
 
@@ -218,14 +223,15 @@ describe( 'block parser', () => {
 		} );
 
 		it( 'should parse the post content, including raw HTML at each end', () => {
-			registerBlockType( 'core/test-block', {} );
+			registerBlockType( 'core/test-block', defaultBlockSettings );
 			registerBlockType( 'core/unknown-block', {
 				// Currently this is the only way to test block content parsing?
 				attributes: function( rawContent ) {
 					return {
 						content: rawContent,
 					};
 				},
+				save: noop,
 			} );
 
 			setUnknownTypeHandler( 'core/unknown-block' );
@@ -252,7 +258,7 @@ describe( 'block parser', () => {
 		} );
 
 		it( 'should parse blocks with empty content', () => {
-			registerBlockType( 'core/test-block', {} );
+			registerBlockType( 'core/test-block', defaultBlockSettings );
 			const parsed = parse(
 				'<!-- wp:core/test-block --><!-- /wp:core/test-block -->'
 			);
@@ -264,8 +270,8 @@ describe( 'block parser', () => {
 		} );
 
 		it( 'should parse void blocks', () => {
-			registerBlockType( 'core/test-block', {} );
-			registerBlockType( 'core/void-block', {} );
+			registerBlockType( 'core/test-block', defaultBlockSettings );
+			registerBlockType( 'core/void-block', defaultBlockSettings );
 			const parsed = parse(
 				'<!-- wp:core/test-block --><!-- /wp:core/test-block -->' +
 				'<!-- wp:core/void-block /-->'