From 6b534bb66ff3047e0d5a3a5bdbf8392408c548f8 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Tue, 24 Jul 2018 10:47:50 +0100 Subject: [PATCH] Don't repersist if the persisted changes are kept identical --- packages/data/src/persist.js | 8 +++++ packages/data/src/registry.js | 24 +++++++++++---- packages/data/src/test/persist.js | 49 ++++++++++++++++++++++++++++++- 3 files changed, 74 insertions(+), 7 deletions(-) diff --git a/packages/data/src/persist.js b/packages/data/src/persist.js index 56134daabde3c..05a2c42b3f389 100644 --- a/packages/data/src/persist.js +++ b/packages/data/src/persist.js @@ -47,6 +47,14 @@ export function restrictPersistence( reducer, keyToPersist ) { const nextState = reducer( state, action ); if ( action.type === 'SERIALIZE' ) { + // Returning the same instance if the state is kept identical avoids reserializing again + if ( + action.previousState && + action.previousState[ keyToPersist ] === nextState[ keyToPersist ] + ) { + return action.previousState; + } + return { [ keyToPersist ]: nextState[ keyToPersist ] }; } diff --git a/packages/data/src/registry.js b/packages/data/src/registry.js index 42d938e8cef26..d0e3df3ff7fb3 100644 --- a/packages/data/src/registry.js +++ b/packages/data/src/registry.js @@ -4,6 +4,11 @@ import { createStore } from 'redux'; import { flowRight, without, mapValues, overEvery, get } from 'lodash'; +/** + * WordPress dependencies + */ +import isShallowEqual from '@wordpress/is-shallow-equal'; + /** * Internal dependencies */ @@ -334,20 +339,27 @@ export function createRegistry( storeConfigs = {} ) { previousValue = persistedData; } - // Persist updated preferences - subscribe( () => { + const triggerPersist = () => { const newValue = Object.entries( namespaces ) .filter( ( [ , { persist } ] ) => persist ) .reduce( ( memo, [ reducerKey, { reducer, store } ] ) => { - memo[ reducerKey ] = reducer( store.getState(), { type: 'SERIALIZE' } ); + memo[ reducerKey ] = reducer( store.getState(), { + type: 'SERIALIZE', + previousState: get( previousValue, reducerKey ), + } ); return memo; }, {} ); - if ( newValue !== previousValue ) { + if ( ! isShallowEqual( newValue, previousValue ) ) { persistenceStorage.setItem( storageKey, JSON.stringify( newValue ) ); - previousValue = newValue; } - } ); + + previousValue = newValue; + }; + + // Persist updated preferences + subscribe( triggerPersist ); + triggerPersist(); } Object.entries( { diff --git a/packages/data/src/test/persist.js b/packages/data/src/test/persist.js index b78e33b58ea4d..4a55ae18905e7 100644 --- a/packages/data/src/test/persist.js +++ b/packages/data/src/test/persist.js @@ -1,13 +1,14 @@ /** * Internal dependencies */ -import { getPersistenceStorage, restrictPersistence } from '../persist'; +import { getPersistenceStorage, setPersistenceStorage, restrictPersistence } from '../persist'; import { createRegistry } from '../registry'; describe( 'persiss registry', () => { let registry; beforeEach( () => { registry = createRegistry(); + setPersistenceStorage( window.localStorage ); } ); it( 'should load the initial value from the local storage integrating it into reducer default value.', () => { @@ -54,6 +55,52 @@ describe( 'persiss registry', () => { expect( JSON.parse( getPersistenceStorage().getItem( storageKey ) ) ) .toEqual( { storeKey: { chicken: true } } ); } ); + + it( 'should not trigger persistence if the value doesn\'t change', () => { + const storageKey = 'dumbStorageKey2'; + let countCalls = 0; + const storage = { + getItem() { + return this.item; + }, + setItem( key, value ) { + countCalls++; + this.item = value; + }, + }; + setPersistenceStorage( storage ); + const reducer = ( state = { ribs: true }, action ) => { + if ( action.type === 'UPDATE' ) { + return { chicken: true }; + } + + return state; + }; + registry.registerStore( 'store1', { + reducer, + persist: true, + actions: { + update: () => ( { type: 'UPDATE' } ), + }, + } ); + registry.registerStore( 'store2', { + reducer, + actions: { + update: () => ( { type: 'UPDATE' } ), + }, + } ); + registry.setupPersistence( storageKey ); + + expect( countCalls ).toBe( 1 ); // Setup trigger initial persistence. + + registry.dispatch( 'store1' ).update(); + + expect( countCalls ).toBe( 2 ); // Updating state trigger persistence. + + registry.dispatch( 'store2' ).update(); + + expect( countCalls ).toBe( 2 ); // If the persisted state doesn't change, don't persist. + } ); } ); describe( 'restrictPersistence', () => {