Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Blocks: Move filter for registering block types before validation happens #5076

Merged
merged 1 commit into from
Feb 19, 2018
Merged

Blocks: Move filter for registering block types before validation happens #5076

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions blocks/api/registration.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,21 +93,24 @@ export function registerBlockType( name, settings ) {
);
return;
}
if ( ! settings || ! isFunction( settings.save ) ) {
if ( blocks[ name ] ) {
console.error(
'The "save" property must be specified and must be a valid function.'
'Block "' + name + '" is already registered.'
);
return;
}
if ( 'edit' in settings && ! isFunction( settings.edit ) ) {

settings = applyFilters( 'blocks.registerBlockType', settings, name );

if ( ! settings || ! isFunction( settings.save ) ) {
console.error(
'The "edit" property must be a valid function.'
'The "save" property must be specified and must be a valid function.'
);
return;
}
if ( blocks[ name ] ) {
if ( 'edit' in settings && ! isFunction( settings.edit ) ) {
console.error(
'Block "' + name + '" is already registered.'
'The "edit" property must be a valid function.'
);
return;
}
Expand Down Expand Up @@ -145,8 +148,6 @@ export function registerBlockType( name, settings ) {
settings.icon = 'block-default';
}

settings = applyFilters( 'blocks.registerBlockType', settings, name );

return blocks[ name ] = settings;
}

Expand Down
68 changes: 41 additions & 27 deletions blocks/api/test/registration.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
/* eslint-disable no-console */

/**
* External dependencies
*/
import { noop } from 'lodash';

/**
* WordPress dependencies
*/
import { addFilter, removeFilter } from '@wordpress/hooks';

/**
* Internal dependencies
*/
Expand All @@ -22,69 +25,62 @@ import {
} from '../registration';

describe( 'blocks', () => {
const error = console.error;
const defaultBlockSettings = { save: noop, category: 'common', title: 'block title' };

beforeAll( () => {
// Load all hooks that modify blocks
require( 'blocks/hooks' );
} );

// Reset block state before each test.
beforeEach( () => {
console.error = jest.fn();
} );

afterEach( () => {
getBlockTypes().forEach( block => {
unregisterBlockType( block.name );
} );
setUnknownTypeHandlerName( undefined );
setDefaultBlockName( undefined );
window._wpBlocks = {};
console.error = error;
} );

describe( 'registerBlockType()', () => {
it( 'should reject numbers', () => {
const block = registerBlockType( 999 );
expect( console.error ).toHaveBeenCalledWith( 'Block names must be strings.' );
expect( console ).toHaveErroredWith( 'Block names must be strings.' );
expect( block ).toBeUndefined();
} );

it( 'should reject blocks without a namespace', () => {
const block = registerBlockType( 'doing-it-wrong' );
expect( console.error ).toHaveBeenCalledWith( 'Block names must contain a namespace prefix, include only lowercase alphanumeric characters or dashes, and start with a letter. Example: my-plugin/my-custom-block' );
expect( console ).toHaveErroredWith( 'Block names must contain a namespace prefix, include only lowercase alphanumeric characters or dashes, and start with a letter. Example: my-plugin/my-custom-block' );
expect( block ).toBeUndefined();
} );

it( 'should reject blocks with too many namespaces', () => {
const block = registerBlockType( 'doing/it/wrong' );
expect( console.error ).toHaveBeenCalledWith( 'Block names must contain a namespace prefix, include only lowercase alphanumeric characters or dashes, and start with a letter. Example: my-plugin/my-custom-block' );
expect( console ).toHaveErroredWith( 'Block names must contain a namespace prefix, include only lowercase alphanumeric characters or dashes, and start with a letter. Example: my-plugin/my-custom-block' );
expect( block ).toBeUndefined();
} );

it( 'should reject blocks with invalid characters', () => {
const block = registerBlockType( 'still/_doing_it_wrong' );
expect( console.error ).toHaveBeenCalledWith( 'Block names must contain a namespace prefix, include only lowercase alphanumeric characters or dashes, and start with a letter. Example: my-plugin/my-custom-block' );
expect( console ).toHaveErroredWith( 'Block names must contain a namespace prefix, include only lowercase alphanumeric characters or dashes, and start with a letter. Example: my-plugin/my-custom-block' );
expect( block ).toBeUndefined();
} );

it( 'should reject blocks with uppercase characters', () => {
const block = registerBlockType( 'Core/Paragraph' );
expect( console.error ).toHaveBeenCalledWith( 'Block names must contain a namespace prefix, include only lowercase alphanumeric characters or dashes, and start with a letter. Example: my-plugin/my-custom-block' );
expect( console ).toHaveErroredWith( 'Block names must contain a namespace prefix, include only lowercase alphanumeric characters or dashes, and start with a letter. Example: my-plugin/my-custom-block' );
expect( block ).toBeUndefined();
} );

it( 'should reject blocks not starting with a letter', () => {
const block = registerBlockType( 'my-plugin/4-fancy-block', defaultBlockSettings );
expect( console.error ).toHaveBeenCalledWith( 'Block names must contain a namespace prefix, include only lowercase alphanumeric characters or dashes, and start with a letter. Example: my-plugin/my-custom-block' );
expect( console ).toHaveErroredWith( 'Block names must contain a namespace prefix, include only lowercase alphanumeric characters or dashes, and start with a letter. Example: my-plugin/my-custom-block' );
expect( block ).toBeUndefined();
} );

it( 'should accept valid block names', () => {
const block = registerBlockType( 'my-plugin/fancy-block-4', defaultBlockSettings );
expect( console.error ).not.toHaveBeenCalled();
expect( console ).not.toHaveErrored();
expect( block ).toEqual( {
name: 'my-plugin/fancy-block-4',
icon: 'block-default',
Expand All @@ -105,62 +101,62 @@ describe( 'blocks', () => {
it( 'should prohibit registering the same block twice', () => {
registerBlockType( 'core/test-block', defaultBlockSettings );
const block = registerBlockType( 'core/test-block', defaultBlockSettings );
expect( console.error ).toHaveBeenCalledWith( 'Block "core/test-block" is already registered.' );
expect( console ).toHaveErroredWith( 'Block "core/test-block" is already registered.' );
expect( block ).toBeUndefined();
} );

it( 'should reject blocks without a save function', () => {
const block = registerBlockType( 'my-plugin/fancy-block-5' );
expect( console.error ).toHaveBeenCalledWith( 'The "save" property must be specified and must be a valid function.' );
expect( console ).toHaveErroredWith( 'The "save" property must be specified and must be a valid function.' );
expect( block ).toBeUndefined();
} );

it( 'should reject blocks with an invalid edit function', () => {
const blockType = { save: noop, edit: 'not-a-function', category: 'common', title: 'block title' },
block = registerBlockType( 'my-plugin/fancy-block-6', blockType );
expect( console.error ).toHaveBeenCalledWith( 'The "edit" property must be a valid function.' );
expect( console ).toHaveErroredWith( 'The "edit" property must be a valid function.' );
expect( block ).toBeUndefined();
} );

it( 'should reject blocks with more than 3 keywords', () => {
const blockType = { save: noop, keywords: [ 'apple', 'orange', 'lemon', 'pineapple' ], category: 'common', title: 'block title' },
block = registerBlockType( 'my-plugin/fancy-block-7', blockType );
expect( console.error ).toHaveBeenCalledWith( 'The block "my-plugin/fancy-block-7" can have a maximum of 3 keywords.' );
expect( console ).toHaveErroredWith( 'The block "my-plugin/fancy-block-7" can have a maximum of 3 keywords.' );
expect( block ).toBeUndefined();
} );

it( 'should reject blocks without category', () => {
const blockType = { settingName: 'settingValue', save: noop, title: 'block title' },
block = registerBlockType( 'my-plugin/fancy-block-8', blockType );
expect( console.error ).toHaveBeenCalledWith( 'The block "my-plugin/fancy-block-8" must have a category.' );
expect( console ).toHaveErroredWith( 'The block "my-plugin/fancy-block-8" must have a category.' );
expect( block ).toBeUndefined();
} );

it( 'should reject blocks with non registered category.', () => {
const blockType = { save: noop, category: 'custom-category-slug', title: 'block title' },
block = registerBlockType( 'my-plugin/fancy-block-9', blockType );
expect( console.error ).toHaveBeenCalledWith( 'The block "my-plugin/fancy-block-9" must have a registered category.' );
expect( console ).toHaveErroredWith( 'The block "my-plugin/fancy-block-9" must have a registered category.' );
expect( block ).toBeUndefined();
} );

it( 'should reject blocks without title', () => {
const blockType = { settingName: 'settingValue', save: noop, category: 'common' },
block = registerBlockType( 'my-plugin/fancy-block-9', blockType );
expect( console.error ).toHaveBeenCalledWith( 'The block "my-plugin/fancy-block-9" must have a title.' );
expect( console ).toHaveErroredWith( 'The block "my-plugin/fancy-block-9" must have a title.' );
expect( block ).toBeUndefined();
} );

it( 'should reject blocks with empty titles', () => {
const blockType = { settingName: 'settingValue', save: noop, category: 'common', title: '' },
block = registerBlockType( 'my-plugin/fancy-block-10', blockType );
expect( console.error ).toHaveBeenCalledWith( 'The block "my-plugin/fancy-block-10" must have a title.' );
expect( console ).toHaveErroredWith( 'The block "my-plugin/fancy-block-10" must have a title.' );
expect( block ).toBeUndefined();
} );

it( 'should reject titles which are not strings', () => {
const blockType = { settingName: 'settingValue', save: noop, category: 'common', title: 12345 },
block = registerBlockType( 'my-plugin/fancy-block-11', blockType );
expect( console.error ).toHaveBeenCalledWith( 'Block titles must be strings.' );
expect( console ).toHaveErroredWith( 'Block titles must be strings.' );
expect( block ).toBeUndefined();
} );

Expand Down Expand Up @@ -214,12 +210,30 @@ describe( 'blocks', () => {
},
} );
} );

describe( 'applyFilters', () => {
afterEach( () => {
removeFilter( 'blocks.registerBlockType', 'core/blocks/without-title' );
} );

it( 'should reject valid blocks when they become invalid after executing filter', () => {
addFilter( 'blocks.registerBlockType', 'core/blocks/without-title', ( settings ) => {
return {
...settings,
title: '',
};
} );
const block = registerBlockType( 'my-plugin/fancy-block-12', defaultBlockSettings );
expect( console ).toHaveErroredWith( 'The block "my-plugin/fancy-block-12" must have a title.' );
expect( block ).toBeUndefined();
} );
} );
} );

describe( 'unregisterBlockType()', () => {
it( 'should fail if a block is not registered', () => {
const oldBlock = unregisterBlockType( 'core/test-block' );
expect( console.error ).toHaveBeenCalledWith( 'Block "core/test-block" is not registered.' );
expect( console ).toHaveErroredWith( 'Block "core/test-block" is not registered.' );
expect( oldBlock ).toBeUndefined();
} );

Expand All @@ -243,7 +257,7 @@ describe( 'blocks', () => {
},
] );
const oldBlock = unregisterBlockType( 'core/test-block' );
expect( console.error ).not.toHaveBeenCalled();
expect( console ).not.toHaveErrored();
expect( oldBlock ).toEqual( {
name: 'core/test-block',
save: noop,
Expand Down