please @btroncone can you check this?

@bufke can you check this please? We can work on it but we need some response to do the PR and merge it.

I've been encountering issues related to this and ngrx-router-store. In my case, it's causing a guard that returns a url tree to fail to complete a navigation due to the store emitting changes before the navigation is complete.

Setting clone: false in the deepmerge options seems to fix it, but that of course negates the reason for using deepmerge in the first place.

I think a better design would be to revert back to using object.assign, and instead allow users to define their own reducer function for how to perform the merge of the local storage with their existing state.

Closing this issue as you can now specify a custom mergeReducer to fix this.

See my stackblitz fork of the original by @Jonnyprof.

import { ActionReducer, MetaReducer } from '@ngrx/store';
import { localStorageSync } from 'ngrx-store-localstorage';
import merge from 'lodash.merge';

const INIT_ACTION = '@ngrx/store/init';
const UPDATE_ACTION = '@ngrx/store/update-reducers';

const mergeReducer = (state: any, rehydratedState: any, action: any) => { 
  if ((action.type === INIT_ACTION || action.type === UPDATE_ACTION) && rehydratedState) {
    state = merge(state, rehydratedState); // <-- this line was changed to not clone
  }
  return state;
};

export function localStorageSyncReducer(reducer: ActionReducer<any>): ActionReducer<any> {
  return localStorageSync({
    keys: ['global'],
    rehydrate: true,
    mergeReducer // <-- use in the config here
  })(reducer);
}

export const metaReducers: Array<MetaReducer<any, any>> = [localStorageSyncReducer];

@BBlackwo Thanks, it's great to override mergeReducer to workaround this issue, but I'm still thinking that the defaultMergeReducer is wrong, because changes the main functionality of ngrx.
I'd forked this repository and tested it for a while, and I think this should fix the issue.

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;
};

Other option is use the merge from three shakeable lodash, for example lodash-es, but not sure if it does the correct merge array for you.

I can do a PR if you see it correct.

Just for context, we did originally use 'lodash.merge' but it wasn't getting updates (at least at the time) and people where opening security vulnerabilities about it that couldn't be fixed. deepmerge fixed that problem and is lighter weight. However it doesn't have the exact behaviour of lodash.merge. See #126

Both ideas where borrowed from vuex-persist which has to solve the same exact problem.

I looked into three shakeable lodash a long time ago and it seemed not feasible for the average human to get working. I don't know the current status.

My opinion would be to only consider switching the default if there was a very strong case for it. Going from lodash > deepmerge> lodash introduces breaking changes each time.

Yes, I know the historial lodash - deepmerge path, because of it I've used deepmerge in my solution.
We have a large application with lazy load modules depending on the page and connection, and if every time that we load an async module all the async pipes to the store are emitted, the loose of performance is huge due to changeDetection is fired for all the components.

Hi @Jonnyprof thanks for the info and suggested code change.

If you're able to open a PR with your suggested code change as well as some unit tests I can take a look, but can't promise I'll merge it. It will need a lot of testing to see how it relates to other issues raised.

Some related issues:

• Issue rehydratedState is not recalculated on UPDATE_ACTION #120 and PR update-reducers fix #122 suggests removing the || action.type === UPDATE_ACTION check completely and only merge state on the INIT_ACTION.
• Rehydrating lazy loaded feaure modules Rehydrating lazy loaded feaure modules #133
• Bug with Lazy Loaded feature modules Bug with Lazy Loaded feature modules #93 (has a really long thread!)

@BBlackwo looks like your comment in PR #122 (comment) is a good workaround.