From 1443f453bf2b4753db3027c23a8b0e9adf35dc06 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= <iseulde@automattic.com>
Date: Wed, 3 Apr 2019 17:56:47 +0200
Subject: [PATCH] Annotations & Performance: remove use of Memize (#14664)
---
packages/annotations/package.json | 1 -
packages/annotations/src/format/annotation.js | 55 ++-----
.../src/components/rich-text/index.js | 78 +++++-----
packages/rich-text/package.json | 1 +
.../rich-text/src/register-format-type.js | 134 +++++++-----------
packages/rich-text/src/to-dom.js | 18 ++-
.../rich-text/src/unregister-format-type.js | 5 +-
7 files changed, 104 insertions(+), 188 deletions(-)
diff --git a/packages/annotations/package.json b/packages/annotations/package.json
index 47ab161e05723c..c9e744157c65bd 100644
--- a/packages/annotations/package.json
+++ b/packages/annotations/package.json
@@ -27,7 +27,6 @@
"@wordpress/i18n": "file:../i18n",
"@wordpress/rich-text": "file:../rich-text",
"lodash": "^4.17.11",
- "memize": "^1.0.5",
"rememo": "^3.0.0",
"uuid": "^3.3.2"
},
diff --git a/packages/annotations/src/format/annotation.js b/packages/annotations/src/format/annotation.js
index e72786bad0e3bc..a2f6f2973c7268 100644
--- a/packages/annotations/src/format/annotation.js
+++ b/packages/annotations/src/format/annotation.js
@@ -1,8 +1,3 @@
-/**
- * External dependencies
- */
-import memize from 'memize';
-
/**
* WordPress dependencies
*/
@@ -120,40 +115,6 @@ function updateAnnotationsWithPositions( annotations, positions, { removeAnnotat
} );
}
-/**
- * Create prepareEditableTree memoized based on the annotation props.
- *
- * @param {Object} The props with annotations in them.
- *
- * @return {Function} The prepareEditableTree.
- */
-const createPrepareEditableTree = memize( ( props ) => {
- const { annotations } = props;
-
- return ( formats, text ) => {
- if ( annotations.length === 0 ) {
- return formats;
- }
-
- let record = { formats, text };
- record = applyAnnotations( record, annotations );
- return record.formats;
- };
-} );
-
-/**
- * Returns the annotations as a props object. Memoized to prevent re-renders.
- *
- * @param {Array} The annotations to put in the object.
- *
- * @return {Object} The annotations props object.
- */
-const getAnnotationObject = memize( ( annotations ) => {
- return {
- annotations,
- };
-} );
-
export const annotation = {
name: FORMAT_NAME,
title: __( 'Annotation' ),
@@ -167,9 +128,21 @@ export const annotation = {
return null;
},
__experimentalGetPropsForEditableTreePreparation( select, { richTextIdentifier, blockClientId } ) {
- return getAnnotationObject( select( STORE_KEY ).__experimentalGetAnnotationsForRichText( blockClientId, richTextIdentifier ) );
+ return {
+ annotations: select( STORE_KEY ).__experimentalGetAnnotationsForRichText( blockClientId, richTextIdentifier ),
+ };
+ },
+ __experimentalCreatePrepareEditableTree( { annotations } ) {
+ return ( formats, text ) => {
+ if ( annotations.length === 0 ) {
+ return formats;
+ }
+
+ let record = { formats, text };
+ record = applyAnnotations( record, annotations );
+ return record.formats;
+ };
},
- __experimentalCreatePrepareEditableTree: createPrepareEditableTree,
__experimentalGetPropsForEditableTreeChangeHandler( dispatch ) {
return {
removeAnnotation: dispatch( STORE_KEY ).__experimentalRemoveAnnotation,
diff --git a/packages/block-editor/src/components/rich-text/index.js b/packages/block-editor/src/components/rich-text/index.js
index ed94653f98be34..1227dd23d4b540 100644
--- a/packages/block-editor/src/components/rich-text/index.js
+++ b/packages/block-editor/src/components/rich-text/index.js
@@ -8,8 +8,6 @@ import {
isEqual,
omit,
pickBy,
- get,
- isPlainObject,
} from 'lodash';
import memize from 'memize';
@@ -48,6 +46,7 @@ import {
import { decodeEntities } from '@wordpress/html-entities';
import { withFilters, IsolatedEventContainer } from '@wordpress/components';
import deprecated from '@wordpress/deprecated';
+import isShallowEqual from '@wordpress/is-shallow-equal';
/**
* Internal dependencies
@@ -91,6 +90,20 @@ const globalStyle = document.createElement( 'style' );
document.head.appendChild( globalStyle );
+function createPrepareEditableTree( props ) {
+ const fns = Object.keys( props ).reduce( ( accumulator, key ) => {
+ if ( key.startsWith( 'format_prepare_functions' ) ) {
+ accumulator.push( props[ key ] );
+ }
+
+ return accumulator;
+ }, [] );
+
+ return ( value ) => fns.reduce( ( accumulator, fn ) => {
+ return fn( accumulator, value.text );
+ }, value.formats );
+}
+
export class RichText extends Component {
constructor( { value, onReplace, multiline } ) {
super( ...arguments );
@@ -202,7 +215,7 @@ export class RichText extends Component {
range,
multilineTag: this.multilineTag,
multilineWrapperTags: this.multilineWrapperTags,
- prepareEditableTree: this.props.prepareEditableTree,
+ prepareEditableTree: createPrepareEditableTree( this.props ),
__unstableIsEditableTree: true,
} );
}
@@ -213,7 +226,7 @@ export class RichText extends Component {
current: this.editableRef,
multilineTag: this.multilineTag,
multilineWrapperTags: this.multilineWrapperTags,
- prepareEditableTree: this.props.prepareEditableTree,
+ prepareEditableTree: createPrepareEditableTree( this.props ),
__unstableDomOnly: domOnly,
} );
}
@@ -484,18 +497,6 @@ export class RichText extends Component {
}
}
- /**
- * Calls all registered onChangeEditableValue handlers.
- *
- * @param {Array} formats The formats of the latest rich-text value.
- * @param {string} text The text of the latest rich-text value.
- */
- onChangeEditableValue( { formats, text } ) {
- get( this.props, [ 'onChangeEditableValue' ], [] ).forEach( ( eventHandler ) => {
- eventHandler( formats, text );
- } );
- }
-
/**
* Sync the value to global state. The node tree and selection will also be
* updated if differences are found.
@@ -509,8 +510,14 @@ export class RichText extends Component {
this.applyRecord( record );
const { start, end, activeFormats = [] } = record;
+ const changeHandlers = pickBy( this.props, ( v, key ) =>
+ key.startsWith( 'format_on_change_functions_' )
+ );
+
+ Object.values( changeHandlers ).forEach( ( changeHandler ) => {
+ changeHandler( record.formats, record.text );
+ } );
- this.onChangeEditableValue( record );
this.savedContent = this.valueToFormat( record );
this.props.onChange( this.savedContent );
this.setState( { start, end, activeFormats } );
@@ -912,23 +919,13 @@ export class RichText extends Component {
this.savedContent = value;
}
- // If any format props update, reapply value.
- const shouldReapply = Object.keys( this.props ).some( ( name ) => {
- if ( name.indexOf( 'format_' ) !== 0 ) {
- return false;
- }
-
- // Allow primitives and arrays:
- if ( ! isPlainObject( this.props[ name ] ) ) {
- return this.props[ name ] !== prevProps[ name ];
- }
-
- return Object.keys( this.props[ name ] ).some( ( subName ) => {
- return this.props[ name ][ subName ] !== prevProps[ name ][ subName ];
- } );
- } );
+ const prefix = 'format_prepare_props_';
+ const predicate = ( v, key ) => key.startsWith( prefix );
+ const prepareProps = pickBy( this.props, predicate );
+ const prevPrepareProps = pickBy( prevProps, predicate );
- if ( shouldReapply ) {
+ // If any format prepare props update, reapply value.
+ if ( ! isShallowEqual( prepareProps, prevPrepareProps ) ) {
const record = this.formatToValue( value );
// Maintain the previous selection if the instance is currently
@@ -942,15 +939,6 @@ export class RichText extends Component {
}
}
- /**
- * Get props that are provided by formats to modify RichText.
- *
- * @return {Object} Props that start with 'format_'.
- */
- getFormatProps() {
- return pickBy( this.props, ( propValue, name ) => name.startsWith( 'format_' ) );
- }
-
/**
* Converts the outside data structure to our internal representation.
*
@@ -988,7 +976,7 @@ export class RichText extends Component {
return unstableToDom( {
value,
multilineTag: this.multilineTag,
- prepareEditableTree: this.props.prepareEditableTree,
+ prepareEditableTree: createPrepareEditableTree( this.props ),
} ).body.innerHTML;
}
@@ -1066,9 +1054,7 @@ export class RichText extends Component {
const record = this.getRecord();
return (
- <div className={ classes }
- onFocus={ this.setFocusedElement }
- >
+ <div className={ classes } onFocus={ this.setFocusedElement }>
{ isSelected && this.multilineTag === 'li' && (
<ListEdit
onTagNameChange={ onTagNameChange }
diff --git a/packages/rich-text/package.json b/packages/rich-text/package.json
index 096b3faa12499a..b2a0f0165b40dc 100644
--- a/packages/rich-text/package.json
+++ b/packages/rich-text/package.json
@@ -25,6 +25,7 @@
"@wordpress/compose": "file:../compose",
"@wordpress/data": "file:../data",
"@wordpress/escape-html": "file:../escape-html",
+ "@wordpress/hooks": "file:../hooks",
"lodash": "^4.17.11",
"rememo": "^3.0.0"
},
diff --git a/packages/rich-text/src/register-format-type.js b/packages/rich-text/src/register-format-type.js
index 8bee07ff8c776a..eb6d48c9e56368 100644
--- a/packages/rich-text/src/register-format-type.js
+++ b/packages/rich-text/src/register-format-type.js
@@ -2,7 +2,6 @@
* External dependencies
*/
import { mapKeys } from 'lodash';
-import memize from 'memize';
/**
* WordPress dependencies
@@ -11,17 +10,6 @@ import { select, dispatch, withSelect, withDispatch } from '@wordpress/data';
import { addFilter } from '@wordpress/hooks';
import { compose } from '@wordpress/compose';
-/**
- * Shared reference to an empty array for cases where it is important to avoid
- * returning a new array reference on every invocation, as in a connected or
- * other pure component which performs `shouldComponentUpdate` check on props.
- * This should be used as a last resort, since the normalized data should be
- * maintained by the reducer result in state.
- *
- * @type {Array}
- */
-const EMPTY_ARRAY = [];
-
/**
* Registers a new format provided a unique name and an object defining its
* behavior.
@@ -135,99 +123,73 @@ export function registerFormatType( name, settings ) {
dispatch( 'core/rich-text' ).addFormatTypes( settings );
- const getFunctionStackMemoized = memize( ( previousStack = EMPTY_ARRAY, newFunction ) => {
- return [
- ...previousStack,
- newFunction,
- ];
- } );
-
- if (
- settings.__experimentalCreatePrepareEditableTree
- ) {
+ if ( settings.__experimentalCreatePrepareEditableTree ) {
addFilter( 'experimentalRichText', name, ( OriginalComponent ) => {
- let Component = OriginalComponent;
- if (
- settings.__experimentalCreatePrepareEditableTree ||
- settings.__experimentalCreateFormatToValue ||
- settings.__experimentalCreateValueToFormat
- ) {
- Component = ( props ) => {
- const additionalProps = {};
-
- if ( settings.__experimentalCreatePrepareEditableTree ) {
- additionalProps.prepareEditableTree = getFunctionStackMemoized(
- props.prepareEditableTree,
- settings.__experimentalCreatePrepareEditableTree( props[ `format_${ name }` ], {
- richTextIdentifier: props.identifier,
- blockClientId: props.clientId,
- } )
- );
+ const selectPrefix = `format_prepare_props_(${ name })_`;
+ const dispatchPrefix = `format_on_change_props_(${ name })_`;
+
+ const Component = ( props ) => {
+ const newProps = { ...props };
+ const propsByPrefix = Object.keys( props ).reduce( ( accumulator, key ) => {
+ if ( key.startsWith( selectPrefix ) ) {
+ accumulator[ key.slice( selectPrefix.length ) ] = props[ key ];
}
- if ( settings.__experimentalCreateOnChangeEditableValue ) {
- const dispatchProps = Object.keys( props ).reduce( ( accumulator, propKey ) => {
- const propValue = props[ propKey ];
- const keyPrefix = `format_${ name }_dispatch_`;
- if ( propKey.startsWith( keyPrefix ) ) {
- const realKey = propKey.replace( keyPrefix, '' );
-
- accumulator[ realKey ] = propValue;
- }
-
- return accumulator;
- }, {} );
-
- additionalProps.onChangeEditableValue = getFunctionStackMemoized(
- props.onChangeEditableValue,
- settings.__experimentalCreateOnChangeEditableValue( {
- ...props[ `format_${ name }` ],
- ...dispatchProps,
- }, {
- richTextIdentifier: props.identifier,
- blockClientId: props.clientId,
- } )
- );
+ if ( key.startsWith( dispatchPrefix ) ) {
+ accumulator[ key.slice( dispatchPrefix.length ) ] = props[ key ];
}
- return <OriginalComponent
- { ...props }
- { ...additionalProps }
- />;
+ return accumulator;
+ }, {} );
+ const args = {
+ richTextIdentifier: props.identifier,
+ blockClientId: props.clientId,
};
- }
+
+ newProps[ `format_prepare_functions_(${ name })` ] =
+ settings.__experimentalCreatePrepareEditableTree(
+ propsByPrefix,
+ args
+ );
+
+ if ( settings.__experimentalCreateOnChangeEditableValue ) {
+ newProps[ `format_on_change_functions_(${ name })` ] =
+ settings.__experimentalCreateOnChangeEditableValue(
+ propsByPrefix,
+ args
+ );
+ }
+
+ return <OriginalComponent { ...newProps } />;
+ };
const hocs = [];
if ( settings.__experimentalGetPropsForEditableTreePreparation ) {
- hocs.push( withSelect( ( sel, { clientId, identifier } ) => ( {
- [ `format_${ name }` ]: settings.__experimentalGetPropsForEditableTreePreparation(
- sel,
- {
+ hocs.push( withSelect( ( sel, { clientId, identifier } ) =>
+ mapKeys(
+ settings.__experimentalGetPropsForEditableTreePreparation( sel, {
richTextIdentifier: identifier,
blockClientId: clientId,
- }
- ),
- } ) ) );
+ } ),
+ ( value, key ) => selectPrefix + key
+ )
+ ) );
}
if ( settings.__experimentalGetPropsForEditableTreeChangeHandler ) {
- hocs.push( withDispatch( ( disp, { clientId, identifier } ) => {
- const dispatchProps = settings.__experimentalGetPropsForEditableTreeChangeHandler(
- disp,
- {
+ hocs.push( withDispatch( ( disp, { clientId, identifier } ) =>
+ mapKeys(
+ settings.__experimentalGetPropsForEditableTreeChangeHandler( disp, {
richTextIdentifier: identifier,
blockClientId: clientId,
- }
- );
-
- return mapKeys( dispatchProps, ( value, key ) => {
- return `format_${ name }_dispatch_${ key }`;
- } );
- } ) );
+ } ),
+ ( value, key ) => dispatchPrefix + key
+ )
+ ) );
}
- return compose( hocs )( Component );
+ return hocs.length ? compose( hocs )( Component ) : Component;
} );
}
diff --git a/packages/rich-text/src/to-dom.js b/packages/rich-text/src/to-dom.js
index 9ebe8365a25bab..33cc08b27e47df 100644
--- a/packages/rich-text/src/to-dom.js
+++ b/packages/rich-text/src/to-dom.js
@@ -113,12 +113,6 @@ function remove( node ) {
return node.parentNode.removeChild( node );
}
-function prepareFormats( prepareEditableTree = [], value ) {
- return prepareEditableTree.reduce( ( accumlator, fn ) => {
- return fn( accumlator, value.text );
- }, value.formats );
-}
-
export function toDom( {
value,
multilineTag,
@@ -128,11 +122,15 @@ export function toDom( {
let startPath = [];
let endPath = [];
- const tree = toTree( {
- value: {
+ if ( prepareEditableTree ) {
+ value = {
...value,
- formats: prepareFormats( prepareEditableTree, value ),
- },
+ formats: prepareEditableTree( value ),
+ };
+ }
+
+ const tree = toTree( {
+ value,
multilineTag,
createEmpty,
append,
diff --git a/packages/rich-text/src/unregister-format-type.js b/packages/rich-text/src/unregister-format-type.js
index 6f6741183ee164..a70c1d1d417f2b 100644
--- a/packages/rich-text/src/unregister-format-type.js
+++ b/packages/rich-text/src/unregister-format-type.js
@@ -22,10 +22,7 @@ export function unregisterFormatType( name ) {
return;
}
- if (
- oldFormat.__experimentalCreatePrepareEditableTree &&
- oldFormat.__experimentalGetPropsForEditableTreePreparation
- ) {
+ if ( oldFormat.__experimentalCreatePrepareEditableTree ) {
removeFilter( 'experimentalRichText', name );
}