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.

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

Block API: Normalize block type function argument #11490

Merged
merged 7 commits into from
Nov 9, 2018
3 changes: 1 addition & 2 deletions packages/block-library/src/heading/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {
createBlock,
getPhrasingContentSchema,
getBlockAttributes,
getBlockType,
} from '@wordpress/blocks';
import { RichText } from '@wordpress/editor';
import {
Expand Down Expand Up @@ -101,7 +100,7 @@ export const settings = {
transform( node ) {
return createBlock( 'core/heading', {
...getBlockAttributes(
getBlockType( 'core/heading' ),
'core/heading',
node.outerHTML
),
level: getLevelFromHeadingNodeName( node.nodeName ),
Expand Down
4 changes: 1 addition & 3 deletions packages/block-library/src/image/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { __ } from '@wordpress/i18n';
import {
createBlock,
getBlockAttributes,
getBlockType,
getPhrasingContentSchema,
} from '@wordpress/blocks';
import { RichText } from '@wordpress/editor';
Expand Down Expand Up @@ -133,8 +132,7 @@ export const settings = {
const anchorElement = node.querySelector( 'a' );
const linkDestination = anchorElement && anchorElement.href ? 'custom' : undefined;
const href = anchorElement && anchorElement.href ? anchorElement.href : undefined;
const blockType = getBlockType( 'core/image' );
const attributes = getBlockAttributes( blockType, node.outerHTML, { align, id, linkDestination, href } );
const attributes = getBlockAttributes( 'core/image', node.outerHTML, { align, id, linkDestination, href } );
return createBlock( 'core/image', attributes );
},
},
Expand Down
3 changes: 1 addition & 2 deletions packages/block-library/src/list/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
createBlock,
getPhrasingContentSchema,
getBlockAttributes,
getBlockType,
} from '@wordpress/blocks';
import {
BlockControls,
Expand Down Expand Up @@ -108,7 +107,7 @@ export const settings = {
transform( node ) {
return createBlock( 'core/list', {
...getBlockAttributes(
getBlockType( 'core/list' ),
'core/list',
node.outerHTML
),
ordered: node.nodeName === 'OL',
Expand Down
6 changes: 6 additions & 0 deletions packages/blocks/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
## 5.3.0 (Unreleased)

### New feature

- `getBlockAttributes`, `getBlockTransforms`, `getSaveContent`, `getSaveElement` and `isValidBlockContent` methods can now take also block's name as the first param ([#11490](https://github.com/WordPress/gutenberg/pull/11490)). Passing a block's type object continues to work as before.

## 5.2.0 (2018-11-09)

- Paste: Google Docs: fix nested formatting, sub, sup and del.
Expand Down
10 changes: 6 additions & 4 deletions packages/blocks/src/api/factory.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { createHooks, applyFilters } from '@wordpress/hooks';
* Internal dependencies
*/
import { getBlockType, getBlockTypes } from './registration';
import { normalizeBlockType } from './utils';

/**
* Returns a block object given its type and attributes.
Expand Down Expand Up @@ -279,21 +280,22 @@ export function findTransform( transforms, predicate ) {
* transform object includes `blockName` as a property.
*
* @param {string} direction Transform direction ("to", "from").
* @param {?string} blockName Optional block name.
* @param {string|Object} blockTypeOrName Block type or name.
*
* @return {Array} Block transforms for direction.
*/
export function getBlockTransforms( direction, blockName ) {
export function getBlockTransforms( direction, blockTypeOrName ) {
// When retrieving transforms for all block types, recurse into self.
if ( blockName === undefined ) {
if ( blockTypeOrName === undefined ) {
return flatMap(
getBlockTypes(),
( { name } ) => getBlockTransforms( direction, name )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be partial( getBlockTransforms, direction ). I dunno that I'd care to change it though (given partial's questionable future)

);
}

// Validate that block type exists and has array of direction.
const { transforms } = getBlockType( blockName ) || {};
const blockType = normalizeBlockType( blockTypeOrName );
const { name: blockName, transforms } = blockType || {};
if ( ! transforms || ! Array.isArray( transforms[ direction ] ) ) {
return [];
}
Expand Down
10 changes: 6 additions & 4 deletions packages/blocks/src/api/parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { createBlock } from './factory';
import { isValidBlockContent } from './validation';
import { getCommentDelimitedContent } from './serializer';
import { attr, html, text, query, node, children, prop } from './matchers';
import { normalizeBlockType } from './utils';

/**
* Sources which are guaranteed to return a string value.
Expand Down Expand Up @@ -277,13 +278,14 @@ export function getBlockAttribute( attributeKey, attributeSchema, innerHTML, com
/**
* Returns the block attributes of a registered block node given its type.
*
* @param {?Object} blockType Block type.
* @param {string} innerHTML Raw block content.
* @param {?Object} attributes Known block attributes (from delimiters).
* @param {string|Object} blockTypeOrName Block type or name.
* @param {string} innerHTML Raw block content.
* @param {?Object} attributes Known block attributes (from delimiters).
*
* @return {Object} All block attributes.
*/
export function getBlockAttributes( blockType, innerHTML, attributes = {} ) {
export function getBlockAttributes( blockTypeOrName, innerHTML, attributes = {} ) {
const blockType = normalizeBlockType( blockTypeOrName );
const blockAttributes = mapValues( blockType.attributes, ( attributeSchema, attributeKey ) => {
return getBlockAttribute( attributeKey, attributeSchema, innerHTML, attributes );
} );
Expand Down
3 changes: 1 addition & 2 deletions packages/blocks/src/api/raw-handling/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { flatMap, filter, compact } from 'lodash';
* Internal dependencies
*/
import { createBlock, getBlockTransforms, findTransform } from '../factory';
import { getBlockType } from '../registration';
import { getBlockContent } from '../serializer';
import { getBlockAttributes, parseWithGrammar } from '../parser';
import normaliseBlocks from './normalise-blocks';
Expand Down Expand Up @@ -103,7 +102,7 @@ function htmlToBlocks( { html, rawTransforms } ) {
return createBlock(
blockName,
getBlockAttributes(
getBlockType( blockName ),
blockName,
node.outerHTML
)
);
Expand Down
23 changes: 13 additions & 10 deletions packages/blocks/src/api/serializer.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
getFreeformContentHandlerName,
getUnregisteredTypeHandlerName,
} from './registration';
import { normalizeBlockType } from './utils';
import BlockContentProvider from '../block-content-provider';

/**
Expand Down Expand Up @@ -54,13 +55,14 @@ export function getBlockMenuDefaultClassName( blockName ) {
* Given a block type containing a save render implementation and attributes, returns the
* enhanced element to be saved or string when raw HTML expected.
*
* @param {Object} blockType Block type.
* @param {Object} attributes Block attributes.
* @param {?Array} innerBlocks Nested blocks.
* @param {string|Object} blockTypeOrName Block type or name.
* @param {Object} attributes Block attributes.
* @param {?Array} innerBlocks Nested blocks.
*
* @return {Object|string} Save element or raw HTML string.
*/
export function getSaveElement( blockType, attributes, innerBlocks = [] ) {
export function getSaveElement( blockTypeOrName, attributes, innerBlocks = [] ) {
const blockType = normalizeBlockType( blockTypeOrName );
let { save } = blockType;

// Component classes are unsupported for save since serialization must
Expand Down Expand Up @@ -113,13 +115,15 @@ export function getSaveElement( blockType, attributes, innerBlocks = [] ) {
* Given a block type containing a save render implementation and attributes, returns the
* static markup to be saved.
*
* @param {Object} blockType Block type.
* @param {Object} attributes Block attributes.
* @param {?Array} innerBlocks Nested blocks.
* @param {string|Object} blockTypeOrName Block type or name.
* @param {Object} attributes Block attributes.
* @param {?Array} innerBlocks Nested blocks.
*
* @return {string} Save content.
*/
export function getSaveContent( blockType, attributes, innerBlocks ) {
export function getSaveContent( blockTypeOrName, attributes, innerBlocks ) {
const blockType = normalizeBlockType( blockTypeOrName );

return renderToString( getSaveElement( blockType, attributes, innerBlocks ) );
}

Expand Down Expand Up @@ -199,7 +203,6 @@ export function serializeAttributes( attributes ) {
*/
export function getBlockContent( block ) {
// @todo why not getBlockInnerHtml?
const blockType = getBlockType( block.name );

// If block was parsed as invalid or encounters an error while generating
// save content, use original content instead to avoid content loss. If a
Expand All @@ -209,7 +212,7 @@ export function getBlockContent( block ) {
let saveContent = block.originalContent;
if ( block.isValid || block.innerBlocks.length ) {
try {
saveContent = getSaveContent( blockType, block.attributes, block.innerBlocks );
saveContent = getSaveContent( block.name, block.attributes, block.innerBlocks );
} catch ( error ) {}
}

Expand Down
21 changes: 20 additions & 1 deletion packages/blocks/src/api/test/factory.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,12 @@ import {
getBlockTransforms,
findTransform,
} from '../factory';
import { getBlockTypes, unregisterBlockType, registerBlockType } from '../registration';
import {
getBlockType,
getBlockTypes,
registerBlockType,
unregisterBlockType,
} from '../registration';

describe( 'block factory', () => {
const defaultBlockSettings = {
Expand Down Expand Up @@ -1181,6 +1186,20 @@ describe( 'block factory', () => {
},
] );
} );

it( 'should return single block type transforms when passed as an object', () => {
const transforms = getBlockTransforms(
'from',
getBlockType( 'core/transform-from-text-block-1' )
);

expect( transforms ).toEqual( [
{
blocks: [ 'core/text-block' ],
blockName: 'core/transform-from-text-block-1',
},
] );
} );
} );

describe( 'findTransform', () => {
Expand Down
20 changes: 20 additions & 0 deletions packages/blocks/src/api/test/parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,26 @@ describe( 'block parser', () => {
undefAmbiguousStringWithDefault: 'ok',
} );
} );

it( 'should work when block type is passed as string', () => {
registerBlockType( 'core/meal', {
title: 'Meal',
category: 'widgets',
attributes: {
content: {
source: 'text',
selector: 'div',
},
},
save: () => {},
} );

const innerHTML = '<div data-number="10">Ribs</div>';

expect( getBlockAttributes( 'core/meal', innerHTML ) ).toEqual( {
content: 'Ribs',
} );
} );
} );

describe( 'getMigratedBlock', () => {
Expand Down
19 changes: 18 additions & 1 deletion packages/blocks/src/api/test/serializer.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,17 +39,34 @@ describe( 'block serializer', () => {

describe( 'getSaveContent()', () => {
describe( 'function save', () => {
const fruitBlockSave = ( { attributes } ) => createElement( 'div', null, attributes.fruit );

it( 'should return element as string if save returns element', () => {
const saved = getSaveContent(
{
save: ( { attributes } ) => createElement( 'div', null, attributes.fruit ),
name: 'core/fruit',
save: fruitBlockSave,
},
{ fruit: 'Bananas' }
);

expect( saved ).toBe( '<div>Bananas</div>' );
} );

it( 'should work when block type is passed as string', () => {
registerBlockType( 'core/fruit', {
title: 'Fruit',
category: 'widgets',
save: fruitBlockSave,
} );

const saved = getSaveContent(
'core/fruit',
{ fruit: 'Bananas' }
);

expect( saved ).toBe( '<div>Bananas</div>' );
} );
} );

describe( 'component save', () => {
Expand Down
16 changes: 14 additions & 2 deletions packages/blocks/src/api/test/validation.js
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ describe( 'validation', () => {
registerBlockType( 'core/test-block', defaultBlockSettings );

const isValid = isValidBlockContent(
getBlockType( 'core/test-block' ),
'core/test-block',
{ fruit: 'Bananas' },
'Apples'
);
Expand All @@ -577,7 +577,7 @@ describe( 'validation', () => {
} );

const isValid = isValidBlockContent(
getBlockType( 'core/test-block' ),
'core/test-block',
{ fruit: 'Bananas' },
'Bananas'
);
Expand All @@ -589,6 +589,18 @@ describe( 'validation', () => {
it( 'returns true is block is valid', () => {
registerBlockType( 'core/test-block', defaultBlockSettings );

const isValid = isValidBlockContent(
'core/test-block',
{ fruit: 'Bananas' },
'Bananas'
);

expect( isValid ).toBe( true );
} );

it( 'works also when block type object is passed as object', () => {
registerBlockType( 'core/test-block', defaultBlockSettings );

const isValid = isValidBlockContent(
getBlockType( 'core/test-block' ),
{ fruit: 'Bananas' },
Expand Down
19 changes: 18 additions & 1 deletion packages/blocks/src/api/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { Component, isValidElement } from '@wordpress/element';
/**
* Internal dependencies
*/
import { getDefaultBlockName } from './registration';
import { getBlockType, getDefaultBlockName } from './registration';
import { createBlock } from './factory';

/**
Expand Down Expand Up @@ -104,3 +104,20 @@ export function normalizeIconObject( icon ) {

return icon;
}

/**
* Normalizes block type passed as param. When string is passed then
* it converts it to the matching block type object.
* It passes the original object otherwise.
*
* @param {string|Object} blockTypeOrName Block type or name.
*
* @return {?Object} Block type.
*/
export function normalizeBlockType( blockTypeOrName ) {
if ( isString( blockTypeOrName ) ) {
return getBlockType( blockTypeOrName );
}

return blockTypeOrName;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A general question (not a blocker!) about our public APIs: Should we be validating their inputs more strictly and e.g. throwing an error if a number or boolean is passed here?

My fear is that we may lose some flexibility to change APIs in the future if there are popular plugins out there that are using our APIs incorrectly.

Copy link
Member Author

@gziolo gziolo Nov 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be validating their inputs more strictly and e.g. throwing an error if a number or boolean is passed here?

As bad as it sounds, wouldn't it be a breaking change? :)

In overall, this is something we should pay more attention to in the 2nd phase of the project.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My 2¢ : While laudable, as a general pattern it's hard to anticipate all nonsense inputs, and in setting expectations of tolerance while being unable to deliver (or deliver consistently), we're doing more a disservice than we're helping.

}
10 changes: 6 additions & 4 deletions packages/blocks/src/api/validation.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import deprecated from '@wordpress/deprecated';
* Internal dependencies
*/
import { getSaveContent } from './serializer';
import { normalizeBlockType } from './utils';

/**
* Globally matches any consecutive whitespace
Expand Down Expand Up @@ -508,13 +509,14 @@ export function isValidBlock( innerHTML, blockType, attributes ) {
*
* Logs to console in development environments when invalid.
*
* @param {string} blockType Block type.
* @param {Object} attributes Parsed block attributes.
* @param {string} innerHTML Original block content.
* @param {string|Object} blockTypeOrName Block type.
* @param {Object} attributes Parsed block attributes.
* @param {string} innerHTML Original block content.
*
* @return {boolean} Whether block is valid.
*/
export function isValidBlockContent( blockType, attributes, innerHTML ) {
export function isValidBlockContent( blockTypeOrName, attributes, innerHTML ) {
const blockType = normalizeBlockType( blockTypeOrName );
let saveContent;
try {
saveContent = getSaveContent( blockType, attributes );
Expand Down
Loading