Meta reducers block initial state

[rjokelai]

I'm submitting a...

[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report  
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request

What is the current behavior?

If adding a single meta reducer, the initial state from the config fails to populate.

Expected behavior:

The initial state should be set in both cases.

Minimal reproduction of the problem with instructions:

Add the following test to modules.spec.ts. When adding the simplest no-op meta reducer, the initial state fails to populate.

  describe(`: With initial state`, () => {
    const initialState: RootState = { fruit: 'banana' };
    const reducerMap: ActionReducerMap<RootState> = {  fruit: rootFruitReducer };
    const noopMetaReducer = (r: any) => r;

    const testWithMetaReducers = (metaReducers: any[]) => () => {
      beforeEach(() => {
        TestBed.configureTestingModule({
          imports: [StoreModule.forRoot(reducerMap, { initialState, metaReducers })],
        });
        store = TestBed.get(Store);
      });
      it('should have initial state', () => {
        store.take(1).subscribe((s: any) => {
          expect(s).toEqual(initialState);
        });
      });
    };

    describe('should add initial state with no meta reducers', testWithMetaReducers([]));
    describe('should add initial state with a simple no-op meta reducer', testWithMetaReducers([noopMetaReducer]));
  });

Version of affected browser(s),operating system(s), npm, node and ngrx:

• @ngrx/store@4.0.2 / master branch

[rjokelai]

Seems that the problem is in the compose utility function that only passes on the first argument:

export function compose(...functions: any[]) {
  return function(arg: any) {
    if (functions.length === 0) {
      return arg;
    }

    const last = functions[functions.length - 1];
    const rest = functions.slice(0, -1);

    return rest.reduceRight((composed, fn) => fn(composed), last(arg));
  };
}

By changing it to the following, the test starts to pass

export function compose(...functions: any[]) {
  return function(...arg: any[]) {
    if (functions.length === 0) {
      return arg;
    }

    const last = functions[functions.length - 1];
    const rest = functions.slice(0, -1);

    return rest.reduceRight((composed, fn) => fn(composed), last(...arg));
  };
}

[rjokelai]

Looking better into it, it's probably the usage of the compose method in createReducerFactory that should be fixed. I'll make a PR

[rjokelai]

I added a couple of more tests to investigate the functionality. Seems that in the previous versions of @ngrx/store, we created the root reducers ourself like this:

const rootReducer = compose(
  metaReducer1,
  metaReducer2,
  combineReducers
)(reducerMap)

There was no problem in using the compose function then as the combineReducers only accepted one argument. Now, in the current version the combineReducers accepts two, the latter of which is the initialState, which incidentally gets dropped by the compose function. It has been working for cases without meta reducers since createReducerFactory uses the compose method only if metaReducers.length > 0 (see here).

This will start working if we add the initially suggested ...args: any[] to the compose method. The downside is that we lose the type safety we get from the compose method providing only one typed argument for the final function. Other way (I'll do a PR of this), is to change how the compose method is used and do a double type assertion of the returned function.

[RaphaelJenni]

I have the same issue. But it isn't fixed for the current version (4.0.3) In my project, I have an initial state and a metaReducer. If I want to import them in the forRoot function in the app module it doesn't work. But if I remove the meta reducer it works again. The strange thing is, that the config is designed to accept a metaReducer and an initialState, but doesn't.

My Code:

@NgModule({
  declarations: [
    AppComponent,
    MainComponent
  ],
  bootstrap: [AppComponent],
  providers: [
    Dialogs,
    {provide: NgModuleFactoryLoader, useClass: NSModuleFactoryLoader}
  ],
  imports: [
    NativeScriptModule,
    NativeScriptFormsModule,
    ReactiveFormsModule,
    StoreModule.forRoot(reducers, {initialState: initialState, metaReducers: metaReducers}),
    EffectsModule.forRoot([StoreEffects]),
    CoreModule.forRoot(),
    SharedModule,
    AppRoutingModule
  ],
  schemas: [
    NO_ERRORS_SCHEMA
  ]
})
export class AppModule {
}

export function logger(reducer: ActionReducer<State>): ActionReducer<State> {
  return function(state: State, action: any): State {
    console.log('state', JSON.stringify(state));
    console.log('action', JSON.stringify(action));

    return reducer(state, action);
  };
}

export const metaReducers: MetaReducer<State>[] = [logger];

export const initialState = function () {
  const secureStorage = new SecureStorage();
  return JSON.parse(secureStorage.getSync({ key: StoreEffects.secureStorageStoreKey }));
};

[jpduckwo]

Can this be reopened? Still an issue I think. Cheers, Joel

[Matmo10]

Yeah, I'm using ngrx-store-localstorage by @btroncone . It looks here like this metareducer is designed to accept the initial state and override it as necessary...but for some reason the initial state isn't passed into the metareducer whenever I debug.

[Matmo10]

Actually I just noticed that this was closed with a fix that went into v4.0.5, but v.4.0.3 is the highest version available on npm?

[brandonroberts]

@Matmo10 4.0.3 is the latest version of @ngrx/store on npm. Will you open a new issue with a reproduction against the latest version?

[Matmo10]

@brandonroberts Sure - opened https://github.com/ngrx/platform/issues/477

[jpduckwo]

Just FYI for anyone who's interested. I work around this by setting the initial state in the reducer. For me it actually makes more sense like this anyway.

export const initialState: State = {
  loading: false,
  something: new Map,
};

export function reducer(state = initialState, action: something.Actions): State {
  switch (action.type) {
    case something.LOAD:
      return {
        ...state,
        loading: true
      };
...
    default: {
      return state;
    }
  }
}