Platform: How do I use action.payload in my reducers?

Created on 25 Jul 2017  路  10Comments  路  Source: ngrx/platform

I'm submitting a...


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

What is the current behavior?

action.payload gives a typscript error in my reducers

Expected behavior:

How do I use a payload in the reducer? So confused.

Most helpful comment

@karptonite @brandonroberts what I ended up doing is explicitly casting my actions inside the reducer.
https://gist.github.com/StevenLiekens/a2aff8366e5f7efdbbf5b6f5b99d83c5

I don't use TypeScript's type inference the way you propose because I think it's too much work to maintain the proposed union types in a large application. Explicit casts are less work. Especially when only few actions actually have payloads.

All 10 comments

Thank you.

@brandonroberts sorry but that page is confusing.

The example code exports a union type named All, but it does this inside a file named counter.actions.ts instead of maybe an index file.

export type All = Increment | Decrement | Reset;

Does "All" mean "all Counter actions" or "all actions in my app"?

Either way I dislike having to maintain a list of actions in my app. This is fine when you only have 4 actions but we're not building simple counters. No offense intended.

@StevenLiekens none taken. It means "all Counter Actions" The list of actions isn't required but if you don't maintain the list here, you'll have to provide the union list of actions in your reducer for type safety. What would be more clear?

export type All = Increment | Decrement | Reset;

OR

export type Actions = Increment | Decrement | Reset;

The latter would be more clear when importing as

import * as Counter from './counter.actions';

reducer(state: State, action: Counter.Actions) => State;

Doesn't that go against the idea that every action passes through every reducer?

What I mean is that action is annotated with type Counter.Actions when it can actually be any Action. That looks like an inside out liskov violation. I'm not sure what else to call it.

@StevenLiekens it is a bit weird, but think of it this way: every action DOES pass through every reducer, but the typing tries to enforce that the reducer makes no changes to the state for actions that are not in the specified union. For all types that aren't members of the union, you just return state. So yes, the type restriction lies about what actions it is receiving, but functionally, it is doing nothing with actions that aren't in the list--which _should_ be functionally identical (aside from a few extra cycles) to the function not running for those actions.

It's that type union that lets your case statements discriminate among the action types, so that the compiler can infer the Action type (and payload, etc, if you have specified). It is certainly a convenient fictionl

@karptonite then how do you handle actions for cross-cutting concerns?

In my app I have a LOGOUT action that is used to delete sensitive info from the store. Using the example counter app, that means resetting the count to its initial value when a LOGOUT action is observed.

export const initialState: number = 0;

export function reducer(state = initialState, action: CounterActions.All): State {
  if (action.type === AuthenticationActions.LOGOUT) {
    // Inferred type of action here is 'never'
    return initialState;
  }
  switch(action.type) {
    case CounterActions.INCREMENT: {
      return state + 1;
    }

    case CounterActions.DECREMENT: {
      return state - 1;
    }

    case CounterActions.RESET: {
      return action.payload; // typed to number
    }

    default: {
      return state;
    }
  }
}

When I do a comparison between CounterActions.All and my new LOGOUT action, TypeScript tells me that this can never happen.

The never type represents the type of values that never occur.
...
Variables also acquire the type never when narrowed by any type guards that can never be true.
https://www.typescriptlang.org/docs/handbook/basic-types.html

To fix it, I would have to make the action argument a union type too. A union of unions.

function reducer(state: number, action: CounterActions.All | AuthenticationActions.AtLeastSome): State { ... }

I can't tell you exactly what is wrong with any of this but it _feels_ like this is bad code.

Two ways to handle it: a union of unions, like you mentioned, or you could pull out a separate action to reset the counter to the initial state, and emit it from the effect that also emits the logout action. I ran into the same issue; I'd say how you handle it depends on how cross cutting the concern is. You don't want a reducer that has a dozen action types it has to know about.

@karptonite @brandonroberts what I ended up doing is explicitly casting my actions inside the reducer.
https://gist.github.com/StevenLiekens/a2aff8366e5f7efdbbf5b6f5b99d83c5

I don't use TypeScript's type inference the way you propose because I think it's too much work to maintain the proposed union types in a large application. Explicit casts are less work. Especially when only few actions actually have payloads.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

oxiumio picture oxiumio  路  3Comments

gperdomor picture gperdomor  路  3Comments

brandonroberts picture brandonroberts  路  3Comments

bhaidar picture bhaidar  路  3Comments

mappedinn picture mappedinn  路  3Comments