Editor: Deeply diff/merge edited property edits/updates

packages/editor/src/store/object.js

@@ -0,0 +1,88 @@
+/**
+ * External dependencies
+ */
+import { isPlainObject } from 'lodash';
+
+/**
+ * Returns an object against which it is safe to perform mutating operations,
+ * given the original object and its current working copy.
+ *
+ * @param {Object} original Original object.
+ * @param {Object} working  Working object.
+ *
+ * @return {Object} Mutation-safe object.
+ */
+export function getMutateSafeObject( original, working ) {
+	if ( original === working ) {
+		return { ...original };
+	}
+
+	return working;
+}
+
+/**
+ * Given two objects, returns the minimal object shape of the difference of
+ * values contained in RHS and not LHS, recursively. Returns RHS reference if
+ * result of difference would be the same as the RHS object.
+ *
+ * @param {Object} lhs Original object to compare against.
+ * @param {Object} rhs New object from which to generate difference.
+ *
+ * @return {Object} Minimal difference.
+ */
+export function diff( lhs, rhs ) {
+	let diffed = rhs;
+	for ( const key in rhs ) {
+		let value = rhs[ key ];
+		if ( isPlainObject( value ) && isPlainObject( lhs[ key ] ) ) {
+			// Recurse to generate diff of child values.
+			value = diff( lhs[ key ], value );
+
+			// If a diff of the child value is non-empty, it's inferred to be
+			// non-equal and should replace the value in the returned diff.
+			// Otherwise, if equal, fall through to delete from diff.
+			if ( Object.keys( value ).length ) {
+				diffed = getMutateSafeObject( rhs, diffed );
+				diffed[ key ] = value;
+				continue;
+			}
+		} else if ( value !== lhs[ key ] ) {
+			// To preserve reference, invert the iteration to _keep_ values
+			// which are different, and mutate only to delete equal values.
+			continue;
+		}
+
+		diffed = getMutateSafeObject( rhs, diffed );
+		delete diffed[ key ];
+	}
+
+	return diffed;
+}
+
+/**
+ * Given objects, returns the combined object shape of the merging of their
+ * values. Returns the reference of the first object if all objects are the
+ * same.
+ *
+ * @param {...Object} objects Objects to merge.
+ *
+ * @return {Object} Merged object.
+ */
+export function merge( ...objects ) {
+	const firstObject = objects[ 0 ];
+	return objects.reduce( ( merged, object ) => {
+		for ( const key in object ) {
+			let value = object[ key ];
+			if ( isPlainObject( merged[ key ] ) && isPlainObject( value ) ) {
+				value = merge( merged[ key ], value );
+			}
+
+			if ( merged[ key ] !== value ) {
+				merged = getMutateSafeObject( firstObject, merged );
+				merged[ key ] = value;
+			}
+		}
+
+		return merged;
+	}, firstObject );
+}

packages/editor/src/store/reducer.js

@@ -34,6 +34,7 @@ import {
 	INITIAL_EDITS_DEFAULTS,
 } from './defaults';
 import { insertAt, moveTo } from './array';
+import { getMutateSafeObject, merge, diff } from './object';
 
 /**
  * Returns a post attribute value, flattening nested rendered content using its
@@ -101,23 +102,6 @@ function getFlattenedBlocks( blocks ) {
 	return flattenedBlocks;
 }
 
-/**
- * Returns an object against which it is safe to perform mutating operations,
- * given the original object and its current working copy.
- *
- * @param {Object} original Original object.
- * @param {Object} working  Working object.
- *
- * @return {Object} Mutation-safe object.
- */
-function getMutateSafeObject( original, working ) {
-	if ( original === working ) {
-		return { ...original };
-	}
-
-	return working;
-}
-
 /**
  * Returns true if the two object arguments have the same keys, or false
  * otherwise.
@@ -240,23 +224,7 @@ export const editor = flow( [
 	edits( state = {}, action ) {
 		switch ( action.type ) {
 			case 'EDIT_POST':
-				const recurseEdits = ( edits, newState ) => {
-					return reduce( edits, ( result, value, key ) => {
-						// Only assign into result if not already same value
-						if ( value !== newState[ key ] ) {
-							result = getMutateSafeObject( state, result );
-
-							if ( typeof newState[ key ] === 'object' && ! Array.isArray( newState[ key ] ) ) {
-								result[ key ] = recurseEdits( value, newState[ key ] );
-							} else {
-								result[ key ] = value;
-							}
-						}
-						return result;
-					}, newState );
-				};
-
-				return recurseEdits( action.edits, state );
+				return merge( state, action.edits );
 
 			case 'RESET_BLOCKS':
 				if ( 'content' in state ) {
@@ -267,19 +235,14 @@ export const editor = flow( [
 
 			case 'UPDATE_POST':
 			case 'RESET_POST':
-				const getCanonicalValue = action.type === 'UPDATE_POST' ?
-					( key ) => action.edits[ key ] :
-					( key ) => getPostRawValue( action.post[ key ] );
-
-				return reduce( state, ( result, value, key ) => {
-					if ( value !== getCanonicalValue( key ) ) {
-						return result;
-					}
+				let updates;
+				if ( action.type === 'UPDATE_POST' ) {
+					updates = action.edits;
+				} else {
+					updates = mapValues( action.post, getPostRawValue );
+				}
 
-					result = getMutateSafeObject( state, result );
-					delete result[ key ];
-					return result;
-				}, state );
+				return diff( updates, state );
 		}
 
 		return state;

packages/editor/src/store/test/object.js

@@ -0,0 +1,81 @@
+/**
+ * Internal dependencies
+ */
+import { getMutateSafeObject, diff, merge } from '../object';
+
+describe( 'object', () => {
+	describe( 'getMutateSafeObject', () => {
+		let original;
+		beforeEach( () => {
+			// Verify cloned by mutating a frozen object.
+			original = Object.freeze( { a: 1 } );
+		} );
+
+		it( 'creates a mutable clone if working with original', () => {
+			let working = original;
+
+			working = getMutateSafeObject( original, working );
+			working.b = 2;
+
+			expect( working ).not.toBe( original );
+			expect( working ).toEqual( { a: 1, b: 2 } );
+		} );
+
+		it( 'creates returns identity if working with cloned', () => {
+			let working = getMutateSafeObject( original, original );
+
+			for ( let i = 0; i < 2; i++ ) {
+				const pendingWorking = getMutateSafeObject( original, working );
+				expect( pendingWorking ).toBe( working );
+				working = pendingWorking;
+			}
+		} );
+	} );
+
+	describe( 'diff', () => {
+		it( 'should return RHS reference if result would be same as RHS', () => {
+			const lhs = {};
+			const rhs = { a: { b: { c: 1 } } };
+			const result = diff( lhs, rhs );
+
+			expect( result ).toBe( rhs );
+		} );
+
+		it( 'should omit deeply equal property values', () => {
+			const lhs = { a: { b: { c: 1 } } };
+			const rhs = { a: { b: { c: 1 } } };
+			const result = diff( lhs, rhs );
+
+			expect( result ).not.toBe( rhs );
+			expect( result ).toEqual( {} );
+		} );
+
+		it( 'should return minimal object difference', () => {
+			const lhs = { a: { b: { c: 1 } } };
+			const rhs = { a: { b: { c: 1, d: 2 } } };
+			const result = diff( lhs, rhs );
+
+			expect( result ).not.toBe( rhs );
+			expect( result ).toEqual( { a: { b: { d: 2 } } } );
+		} );
+	} );
+
+	describe( 'merge', () => {
+		it( 'should return equal reference if same, deeply', () => {
+			const obj1 = { a: { b: { c: 1 } } };
+			const obj2 = { a: { b: { c: 1 } } };
+			const result = merge( obj1, obj2 );
+
+			expect( result ).toBe( obj1 );
+		} );
+
+		it( 'should deeply merged object', () => {
+			const obj1 = { a: { b: { c: 1 } } };
+			const obj2 = { a: { b: { d: 2 } } };
+			const result = merge( obj1, obj2 );
+
+			expect( result ).not.toBe( obj1 );
+			expect( result ).toEqual( { a: { b: { c: 1, d: 2 } } } );
+		} );
+	} );
+} );

packages/editor/src/store/test/reducer.js

@@ -1040,6 +1040,119 @@ describe( 'state', () => {
 				} );
 			} );
 
+			it( 'should merge object values', () => {
+				const original = editor( undefined, {
+					type: 'EDIT_POST',
+					edits: {
+						meta: {
+							a: 1,
+						},
+					},
+				} );
+
+				const state = editor( original, {
+					type: 'EDIT_POST',
+					edits: {
+						meta: {
+							b: 2,
+						},
+					},
+				} );
+
+				expect( state.present.edits ).toEqual( {
+					meta: {
+						a: 1,
+						b: 2,
+					},
+				} );
+			} );
+
+			it( 'return state by reference on unchanging update', () => {
+				const original = editor( undefined, {} );
+
+				const state = editor( original, {
+					type: 'UPDATE_POST',
+					edits: {},
+				} );
+
+				expect( state.present.edits ).toBe( original.present.edits );
+			} );
+
+			it( 'unset updated post values which match', () => {
+				const original = editor( undefined, {
+					type: 'EDIT_POST',
+					edits: {
+						title: 'modified title',
+						meta: {
+							a: 1,
+							b: 2,
+						},
+					},
+				} );
+
+				const state = editor( original, {
+					type: 'UPDATE_POST',
+					edits: {
+						title: 'modified title',
+						meta: {
+							a: 1,
+						},
+					},
+				} );
+
+				expect( state.present.edits ).toEqual( {
+					meta: {
+						b: 2,
+					},
+				} );
+			} );
+
+			it( 'unset reset post values which match by canonical value', () => {
+				const original = editor( undefined, {
+					type: 'EDIT_POST',
+					edits: {
+						title: 'modified title',
+					},
+				} );
+
+				const state = editor( original, {
+					type: 'RESET_POST',
+					post: {
+						title: {
+							raw: 'modified title',
+						},
+					},
+				} );
+
+				expect( state.present.edits ).toEqual( {} );
+			} );
+
+			it( 'unset top-level key of empty object value', () => {
+				const original = editor( undefined, {
+					type: 'EDIT_POST',
+					edits: {
+						title: 'modified title',
+						meta: {
+							a: 1,
+							b: 2,
+						},
+					},
+				} );
+
+				const state = editor( original, {
+					type: 'UPDATE_POST',
+					edits: {
+						title: 'modified title',
+						meta: {
+							a: 1,
+							b: 2,
+						},
+					},
+				} );
+
+				expect( state.present.edits ).toEqual( {} );
+			} );
+
 			it( 'should omit content when resetting', () => {
 				// Use case: When editing in Text mode, we defer to content on
 				// the property, but we reset blocks by parse when switching