Lodash: Refactor away from _.omit() in style addSaveProps() (#45014)

* Lodash: Refactor away from _.omit() in addSaveProps() * Fix whitespacing in JSdoc as per review suggestions Co-authored-by: Aaron Robertshaw <[email protected]> * Fix typo in test name Co-authored-by: Aaron Robertshaw <[email protected]> * Add test for no property match for given path * Fix array access, add length check and additional test Co-authored-by: Aaron Robertshaw <[email protected]>
WordPress · Oct 19, 2022 · f1d0cdc · f1d0cdc
1 parent 5fff06a
commit f1d0cdc
Show file tree

Hide file tree

Showing 2 changed files with 328 additions and 4 deletions.
diff --git a/packages/block-editor/src/hooks/style.js b/packages/block-editor/src/hooks/style.js
@@ -1,7 +1,6 @@
 /**
  * External dependencies
  */
-import { omit } from 'lodash';
 import classnames from 'classnames';
 
 /**
@@ -134,6 +133,126 @@ const skipSerializationPathsSave = {
  */
 const renamedFeatures = { gradients: 'gradient' };
 
+/**
+ * A utility function used to remove one or more paths from a style object.
+ * Works in a way similar to Lodash's `omit()`. See unit tests and examples below.
+ *
+ * It supports a single string path:
+ *
+ * ```
+ * omitStyle( { color: 'red' }, 'color' ); // {}
+ * ```
+ *
+ * or an array of paths:
+ *
+ * ```
+ * omitStyle( { color: 'red', background: '#fff' }, [ 'color', 'background' ] ); // {}
+ * ```
+ *
+ * It also allows you to specify paths at multiple levels in a string.
+ *
+ * ```
+ * omitStyle( { typography: { textDecoration: 'underline' } }, 'typography.textDecoration' ); // {}
+ * ```
+ *
+ * You can remove multiple paths at the same time:
+ *
+ * ```
+ * omitStyle(
+ * 		{
+ * 			typography: {
+ * 				textDecoration: 'underline',
+ * 				textTransform: 'uppercase',
+ * 			}
+ *		},
+ *		[
+ * 			'typography.textDecoration',
+ * 			'typography.textTransform',
+ *		]
+ * );
+ * // {}
+ * ```
+ *
+ * You can also specify nested paths as arrays:
+ *
+ * ```
+ * omitStyle(
+ * 		{
+ * 			typography: {
+ * 				textDecoration: 'underline',
+ * 				textTransform: 'uppercase',
+ * 			}
+ *		},
+ *		[
+ * 			[ 'typography', 'textDecoration' ],
+ * 			[ 'typography', 'textTransform' ],
+ *		]
+ * );
+ * // {}
+ * ```
+ *
+ * With regards to nesting of styles, infinite depth is supported:
+ *
+ * ```
+ * omitStyle(
+ * 		{
+ * 			border: {
+ * 				radius: {
+ * 					topLeft: '10px',
+ * 					topRight: '0.5rem',
+ * 				}
+ * 			}
+ *		},
+ *		[
+ * 			[ 'border', 'radius', 'topRight' ],
+ *		]
+ * );
+ * // { border: { radius: { topLeft: '10px' } } }
+ * ```
+ *
+ * The third argument, `preserveReference`, defines how to treat the input style object.
+ * It is mostly necessary to properly handle mutation when recursively handling the style object.
+ * Defaulting to `false`, this will always create a new object, avoiding to mutate `style`.
+ * However, when recursing, we change that value to `true` in order to work with a single copy
+ * of the original style object.
+ *
+ * @see https://lodash.com/docs/4.17.15#omit
+ *
+ * @param {Object}       style             Styles object.
+ * @param {Array|string} paths             Paths to remove.
+ * @param {boolean}      preserveReference True to mutate the `style` object, false otherwise.
+ * @return {Object}      Styles object with the specified paths removed.
+ */
+export function omitStyle( style, paths, preserveReference = false ) {
+	if ( ! style ) {
+		return style;
+	}
+
+	let newStyle = style;
+	if ( ! preserveReference ) {
+		newStyle = JSON.parse( JSON.stringify( style ) );
+	}
+
+	if ( ! Array.isArray( paths ) ) {
+		paths = [ paths ];
+	}
+
+	paths.forEach( ( path ) => {
+		if ( ! Array.isArray( path ) ) {
+			path = path.split( '.' );
+		}
+
+		if ( path.length > 1 ) {
+			const [ firstSubpath, ...restPath ] = path;
+			omitStyle( newStyle[ firstSubpath ], [ restPath ], true );
+		} else if ( path.length === 1 ) {
+			delete newStyle[ path[ 0 ] ];
+		}
+	} );
+
+	return newStyle;
+}
+
 /**
  * Override props assigned to save component to inject the CSS variables definition.
  *
@@ -159,13 +278,13 @@ export function addSaveProps(
 		const skipSerialization = getBlockSupport( blockType, indicator );
 
 		if ( skipSerialization === true ) {
-			style = omit( style, path );
+			style = omitStyle( style, path );
 		}
 
 		if ( Array.isArray( skipSerialization ) ) {
 			skipSerialization.forEach( ( featureName ) => {
 				const feature = renamedFeatures[ featureName ] || featureName;
-				style = omit( style, [ [ ...path, feature ] ] );
+				style = omitStyle( style, [ [ ...path, feature ] ] );
 			} );
 		}
 	} );

diff --git a/packages/block-editor/src/hooks/test/style.js b/packages/block-editor/src/hooks/test/style.js
@@ -6,7 +6,7 @@ import { applyFilters } from '@wordpress/hooks';
 /**
  * Internal dependencies
  */
-import { getInlineStyles } from '../style';
+import { getInlineStyles, omitStyle } from '../style';
 
 describe( 'getInlineStyles', () => {
 	it( 'should return an empty object when called with undefined', () => {
@@ -202,3 +202,208 @@ describe( 'addSaveProps', () => {
 		} );
 	} );
 } );
+
+describe( 'omitStyle', () => {
+	it( 'should remove a single path', () => {
+		const style = { color: '#d92828', padding: '10px' };
+		const path = 'color';
+		const expected = { padding: '10px' };
+
+		expect( omitStyle( style, path ) ).toEqual( expected );
+	} );
+
+	it( 'should remove multiple paths', () => {
+		const style = { color: '#d92828', padding: '10px', background: 'red' };
+		const path = [ 'color', 'background' ];
+		const expected = { padding: '10px' };
+
+		expect( omitStyle( style, path ) ).toEqual( expected );
+	} );
+
+	it( 'should remove nested paths when specified as a string', () => {
+		const style = {
+			color: {
+				text: '#d92828',
+			},
+			typography: {
+				textDecoration: 'underline',
+				textTransform: 'uppercase',
+			},
+		};
+		const path = 'typography.textTransform';
+		const expected = {
+			color: {
+				text: '#d92828',
+			},
+			typography: {
+				textDecoration: 'underline',
+			},
+		};
+
+		expect( omitStyle( style, path ) ).toEqual( expected );
+	} );
+
+	it( 'should remove nested paths when specified as an array', () => {
+		const style = {
+			color: {
+				text: '#d92828',
+			},
+			typography: {
+				textDecoration: 'underline',
+				textTransform: 'uppercase',
+			},
+		};
+		const path = [ [ 'typography', 'textTransform' ] ];
+		const expected = {
+			color: {
+				text: '#d92828',
+			},
+			typography: {
+				textDecoration: 'underline',
+			},
+		};
+
+		expect( omitStyle( style, path ) ).toEqual( expected );
+	} );
+
+	it( 'should remove multiple nested paths', () => {
+		const style = {
+			color: {
+				text: '#d92828',
+			},
+			typography: {
+				textDecoration: 'underline',
+				textTransform: 'uppercase',
+			},
+		};
+		const path = [
+			[ 'typography', 'textTransform' ],
+			'typography.textDecoration',
+		];
+		const expected = {
+			color: {
+				text: '#d92828',
+			},
+			typography: {},
+		};
+
+		expect( omitStyle( style, path ) ).toEqual( expected );
+	} );
+
+	it( 'should remove paths with different nesting', () => {
+		const style = {
+			color: {
+				text: '#d92828',
+			},
+			typography: {
+				textDecoration: 'underline',
+				textTransform: 'uppercase',
+			},
+		};
+		const path = [
+			'color',
+			[ 'typography', 'textTransform' ],
+			'typography.textDecoration',
+		];
+		const expected = {
+			typography: {},
+		};
+
+		expect( omitStyle( style, path ) ).toEqual( expected );
+	} );
+
+	it( 'should support beyond 2 levels of nesting when passed as a single string', () => {
+		const style = {
+			border: {
+				radius: {
+					topLeft: '10px',
+					topRight: '0.5rem',
+				},
+			},
+		};
+		const path = 'border.radius.topRight';
+		const expected = {
+			border: {
+				radius: {
+					topLeft: '10px',
+				},
+			},
+		};
+
+		expect( omitStyle( style, path ) ).toEqual( expected );
+	} );
+
+	it( 'should support beyond 2 levels of nesting when passed as array of strings', () => {
+		const style = {
+			border: {
+				radius: {
+					topLeft: '10px',
+					topRight: '0.5rem',
+				},
+			},
+		};
+		const path = [ 'border.radius.topRight' ];
+		const expected = {
+			border: {
+				radius: {
+					topLeft: '10px',
+				},
+			},
+		};
+
+		expect( omitStyle( style, path ) ).toEqual( expected );
+	} );
+
+	it( 'should support beyond 2 levels of nesting when passed as array of arrays', () => {
+		const style = {
+			border: {
+				radius: {
+					topLeft: '10px',
+					topRight: '0.5rem',
+				},
+			},
+		};
+		const path = [ [ 'border', 'radius', 'topRight' ] ];
+		const expected = {
+			border: {
+				radius: {
+					topLeft: '10px',
+				},
+			},
+		};
+
+		expect( omitStyle( style, path ) ).toEqual( expected );
+	} );
+
+	it( 'should ignore a nullish style object', () => {
+		expect( omitStyle( undefined, 'color' ) ).toEqual( undefined );
+		expect( omitStyle( null, 'color' ) ).toEqual( null );
+	} );
+
+	it( 'should ignore a missing object property', () => {
+		const style1 = { typography: {} };
+		expect( omitStyle( style1, 'color' ) ).toEqual( style1 );
+
+		const style2 = { color: { text: '#d92828' } };
+		expect( omitStyle( style2, 'color.something' ) ).toEqual( style2 );
+
+		const style3 = {
+			border: {
+				radius: {
+					topLeft: '10px',
+					topRight: '0.5rem',
+				},
+			},
+		};
+		expect(
+			omitStyle( style3, [ [ 'border', 'radius', 'bottomLeft' ] ] )
+		).toEqual( style3 );
+	} );
+
+	it( 'should ignore an empty array path', () => {
+		const style = { typography: {}, '': 'test' };
+
+		expect( omitStyle( style, [] ) ).toEqual( style );
+		expect( omitStyle( style, [ [] ] ) ).toEqual( style );
+	} );
+} );