Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(store): Not change store reference on every UPDATE_ACTION #150

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 44 additions & 9 deletions spec/index_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import * as deepmerge from 'deepmerge';
import 'localstorage-polyfill';
import { dateReviver, localStorageSync, rehydrateApplicationState, syncStateUpdate } from '../src/index';
const INIT_ACTION = '@ngrx/store/init';
const UPDATE_ACTION = '@ngrx/store/update-reducers';

// Very simple classes to test serialization options. They cover string, number, date, and nested classes
// The top level class has static functions to help test reviver, replacer, serialize and deserialize
Expand Down Expand Up @@ -453,7 +454,7 @@ describe('ngrxLocalStorage', () => {
const metaReducer = localStorageSync({keys: ['state'], rehydrate: true});
const action = {type: INIT_ACTION};

// Resultant state should merge the oldstring state and our initual state
// Resultant state should merge the oldstring state and our initial state
const finalState = metaReducer(reducer)(initialState, action);
expect(finalState.state.astring).toEqual(initialState.state.astring);
});
Expand Down Expand Up @@ -493,38 +494,72 @@ describe('ngrxLocalStorage', () => {
feature1: { slice11: false, slice12: [], slice13: {} },
feature2: { slice21: false, slice22: [], slice23: {} },
};

// A legit case where state is saved in chunks rather than as a single object
localStorage.setItem('feature1', JSON.stringify({ slice11: true, slice12: [1, 2] }));
localStorage.setItem('feature2', JSON.stringify({ slice21: true, slice22: [1, 2] }));

// Set up reducers
const reducer = (state = initialState, action) => state;
const mergeReducer = (state, rehydratedState, action) => {
// Perform a merge where we only want a single property from feature1
// but a deepmerge with feature2

return {
return {
...state,
feature1: {
slice11: rehydratedState.feature1.slice11
},
feature2: deepmerge(state.feature2, rehydratedState.feature2)
}
}
};
};
const metaReducer = localStorageSync({keys: [
{'feature1': ['slice11', 'slice12']},
{'feature2': ['slice21', 'slice22']},
], rehydrate: true, mergeReducer});

const action = {type: INIT_ACTION};

// Resultant state should merge the rehydrated partial state and our initial state
const finalState = metaReducer(reducer)(initialState, action);
expect(finalState).toEqual({
app: { app1: false, app2: [], app3: {} },
feature1: { slice11: true },
feature2: { slice21: true, slice22: [1, 2], slice23: {} },
});
});
});

it('should have same reference after rehydrate', () => {
const myInitialState = {
app: t1,
feature3: { hello: 'World' },
feature4: { slice21: false, slice22: [], slice23: {} },
};
const feature3LocalStorage = { hello: 'Peter' };

localStorage.setItem('feature3', JSON.stringify(feature3LocalStorage));

// Set up reducers
const reducer = (state = myInitialState, action) => state;
const metaReducer = localStorageSync({keys: ['feature3', 'feature4'], rehydrate: true});

const action = {type: UPDATE_ACTION};

// Resultant state should merge the rehydrated partial state and our initial state
const finalState = metaReducer(reducer)(myInitialState, action);

// Global state should not have same reference
expect(myInitialState).not.toBe(finalState);

// App state should have the same with same reference
expect(myInitialState.app).toBe(finalState.app);

// Feature3 state should not have same reference
expect(myInitialState.feature3).not.toBe(finalState.feature3);
// Feature3 state should have localStorage value
expect(finalState.feature3).toEqual(feature3LocalStorage);

// Feature4 state should be the same with same reference
expect(myInitialState.feature4).toBe(finalState.feature4);
});
});
22 changes: 12 additions & 10 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export const dateReviver = (key: string, value: any) => {
const dummyReviver = (key: string, value: any) => value;

const checkIsBrowserEnv = () => {
return typeof window !== 'undefined'
return typeof window !== 'undefined';
};

const validateStateKeys = (keys: any[]) => {
Expand Down Expand Up @@ -220,15 +220,17 @@ export const syncStateUpdate = (
};

// Default merge strategy is a full deep merge.
export const defaultMergeReducer = (state: any, rehydratedState: any, action: any) => {

if ((action.type === INIT_ACTION || action.type === UPDATE_ACTION) && rehydratedState) {
const overwriteMerge = (destinationArray, sourceArray, options) => sourceArray;
const options: deepmerge.Options = {
arrayMerge: overwriteMerge
};

export const defaultMergeReducer = (state: any, rehydratedState: any, action: any) => {
const overwriteMerge = (destinationArray, sourceArray, options) => sourceArray;
const options: deepmerge.Options = {
arrayMerge: overwriteMerge
};
if (action.type === INIT_ACTION && rehydratedState) {
state = deepmerge(state, rehydratedState, options);
} else if (action.type === UPDATE_ACTION && rehydratedState) {
Object.keys(rehydratedState).forEach(partialState => {
state[partialState] = deepmerge(state[partialState] || {}, rehydratedState[partialState], options);
});
}

return state;
Expand Down Expand Up @@ -282,7 +284,7 @@ export const localStorageSync = (config: LocalStorageConfig) => (
// Merge the store state with the rehydrated state using
// either a user-defined reducer or the default.
nextState = mergeReducer(nextState, rehydratedState, action);

nextState = reducer(nextState, action);

if (action.type !== INIT_ACTION) {
Expand Down