Merge pull request #3544 from WordPress/fix/change-detection-multi-re…

…duce Framework: Fix change detection to maintain multiple instances of state
WordPress · Nov 20, 2017 · 6f5e90c · 6f5e90c
2 parents bc46f77 + 5660eb0
commit 6f5e90c
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 30 deletions.
diff --git a/editor/utils/with-change-detection/index.js b/editor/utils/with-change-detection/index.js
@@ -14,10 +14,8 @@ import { includes } from 'lodash';
  * @return {Function}                    Enhanced reducer
  */
 export default function withChangeDetection( reducer, options = {} ) {
-	let originalState;
-
 	return ( state, action ) => {
-		let nextState = reducer( state, action );
+		const nextState = reducer( state, action );
 
 		// Reset at:
 		//  - Initial state
@@ -27,22 +25,17 @@ export default function withChangeDetection( reducer, options = {} ) {
 			includes( options.resetTypes, action.type )
 		);
 
-		const nextIsDirty = ! isReset && originalState !== nextState;
-
-		// Only revise state if changing.
-		if ( nextIsDirty !== nextState.isDirty ) {
-			// In case the original reducer returned the same reference and we
-			// intend to mutate, create a shallow clone.
-			if ( state === nextState ) {
-				nextState = { ...nextState };
-			}
-
-			nextState.isDirty = nextIsDirty;
+		if ( isReset ) {
+			return {
+				...nextState,
+				isDirty: false,
+			};
 		}
 
-		// Track original state against which dirty test compares reference
-		if ( isReset ) {
-			originalState = nextState;
+		const isChanging = state !== nextState;
+
+		if ( isChanging ) {
+			nextState.isDirty = true;
 		}
 
 		return nextState;

diff --git a/editor/utils/with-change-detection/test/index.js b/editor/utils/with-change-detection/test/index.js
@@ -9,7 +9,7 @@ import deepFreeze from 'deep-freeze';
 import withChangeDetection from '../';
 
 describe( 'withChangeDetection()', () => {
-	const initialState = { count: 0 };
+	const initialState = deepFreeze( { count: 0 } );
 
 	function originalReducer( state = initialState, action ) {
 		switch ( action.type ) {
@@ -18,9 +18,6 @@ describe( 'withChangeDetection()', () => {
 
 			case 'RESET_AND_CHANGE_REFERENCE':
 				return { ...state };
-
-			case 'SET_STATE':
-				return action.state;
 		}
 
 		return state;
@@ -67,19 +64,19 @@ describe( 'withChangeDetection()', () => {
 		expect( state ).toEqual( { count: 1, isDirty: true } );
 	} );
 
-	it( 'should reset if state reverts to its original reference', () => {
-		const reducer = withChangeDetection( originalReducer, { resetTypes: [ 'RESET' ] } );
+	it( 'should maintain separate states', () => {
+		const reducer = withChangeDetection( originalReducer );
 
-		let state;
+		let firstState;
 
-		const originalState = state = reducer( undefined, {} );
-		expect( state ).toEqual( { count: 0, isDirty: false } );
+		firstState = reducer( undefined, {} );
+		expect( firstState ).toEqual( { count: 0, isDirty: false } );
 
-		state = reducer( deepFreeze( state ), { type: 'INCREMENT' } );
-		expect( state ).toEqual( { count: 1, isDirty: true } );
+		const secondState = reducer( undefined, { type: 'INCREMENT' } );
+		expect( secondState ).toEqual( { count: 1, isDirty: false } );
 
-		state = reducer( deepFreeze( state ), { type: 'SET_STATE', state: originalState } );
-		expect( state ).toEqual( { count: 0, isDirty: false } );
+		firstState = reducer( deepFreeze( firstState ), {} );
+		expect( firstState ).toEqual( { count: 0, isDirty: false } );
 	} );
 
 	it( 'should flag as not dirty even if reset type causes reference change', () => {