From 254cf3360981ea9132f1a0a5a9edb0b713df4b08 Mon Sep 17 00:00:00 2001 From: Anton Timmermans Date: Wed, 21 Nov 2018 10:16:26 +0100 Subject: [PATCH 1/3] Fix RichText rerendering when it shouldn't The prepareEditableTree and onChangeEditableValue function stacks would have a new reference on every render. This prevents that by memoized the stack based on the previous stack and the newly added function. --- packages/annotations/src/format/annotation.js | 55 ++++++++++++++----- packages/annotations/src/store/selectors.js | 6 +- .../rich-text/src/register-format-type.js | 25 ++++++--- 3 files changed, 62 insertions(+), 24 deletions(-) diff --git a/packages/annotations/src/format/annotation.js b/packages/annotations/src/format/annotation.js index a2f6f2973c726..e72786bad0e3b 100644 --- a/packages/annotations/src/format/annotation.js +++ b/packages/annotations/src/format/annotation.js @@ -1,3 +1,8 @@ +/** + * External dependencies + */ +import memize from 'memize'; + /** * WordPress dependencies */ @@ -115,6 +120,40 @@ 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' ), @@ -128,21 +167,9 @@ export const annotation = { return null; }, __experimentalGetPropsForEditableTreePreparation( select, { richTextIdentifier, blockClientId } ) { - 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; - }; + return getAnnotationObject( select( STORE_KEY ).__experimentalGetAnnotationsForRichText( blockClientId, richTextIdentifier ) ); }, + __experimentalCreatePrepareEditableTree: createPrepareEditableTree, __experimentalGetPropsForEditableTreeChangeHandler( dispatch ) { return { removeAnnotation: dispatch( STORE_KEY ).__experimentalRemoveAnnotation, diff --git a/packages/annotations/src/store/selectors.js b/packages/annotations/src/store/selectors.js index ca6fcb64796d5..f161b94a79fea 100644 --- a/packages/annotations/src/store/selectors.js +++ b/packages/annotations/src/store/selectors.js @@ -4,6 +4,8 @@ import createSelector from 'rememo'; import { get, flatMap } from 'lodash'; +const emptyArray = []; + /** * Returns the annotations for a specific client ID. * @@ -19,7 +21,7 @@ export const __experimentalGetAnnotationsForBlock = createSelector( } ); }, ( state, blockClientId ) => [ - get( state, blockClientId, [] ), + get( state, blockClientId, emptyArray ), ] ); @@ -54,7 +56,7 @@ export const __experimentalGetAnnotationsForRichText = createSelector( } ); }, ( state, blockClientId ) => [ - get( state, blockClientId, [] ), + get( state, blockClientId, emptyArray ), ] ); diff --git a/packages/rich-text/src/register-format-type.js b/packages/rich-text/src/register-format-type.js index 8b61fc3a4308b..1fd7206ad5892 100644 --- a/packages/rich-text/src/register-format-type.js +++ b/packages/rich-text/src/register-format-type.js @@ -2,6 +2,7 @@ * External dependencies */ import { mapKeys } from 'lodash'; +import memize from 'memize'; /** * WordPress dependencies @@ -119,6 +120,14 @@ export function registerFormatType( name, settings ) { dispatch( 'core/rich-text' ).addFormatTypes( settings ); + const emptyArray = []; + const getFunctionStackMemoized = memize( ( previousStack = emptyArray, newFunction ) => { + return [ + ...previousStack, + newFunction, + ]; + } ); + if ( settings.__experimentalGetPropsForEditableTreePreparation ) { @@ -133,13 +142,13 @@ export function registerFormatType( name, settings ) { const additionalProps = {}; if ( settings.__experimentalCreatePrepareEditableTree ) { - additionalProps.prepareEditableTree = [ - ...( props.prepareEditableTree || [] ), + additionalProps.prepareEditableTree = getFunctionStackMemoized( + props.prepareEditableTree, settings.__experimentalCreatePrepareEditableTree( props[ `format_${ name }` ], { richTextIdentifier: props.identifier, blockClientId: props.clientId, - } ), - ]; + } ) + ); } if ( settings.__experimentalCreateOnChangeEditableValue ) { @@ -155,16 +164,16 @@ export function registerFormatType( name, settings ) { return accumulator; }, {} ); - additionalProps.onChangeEditableValue = [ - ...( props.onChangeEditableValue || [] ), + additionalProps.onChangeEditableValue = getFunctionStackMemoized( + props.onChangeEditableValue, settings.__experimentalCreateOnChangeEditableValue( { ...props[ `format_${ name }` ], ...dispatchProps, }, { richTextIdentifier: props.identifier, blockClientId: props.clientId, - } ), - ]; + } ) + ); } return Date: Wed, 21 Nov 2018 10:35:19 +0100 Subject: [PATCH 2/3] Rename emptyArray to EMPTY_ARRAY --- packages/annotations/src/store/selectors.js | 17 +++++++++++++---- packages/rich-text/src/register-format-type.js | 14 ++++++++++++-- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/packages/annotations/src/store/selectors.js b/packages/annotations/src/store/selectors.js index f161b94a79fea..a39a315c92f90 100644 --- a/packages/annotations/src/store/selectors.js +++ b/packages/annotations/src/store/selectors.js @@ -4,7 +4,16 @@ import createSelector from 'rememo'; import { get, flatMap } from 'lodash'; -const emptyArray = []; +/** + * 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 = []; /** * Returns the annotations for a specific client ID. @@ -21,12 +30,12 @@ export const __experimentalGetAnnotationsForBlock = createSelector( } ); }, ( state, blockClientId ) => [ - get( state, blockClientId, emptyArray ), + get( state, blockClientId, EMPTY_ARRAY ), ] ); export const __experimentalGetAllAnnotationsForBlock = function( state, blockClientId ) { - return get( state, blockClientId, [] ); + return get( state, blockClientId, EMPTY_ARRAY ); }; /** @@ -56,7 +65,7 @@ export const __experimentalGetAnnotationsForRichText = createSelector( } ); }, ( state, blockClientId ) => [ - get( state, blockClientId, emptyArray ), + get( state, blockClientId, EMPTY_ARRAY ), ] ); diff --git a/packages/rich-text/src/register-format-type.js b/packages/rich-text/src/register-format-type.js index 1fd7206ad5892..262812a22b248 100644 --- a/packages/rich-text/src/register-format-type.js +++ b/packages/rich-text/src/register-format-type.js @@ -11,6 +11,17 @@ 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. @@ -120,8 +131,7 @@ export function registerFormatType( name, settings ) { dispatch( 'core/rich-text' ).addFormatTypes( settings ); - const emptyArray = []; - const getFunctionStackMemoized = memize( ( previousStack = emptyArray, newFunction ) => { + const getFunctionStackMemoized = memize( ( previousStack = EMPTY_ARRAY, newFunction ) => { return [ ...previousStack, newFunction, From cb37598f38e26af3c0401ca73e6d839b1367202e Mon Sep 17 00:00:00 2001 From: Anton Timmermans Date: Wed, 21 Nov 2018 10:46:54 +0100 Subject: [PATCH 3/3] Add memize as a dependency to annotations package --- package-lock.json | 1 + packages/annotations/package.json | 1 + 2 files changed, 2 insertions(+) diff --git a/package-lock.json b/package-lock.json index 5dca87bdac584..3b25939df9b96 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2283,6 +2283,7 @@ "@wordpress/i18n": "file:packages/i18n", "@wordpress/rich-text": "file:packages/rich-text", "lodash": "^4.17.10", + "memize": "^1.0.5", "rememo": "^3.0.0", "uuid": "^3.3.2" } diff --git a/packages/annotations/package.json b/packages/annotations/package.json index 89eb55c53d0a6..b000f3e7cb454 100644 --- a/packages/annotations/package.json +++ b/packages/annotations/package.json @@ -26,6 +26,7 @@ "@wordpress/i18n": "file:../i18n", "@wordpress/rich-text": "file:../rich-text", "lodash": "^4.17.10", + "memize": "^1.0.5", "rememo": "^3.0.0", "uuid": "^3.3.2" },