Skip to content

Commit

Permalink
Fix RichText rerendering when it shouldn't (#12161)
Browse files Browse the repository at this point in the history
* 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.

* Rename emptyArray to EMPTY_ARRAY

* Add memize as a dependency to annotations package
  • Loading branch information
atimmer authored and youknowriad committed Nov 21, 2018
1 parent e651069 commit 01be7ac
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 25 deletions.
1 change: 1 addition & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions packages/annotations/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down
55 changes: 41 additions & 14 deletions packages/annotations/src/format/annotation.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
/**
* External dependencies
*/
import memize from 'memize';

/**
* WordPress dependencies
*/
Expand Down Expand Up @@ -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' ),
Expand All @@ -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,
Expand Down
17 changes: 14 additions & 3 deletions packages/annotations/src/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,17 @@
import createSelector from 'rememo';
import { get, flatMap } from 'lodash';

/**
* 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.
*
Expand All @@ -19,12 +30,12 @@ export const __experimentalGetAnnotationsForBlock = createSelector(
} );
},
( state, blockClientId ) => [
get( state, blockClientId, [] ),
get( state, blockClientId, EMPTY_ARRAY ),
]
);

export const __experimentalGetAllAnnotationsForBlock = function( state, blockClientId ) {
return get( state, blockClientId, [] );
return get( state, blockClientId, EMPTY_ARRAY );
};

/**
Expand Down Expand Up @@ -54,7 +65,7 @@ export const __experimentalGetAnnotationsForRichText = createSelector(
} );
},
( state, blockClientId ) => [
get( state, blockClientId, [] ),
get( state, blockClientId, EMPTY_ARRAY ),
]
);

Expand Down
35 changes: 27 additions & 8 deletions packages/rich-text/src/register-format-type.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* External dependencies
*/
import { mapKeys } from 'lodash';
import memize from 'memize';

/**
* WordPress dependencies
Expand All @@ -10,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.
Expand Down Expand Up @@ -119,6 +131,13 @@ export function registerFormatType( name, settings ) {

dispatch( 'core/rich-text' ).addFormatTypes( settings );

const getFunctionStackMemoized = memize( ( previousStack = EMPTY_ARRAY, newFunction ) => {
return [
...previousStack,
newFunction,
];
} );

if (
settings.__experimentalGetPropsForEditableTreePreparation
) {
Expand All @@ -133,13 +152,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 ) {
Expand All @@ -155,16 +174,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 <OriginalComponent
Expand Down

0 comments on commit 01be7ac

Please sign in to comment.