Skip to content

Commit

Permalink
Improve the performance of the parser by removing the automatic custo…
Browse files Browse the repository at this point in the history
…m classnames handling. (#33903)
  • Loading branch information
youknowriad authored Aug 10, 2021
1 parent 0be70f3 commit 984160d
Show file tree
Hide file tree
Showing 31 changed files with 2,198 additions and 2,080 deletions.
69 changes: 1 addition & 68 deletions packages/block-editor/src/hooks/custom-class-name.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
/**
* External dependencies
*/
import { difference, omit } from 'lodash';
import classnames from 'classnames';

/**
Expand All @@ -10,11 +9,7 @@ import classnames from 'classnames';
import { addFilter } from '@wordpress/hooks';
import { TextControl } from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import {
hasBlockSupport,
parseWithAttributeSchema,
getSaveContent,
} from '@wordpress/blocks';
import { hasBlockSupport } from '@wordpress/blocks';
import { createHigherOrderComponent } from '@wordpress/compose';

/**
Expand Down Expand Up @@ -117,63 +112,6 @@ export function addSaveProps( extraProps, blockType, attributes ) {
return extraProps;
}

/**
* Given an HTML string, returns an array of class names assigned to the root
* element in the markup.
*
* @param {string} innerHTML Markup string from which to extract classes.
*
* @return {string[]} Array of class names assigned to the root element.
*/
export function getHTMLRootElementClasses( innerHTML ) {
innerHTML = `<div data-custom-class-name>${ innerHTML }</div>`;

const parsed = parseWithAttributeSchema( innerHTML, {
type: 'string',
source: 'attribute',
selector: '[data-custom-class-name] > *',
attribute: 'class',
} );

return parsed ? parsed.trim().split( /\s+/ ) : [];
}

/**
* Given a parsed set of block attributes, if the block supports custom class
* names and an unknown class (per the block's serialization behavior) is
* found, the unknown classes are treated as custom classes. This prevents the
* block from being considered as invalid.
*
* @param {Object} blockAttributes Original block attributes.
* @param {Object} blockType Block type settings.
* @param {string} innerHTML Original block markup.
*
* @return {Object} Filtered block attributes.
*/
export function addParsedDifference( blockAttributes, blockType, innerHTML ) {
if ( hasBlockSupport( blockType, 'customClassName', true ) ) {
// To determine difference, serialize block given the known set of
// attributes, with the exception of `className`. This will determine
// the default set of classes. From there, any difference in innerHTML
// can be considered as custom classes.
const attributesSansClassName = omit( blockAttributes, [
'className',
] );
const serialized = getSaveContent( blockType, attributesSansClassName );
const defaultClasses = getHTMLRootElementClasses( serialized );
const actualClasses = getHTMLRootElementClasses( innerHTML );
const customClasses = difference( actualClasses, defaultClasses );

if ( customClasses.length ) {
blockAttributes.className = customClasses.join( ' ' );
} else if ( serialized ) {
delete blockAttributes.className;
}
}

return blockAttributes;
}

addFilter(
'blocks.registerBlockType',
'core/custom-class-name/attribute',
Expand All @@ -189,8 +127,3 @@ addFilter(
'core/custom-class-name/save-props',
addSaveProps
);
addFilter(
'blocks.getBlockAttributes',
'core/custom-class-name/addParsedDifference',
addParsedDifference
);
70 changes: 1 addition & 69 deletions packages/block-editor/src/hooks/custom-class-name.native.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,13 @@
/**
* External dependencies
*/
import { difference, compact } from 'lodash';
import classnames from 'classnames';

/**
* WordPress dependencies
*/
import { addFilter } from '@wordpress/hooks';
import {
hasBlockSupport,
getSaveContent,
parseWithAttributeSchema,
} from '@wordpress/blocks';
import { hasBlockSupport } from '@wordpress/blocks';

/**
* Filters registered block settings, extending attributes with anchor using ID
Expand Down Expand Up @@ -61,64 +56,6 @@ export function addSaveProps( extraProps, blockType, attributes ) {
return extraProps;
}

/**
* Given an HTML string, returns an array of class names assigned to the root
* element in the markup.
*
* @param {string} innerHTML Markup string from which to extract classes.
*
* @return {string[]} Array of class names assigned to the root element.
*/
export function getHTMLRootElementClasses( innerHTML ) {
innerHTML = `<div data-custom-class-name>${ innerHTML }</div>`;

const parsed = parseWithAttributeSchema( innerHTML, {
type: 'string',
source: 'attribute',
selector: '[data-custom-class-name] > *',
attribute: 'class',
} );

return parsed ? parsed.trim().split( /\s+/ ) : [];
}

/**
* Given a parsed set of block attributes, if the block supports custom class
* names and an unknown class (per the block's serialization behavior) is
* found, the unknown classes are treated as custom classes. This prevents the
* block from being considered as invalid.
*
* @param {Object} blockAttributes Original block attributes.
* @param {Object} blockType Block type settings.
* @param {string} innerHTML Original block markup.
*
* @return {Object} Filtered block attributes.
*/
export function addParsedDifference( blockAttributes, blockType, innerHTML ) {
if ( hasBlockSupport( blockType, 'customClassName', true ) ) {
// To determine difference, serialize block given the known set of
// attributes. If there are classes which are mismatched with the
// incoming HTML of the block, add to filtered result.
const serialized = getSaveContent( blockType, blockAttributes );
const classes = getHTMLRootElementClasses( serialized );
const parsedClasses = getHTMLRootElementClasses( innerHTML );
const customClasses = difference( parsedClasses, classes );

const filteredClassName = compact( [
blockAttributes.className,
...customClasses,
] ).join( ' ' );

if ( filteredClassName ) {
blockAttributes.className = filteredClassName;
} else {
delete blockAttributes.className;
}
}

return blockAttributes;
}

addFilter(
'blocks.registerBlockType',
'core/custom-class-name/attribute',
Expand All @@ -129,8 +66,3 @@ addFilter(
'core/custom-class-name/save-props',
addSaveProps
);
addFilter(
'blocks.getBlockAttributes',
'core/custom-class-name/addParsedDifference',
addParsedDifference
);
116 changes: 1 addition & 115 deletions packages/block-editor/src/hooks/test/custom-class-name.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { applyFilters } from '@wordpress/hooks';
/**
* Internal dependencies
*/
import { getHTMLRootElementClasses } from '../custom-class-name';
import '../custom-class-name';

describe( 'custom className', () => {
const blockSettings = {
Expand All @@ -15,12 +15,6 @@ describe( 'custom className', () => {
title: 'block title',
};

const dynamicBlockSettings = {
save: () => null,
category: 'text',
title: 'block title',
};

describe( 'addAttribute()', () => {
const addAttribute = applyFilters.bind(
null,
Expand Down Expand Up @@ -78,112 +72,4 @@ describe( 'custom className', () => {
expect( extraProps.className ).toBe( 'foo bar' );
} );
} );

describe( 'getHTMLRootElementClasses', () => {
it( 'should return an empty array if there are no classes', () => {
const classes = getHTMLRootElementClasses( '<div></div>' );

expect( classes ).toEqual( [] );
} );

it( 'return an array of parsed classes from inner HTML', () => {
const classes = getHTMLRootElementClasses(
'<div class=" foo bar "></div>'
);

expect( classes ).toEqual( [ 'foo', 'bar' ] );
} );
} );

describe( 'addParsedDifference', () => {
const addParsedDifference = applyFilters.bind(
null,
'blocks.getBlockAttributes'
);

it( 'should do nothing if the block settings do not define custom className support', () => {
const attributes = addParsedDifference(
{ className: 'foo' },
{
...blockSettings,
supports: {
customClassName: false,
},
},
'<div class="bar baz"></div>'
);

expect( attributes.className ).toBe( 'foo' );
} );

it( 'should inject the className differences from parsed attributes', () => {
const attributes = addParsedDifference(
{ className: 'foo' },
blockSettings,
'<div class="default foo bar baz"></div>'
);

expect( attributes.className ).toBe( 'foo bar baz' );
} );

it( 'should assign as undefined if there are no classes', () => {
const attributes = addParsedDifference(
{},
blockSettings,
'<div class=""></div>'
);

expect( attributes.className ).toBeUndefined();
} );

it( 'should add a custom class name to an element without a class', () => {
const attributes = addParsedDifference(
{},
blockSettings,
'<div class="default foo"></div>'
);

expect( attributes.className ).toBe( 'foo' );
} );

it( 'should remove the custom class and retain default class', () => {
const attributes = addParsedDifference(
{ className: 'custom1 custom2' },
blockSettings,
'<div class="default custom1"></div>'
);

expect( attributes.className ).toBe( 'custom1' );
} );

it( 'should remove the custom class from an element originally without a class', () => {
const attributes = addParsedDifference(
{ className: 'foo' },
blockSettings,
'<div></div>'
);

expect( attributes.className ).toBeUndefined();
} );

it( 'should remove the custom classes and retain default and other custom class', () => {
const attributes = addParsedDifference(
{ className: 'custom1 custom2 custom3' },
blockSettings,
'<div class="default custom1 custom3"></div>'
);

expect( attributes.className ).toBe( 'custom1 custom3' );
} );

it( 'should not remove the custom classes for dynamic blocks', () => {
const attributes = addParsedDifference(
{ className: 'custom1' },
dynamicBlockSettings,
null
);

expect( attributes.className ).toBe( 'custom1' );
} );
} );
} );
4 changes: 2 additions & 2 deletions packages/blocks/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -711,7 +711,7 @@ _Parameters_

_Returns_

- `?WPBlock`: The block, if it has been successfully registered; otherwise `undefined`.
- `?WPBlockType`: The block, if it has been successfully registered; otherwise `undefined`.

### registerBlockVariation

Expand Down Expand Up @@ -837,7 +837,7 @@ _Parameters_

_Returns_

- `?WPBlock`: The previous block value, if it has been successfully unregistered; otherwise `undefined`.
- `?WPBlockType`: The previous block value, if it has been successfully unregistered; otherwise `undefined`.

### unregisterBlockVariation

Expand Down
4 changes: 2 additions & 2 deletions packages/blocks/src/api/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ export {
// block. For composition, it also means inner blocks can effectively be child
// components whose mechanisms can be shielded from the `edit` implementation
// and just passed along.
export { default as parse } from './parser';
export {
default as parse,
getBlockAttributes,
parseWithAttributeSchema,
} from './parser';
} from './parser/get-block-attributes';

// While block transformations account for a specific surface of the API, there
// are also raw transformations which handle arbitrary sources not made out of
Expand Down
Loading

0 comments on commit 984160d

Please sign in to comment.