ngrx / platform

Reactive State for Angular
https://ngrx.io
Other
8.03k stars 1.97k forks source link

metaReducer typing expects ActionReducer array #170

Closed luchsamapparat closed 7 years ago

luchsamapparat commented 7 years ago

According to the documentation, the metaReducer property of the store configuration expects an array of functions that accept the reducer and return a reducer function.

function debug(reducer) {
  return function(state, action) {
    console.log('state', state);
    console.log('action', action);

    return reducer(state, action);
  }
}

const metaReducers = [debug];

@NgModule({
  imports: [
    StoreModule.forRoot(reducers, { metaReducers })
  ]
})

https://github.com/ngrx/platform/blob/master/docs/store/api.md#meta-reducers

I tested this using this code...

        StoreModule.forRoot(REDUCERS_TOKEN, {
            metaReducers: [
                (...args) => console.log(args)
            ]
        }),

... and the browser console confirms this:

image

However, when I want to actually implement the code from the documentation, I get typing issues.

[ts]
Argument of type '{ metaReducers: ((reducer: AppState) => (state: any, action: any) => any)[]; }' is not assignable to parameter of type 'StoreConfig<AppState, Action>'.
  Types of property 'metaReducers' are incompatible.
    Type '((reducer: AppState) => (state: any, action: any) => any)[]' is not assignable to type 'ActionReducer<AppState, Action>[]'.
      Type '(reducer: AppState) => (state: any, action: any) => any' is not assignable to type 'ActionReducer<AppState, Action>'.
        Type '(state: any, action: any) => any' is not assignable to type 'AppState'.

From what I see, the StoreConfig interface incorrectly defines metaReducers as ActionReducer<any, any>[];

export interface StoreFeature<T, V extends Action = Action> {  key: string;
  reducers: ActionReducerMap<T, V> | ActionReducer<T, V>;
  reducerFactory: ActionReducerFactory<T, V>;
  initialState?: InitialState<T>;
  metaReducers?: ActionReducer<any, any>[];
}

https://github.com/ngrx/platform/blob/d2295c78a29635d40b70b44155d7f0e783310c0b/modules/store/src/models.ts#L29

From what I understand the correct typing should be ActionReducer<T, V>[].

Can anyone confirm this?

Update

Here's a repo to reproduce this issue:

https://github.com/luchsamapparat/ngrx-meta-reducer-issue

Just run npm install and then npm start

luchsamapparat commented 7 years ago

BTW, defining meta reducers like this also lead to AOT issues:

ERROR in Error encountered resolving symbol values statically. Function calls are not supported. Consider replacing the function or lambda with a reference to an exported function (position 35:17 in the original .ts file), resolving symbol AppModule in C:/Users/mal/Projekte/etpdash/src/app/app.module.ts

Does anyone know how to get around this issue?

brandonroberts commented 7 years ago

You can't use anonymous arrow functions with AOT. The function has to be a named export. Meta reducers take a reducer and return a new reducer function like the debug example. The composition is from right to left on the items in the array. Underneath, combineReducers is being used to form your top level reducer, then it goes through each meta reducer. The typings issue does need to be corrected.

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

            return reducer(state, action);
          };
        }

@NgModule({
  declarations: [
    AppComponent
  ],
  imports: [
    BrowserModule,
    StoreModule.forRoot(REDUCERS_TOKEN, {
      metaReducers: [
        // (...args) => console.log(args)
        logger
      ]
    })
  ],
  providers: [
    { provide: REDUCERS_TOKEN, useValue: reducers }
  ],
  bootstrap: [AppComponent]
})
export class AppModule { }
luchsamapparat commented 7 years ago

It works indeed when the logger is moved to a separate module. I don't know why the typescript compiler doesn't complain about it as from what I see the types are still incompatible.

It originally stumbled upon this issue when trying to integrate btroncone/ngrx-store-logger which exposes the same API when calling storeLogger()

export declare const storeLogger: (opts?: LoggerOptions) => (reducer: Function) => (state: any, action: any) => any;

So even though logger from the example above as well as the return value from storeLogger() call return the same type, the latter fails with the error mentioned above.

I updated the repo to reflect this.

brandonroberts commented 7 years ago

The same rule still applies with static analysis, where the code needs to be read without being executed. In order to use storeLogger, you need to wrap it in an exported function.

import { storeLogger } from 'ngrx-store-logger';

// console.log all actions
export function logger(reducer: ActionReducer<State>): ActionReducer<any, any> {
  return storeLogger()(reducer);
}

export const metaReducers: ActionReducer<any, any>[] = !environment.production
  ? [logger]
  : [];
saulshanabrook commented 7 years ago

@brandonroberts I am also confused by the type signature. Could you explain why it is correct? I feel like a meta reducer should be a function that takes in an ActionReducer and returns another one, and not be in itself.

GiuseppePiscopo commented 7 years ago

I created this issue before actually noticing the last comment here

GiuseppePiscopo commented 7 years ago

BTW I was able to make our app work with this meta-reducer:

function consoleLogEnhancer(reducer) {

  function loggingReducer(state, action) {
    //console.log('state', state);
    console.log('action', action);

    return reducer(state, action);
  }

  return loggingReducer;
}

while the following causes errors at runtime, it seems like is messing up the state passed around:

function storeLoggerEnhancer(reducer) {

  function storeLoggingReducer(state, action) {
    const opts = {
      //timestamp: false,
    };

    return storeLogger(opts)(reducer);
  }

  return storeLoggingReducer;
}
karlhaas commented 7 years ago

@brandonroberts when adding the meta reducer as you mentioned I get the following error:

image

this started to happen when using inject tokens for the reducers and updating to the latest version of ngrx.

When using the following meta reducer:

export function logger2(reducer: ActionReducer<CoreState>): ActionReducer<any, any> {
  return function(state: CoreState, action: any): CoreState {
    console.group(action ? action.type : 'undefined');
    console.log('state', state);
    console.log('action', action);
    const newState = reducer(state, action);
    console.log('new state', newState);
    console.groupEnd();
    return newState;
  };
}

the meta reducer is called with action undefined:

image

My meta reducers: export const META_REDUCERS: ActionReducer<any, any>[] = !environment.production ? [logger2] : [];

App-Module: StoreModule.forRoot(REDUCER_TOKEN, {metaReducers: META_REDUCERS}),

Any suggestions?

brandonroberts commented 7 years ago

@karlhaas I think this issue was introduced by: https://github.com/ngrx/platform/pull/252 if you are running the nightlies. We are looking into it and should have a resolution soon.

brandonroberts commented 7 years ago

@saulshanabrook @GiuseppePiscopo the type signature was incorrect for the meta-reducer array and has been resolved here #270

karlhaas commented 7 years ago

@brandonroberts I got the latest version including the MetaReducer type and use the following code:

export function logger(reducer: ActionReducer<CoreState>): ActionReducer<CoreState> {
  console.log('reducer', reducer);
  return function(state: CoreState, action: FbrStoreAction): CoreState {
    console.log(`action ${action} state`, state);
    const newState = reducer(state, action);
    console.log(`${action.type}: state ${state} action ${action} newState ${newState}`);
    return newState;
  };
}

export const META_REDUCERS: MetaReducer<CoreState>[] = !environment.production ? [logger] : [];

It fails with the same error:

image

brandonroberts commented 7 years ago

@karlhaas the action is undefined. Its a bug introduced from #252. We'll revert that commit with another solution.

karlhaas commented 7 years ago

thx @brandonroberts, it seems to be fixed

kuncevic commented 7 years ago

Any ideas on that https://github.com/btroncone/ngrx-store-localstorage/issues/59 ? Just trying to setup ngrx-store-localstorage getting error Types of property 'metaReducers' are incompatible. ActionReducer<any, any>[]' is not assignable to type 'MetaReducer<State, Action>[]'.

danielstern commented 6 years ago

This issue appears to still be around in version 5.2.0

omaracrystal commented 4 years ago

I keep running into this type check error TS2345

CODE:

index.ts (for state)

export function logger(reducer:ActionReducer<any>)
  : ActionReducer<any> {
  return (state, action) => {
    console.log("state: ", state);
    console.log("action", action);

    return reducer(state, action);
  }
}

export const metaReducers: MetaReducer<IState>[] =
  !environment.production ? [logger] : [];

core.module.ts:

    StoreModule.forRoot(reducers, {
      metaReducers,
      runtimeChecks : {
        strictStateImmutability: true,
        strictActionImmutability: true,
        strictActionSerializability: true,
        strictStateSerializability: true,
      }
    }),

ERROR:

Argument of type '{ metaReducers: MetaReducer<AppState, Action>[]; 
runtimeChecks: { strictStateImmutability: boolean; strictActionImmutability: boolean;
strictActionSerializability: boolean; strictStateSerializability: boolean; }; }' 
is not assignable to parameter of type 'StoreConfig<AppState, Action>'.
Object literal may only specify known properties, and 'runtimeChecks' 
does not exist in type 'StoreConfig<AppState, Action>'.