Bug with Lazy Loaded feature modules

[bmalinconico]

Hello,

The following PR introduced a bug related to lazy loaded feature modules: https://github.com/btroncone/ngrx-store-localstorage/pull/76/files

The net effect of this PR is to re-hydrate the state when every feature module is loaded, and this breaks sections of the state which are selectively serialized (localStorageSync({keys: [{todos: ['name', 'status'] }, ... ]}))).

The result is the state being rehydrated and deleting all other keys not in local storage.

[philjones88]

I’ve just hit this too

[bmalinconico]

@philjones88 Possible solution that works for my usage: #94

[btroncone]

@bmalinconico Do you want to take the lead on this? I can make you a collaborator to this project. I am not able to put much time into this right now and do not want to be a bottleneck.

[bmalinconico]

@btroncone Sure, I think the biggest risk of my PR is the kinda "stupid" way it determines the state key name const name = Object.keys(key)[0]; What do you want me to do beyond deal with bugs I create myself? :)

[btroncone]

@bmalinconico Really looking for another extra set of eyes that is using this library every day. Need help determining priority and impact of some of the proposed PR's / issues.

[tanyagray]

I pulled it down but had problems getting the tests running. I'm also hesitant to merge anything that's untested cos I'm not a "regular" user as such. I do use feature modules though so maybe @bmalinconico we could collab on this one? Happy to live chat elsewhere.

[btroncone]

@tanyagray Agree, definitely want to be careful here. Thanks for taking a look! 👍

[bmalinconico]

@tanyagray odd, tests ran fine for me. Was it a suite error or a test failure?

[bmalinconico]

@tanyagray I'd be happy to collab on this if we can find time. I'm on the east coast of the USA and am really only on during work hours.

[magnattic]

Is there a workaround or older version to revert to? I just introduced lazy loaded modules to my project and this resets my state when loading the module.

[bmalinconico]

@magnattic My PR (#94) works for my use-case and I'm using it in production.

In your package.json

    "ngrx-store-localstorage": "https://github.com/bmalinconico/ngrx-store-localstorage.git#fix_partial_rehydration_of_keys_dist",

However I started by reverting to 0.3.0 and that worked for me too but had its own issues relate to feature modules.

[magnattic]

Thanks for providing your PR. Unfortunately I can't get it to work with my setup:
I get an

ERROR TypeError: keys.map is not a function

in mergePartialStates, this is the line:

keys.map(function (key) { return newValues_1[key] = rehydratedState[name_2][key]; });

The problem seems to be that it's not compatible with the custom serialize/deserialize functions I am using. This is my setup:

export function localStorageSyncReducer(reducer: ActionReducer<any>): ActionReducer<any> {
  return localStorageSync({
    keys: [
      {
        shop: {
          deserialize: (json: any) => {
            const jsonFixed = fixInvalidCombinations(fixOldState(json));
            return _.defaultsDeep(jsonFixed, { homePage: fromHomepage.initialState, prospect: fromProspect.initialState });
          },
          serialize: (state: ShopState) => {
            const { numCardsVisible, ...serializedHomePage } = state.homePage;
            return { homePage: serializedHomePage, prospect: state.prospect };
          }
        }
      },
      {
        checkout: {
          deserialize: (json: any) => _.defaultsDeep(json, { customer: fromCustomer.initialState })
        }
      }
    ],
    rehydrate: true
  })(reducer);
}

[bmalinconico]

@magnattic I see what is wrong, and can update the PR. It looks like you serializing the entire shop and checkout state?

If you are I'll make a small adjustment, but if you are serializing the whole state you are probably running into a different problem then I was trying to solve.

[bmalinconico]

@magnattic updated my branch, give that a try. PR hasn't updated yet.

[magnattic]

Thanks, this removes the error, but still overwrites my State when the feature module is loaded.

This is my scenario:

• I have 2 keys in my state: shop and checkout
• shop has 3 subkeys: homePage, prospect and offerData
• checkout is part of my feature module and is being lazy loaded.
• I use custom serializers/deserializers for shop and checkout (as shown above)
• I don't want to store shop.offerData in localstorage, as this is loaded on the fly.

What happens?

When I load my feature module for checkout, the whole state for shop.offerData is reset to its default of null.

Basically what I want is for localStorageSync to leave shop.offerData alone and neither sync nor overwrite it.

Do I have to setup my sync differently?

[bmalinconico]

@magnattic So the problem is how do we determine when to partially overwrite the state from localstorage, and when do we perform a merge.

The current rules are:

• If rehydration key is a string or object (your case), we replace the state with local storage
• If rehydration key is an array, we only replace the strings in that array.

If it is a object and custom serializers/deserializers are used, how do we determine if we should perform a merge or a replace.

I can provide an update to get you moving if 0.3.0 isn't going to work for you, but this behavior determination is something I should defer to an owner or collaborator. I'll update my solution with their chosen solution.

We could always add a parameter to the object to indicate the rehydrated state, when provided an object, should be merged.

[magnattic]

@bmalinconico I realized I misrepresented my scenario, I updated my comment above.

Sorry if I am missing something, but why can't we simply do something like this?

if ((action.type === INIT_ACTION || action.type === UPDATE_ACTION) && rehydratedState) {
      state = _.defaultsDeep({}, rehydratedState, state);

Use the rehydratedState where possible and if a property is missing somewhere, add it from the existing state. This is working recursively.
(I am using lodash here, but I guess the function could be replicated to remove the dependency.)

I am not sure why we need the stateKeys at all at this point. Aren't they only needed to get the rehydratedState?

[bmalinconico]

I feel like, if you are rehydrating the entire section of a state an undefined value coming from localstorage should be respected. We should only do a deep merge when you are selectively rehydrating keys.

We should also be careful to ensure we are only duping one level deep, unless a true deep merge is what we are after.

Either way these are behavior choices I'll leave up to owners.

[bmalinconico]

@magnattic ah I didn't see the filter property on that object, perhaps that could be used to control the behavior.

[magnattic]

I am still massively confused by the whole thing. Why are we even touching the whole state when a feature module is loaded? The 'update-reducers' action has a property feature where it states which feature is being loaded, so why are we not limiting our rehydration to that slice of state? Why would we be overwriting or merging anything from localstorage a that point that is not part of the feature slice?

[voidek-work]

In your package.json

"ngrx-store-localstorage": "https://github.com/bmalinconico/ngrx-store-localstorage.git#fix_partial_rehydration_of_keys_dist",

However I started by reverting to 0.3.0 and that worked for me too but had its own issues relate to feature modules.

Hi, I tried to put this PR when I noticed that my lazy loadable modules get the default state instead of the saved one. Everything worked as it should, but as soon as I connected one more module everything broke. Sorry my English :)

[voidek-work]

The problem was solved when I specified a synchronous module in an array of keys
localStorageSync({ keys: ['authContainer', 'layout', 'admin'], removeOnUndefined: true, rehydrate: true })(reducer);
layout is not lazy

[bmalinconico]

@magnattic So the reason inevitable is how NGRX gets a feature module bootstrapped. See this line: https://github.com/ngrx/platform/blob/cb473c00475289509f0924321bb293d0ad1137ca/modules/store/src/reducer_manager.ts#L73

When a new feature module is bootstrapped, it isn't bootstraping with an initial state of what is currently in the state, it bootstraps with an initial state of what is provided via the INITIAL_STATE injectable. As such, when NGRX performs this bootstrapping, this lib comes along a little later and rebuilds it, or merges it.

As for using the feature key to selectivity refresh the data. At first swag that seems like an option, I'm mostly extending and correcting existing functionality and this seemed like the smallest touch to correct the issue.

@voidek-work I'm glad that fixes it for you, but what was the localStorageSync config that wasn't working? That is the only way I can fix it :D

[voidek-work]

@bmalinconico If I get you right...
this working:

export function localStorageSyncReducer(
  reducer: ActionReducer<any>,
): ActionReducer<any> {
  return localStorageSync({ keys: ['authContainer', 'layout', 'admin'], removeOnUndefined: true, rehydrate: true })(reducer);
}

not working:

export function localStorageSyncReducer(
  reducer: ActionReducer<any>,
): ActionReducer<any> {
  return localStorageSync({ keys: ['authContainer', 'admin'], removeOnUndefined: true, rehydrate: true })(reducer);
}

layout - state not lazy feature module.
authContainer, admin - state lazy loading feature module.
for example auth module.

@NgModule({
  imports: [
    ReactiveFormsModule,
    FormsModule,
    MaterialModule,
    CommonModule,
    AuthRoutingModule,
    TextMaskModule,
  ],
  declarations: COMPONENTS,
  exports: COMPONENTS,
})
export class AuthModule {
  static forRoot(): ModuleWithProviders {
    return {
      ngModule: RootAuthModule,
    };
  }
}

@NgModule({
  imports: [
    AuthModule,
    StoreModule.forFeature('authContainer', reducersAuth),
    EffectsModule.forFeature([AuthEffects, VerifyEffects, IdentityEffects, BanksEffects, StepEffects]),
  ],
})
export class RootAuthModule {}

if I rehydrate only lazy modules, then state reset to default.

[bmalinconico]

@voidek-work I don't see any reason in my patch for why the lack of the 'layout' key itself is special.

[voidek-work]

@bmalinconico
Unfortunately, I'm just getting acquainted with the technology, so I'm not sure exactly what the problem is. But the essence I described. In any case, thanks for the fixes

[ilennert]

Basically this renders this mechanism for restoring my state, useless... I don't understand what is happening but I know this is the cause. If I remove the persistence my problem goes away, unfortunately, so does my persisted state... Is anyone planing on fixing this problem?

[bmalinconico]

@ilennert I have been using my fork in production for a few months now and it is working fine. Please give it a try and report back any issues you encounter and I'll make an effort to fix them.

The collaborators are not really responding to this PR...

[pedrofurtado]

Is there any workaround for this scenario?

[bmalinconico]

@pedrofurtado Please see my fork and report back issues with it.

[ilennert]

As of 2 months ago the fork you provided me didn't work, had you presently updated?

[pedrofurtado]

@bmalinconico Ok, thanks for feedback! I will test the fork 👍

[zack9433]

#114 this pr causes a side effect when lazy load feature module happen.

Using deep merge return a new state object will trigger unexpected changing detection.

There is a demo:
https://angular-v5rdlk.stackblitz.io

Printing test state change twice in the console when you click account page link.

[chikakow]

So I have main root state and a lot lazy loaded modules with feature states. I just need to store root state on local storage. So I'm doing
return localStorageSync({keys: ['appState'], rehydrate: true})(reducer);

But I realized this breaks lazy loaded modules states. When I refresh a container component page that dispatches a bunch of load actions, the state should be loaded but instead state replaced with initial state.

If I remove rehydrate: true, then after the refresh the state is loaded with my stuff and not replaced with initial state.

But I need to keep rehydrate true there because I only fill the appState when users log in and the app state needs to be carried all across other modules and after refreshes.

Work around is to put all other lazy modules with feature states in the key array.
keys: ['appState', 'lazyState1', 'lazyState2', 'lazyState3' ...]
But I don't want to keep things that I don't need in localStorage.

This bug report sounds like the issue that I am having.

[Empereol]

@chikakow , are you registering the localStorageSyncReducer as part of the meta reducers on your root store module (ie: StoreModule.forRoot(reducers, {metaReducers: [localStorageSyncReducer]})?

I was having this issue when I was adding the localStorageSyncReducer to the meta reducers of a StoreModule.forChild(). Once moving it to the root store, it worked fine and storing just the keys I need.

[ilennert]

@Empereol, Yes ths was exactly my problem and moving it to "forRoot" StoreModule has fixed my code!

[ivayloc]

I am using ngrx-store-localstorage with NX workspace and I tried all workarounds but still can't make it work with lazyloaded modules in the case when I want to rehydrate specific slices from the state, on page refresh the initial state is being replaced by the saved slices from the localstorage.

The root store is called merchant and the feature store is called onboarding.

This is my setup:

merchant.meta-reducers.ts - for the host app

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);
  }
  return state;
};

export function sessionStorageSyncReducer(
  reducer: ActionReducer<any>
): ActionReducer<any> {
  return localStorageSync({
    keys: [
      {
        merchant: [
          { authProcess: ['id'] },
          { stepTasks: [{ 0: ['id'] }] },
          { registrationDetails: ['email'] },
        ],
      },
      {
        onboarding: [
          { onboardingProcess: ['id'] },
          { stepTasks: [{ 0: ['id'] }] },
        ],
      },
    ],
    rehydrate: true,
    removeOnUndefined: true,
    storage: sessionStorage,
    mergeReducer,
  })(reducer);

app.module.ts - host app

    StoreModule.forRoot(
      { merchant: merchantsReducer },
      {
        metaReducers: [sessionStorageSyncReducer],
        runtimeChecks: {
          strictActionImmutability: true,
          strictStateImmutability: true,
        },
      }
    ),

entry.module.ts

    StoreModule.forFeature(
      'onboarding',
      onboardingReducer,
      {
        metaReducers: [sessionStorageSyncReducer],
        initialState,
      }
    ),

onboarding.meta-reducer.ts - metareducer for the lazyloaded module

export function sessionStorageSyncReducer(
  reducer: ActionReducer<any>
): ActionReducer<any> {
  return localStorageSync({
    keys: [
      {
        onboarding: [
          { onboardingProcess: ['id'] },
          { stepTasks: [{ 0: ['id'] }] },
        ],
      },
    ],
    rehydrate: true,
    storage: sessionStorage,
  })(reducer);
}

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