ngrx payload in reducer action not compiling

3.4k views Asked by At

I am trying to migrate my angular 2 app to angular 4 and ngrx 4.

I am tackling a weird typescript compilation problem that I didn't have before this update. I work in an off line environment, so I cant share the exact code here.

I looked for the problem online, the closest thing I could find was this question: question about payload types The code in this question is somehow similar to what im trying to do, but the difference is that I removed the ActionTypes object from the action (which is the suggest answer for that question) but it did not resolve my compilation error.

I can't put the exact code im tackling with here, but im completely basing my code on the ngrx example ngrx official example on github

Lets look for example on the book reducer and book actions book actions book reducer

My code is basically the same, and the error will occur for each reducer case (in the switch case) when I try to get the payload out of the action, it tells me something like:

Type 'string | Book | Book[]' is not assignable to type 'Book'. Type 'string' is not assignable to type 'Book'.

(In my code, instead of Book I have different classes, but its pretty much the same idea)

For now i'm solving the errors by casting the action type from A | B | B to be the specific action type corresponding to the case in the switch (using the as typescript keyword). This solution feels bad and hacky, I would like to know if there is a better one.

The versions im using: typescript: 2.4.1 (I suspect that it might be connected to ngrx examples not using the same updated typescript version as me) ngrx@store: 4.0.3 ngrx@core: 1.2.0 angular: (latest as of 17.9.17, there are many packages inside angular, and I dont think it accually matters which angular im using for this error but I say this just to be on the safe side) typescript: (2.4.1) (I read the ngrx official migration doc about needing to upgrade typescript, and so I did)


UPDATE: Since I got an answer that isn't relevant to my problem and also I've been asked, here is the part of my code that fails (I cant post the exact post but im hand copying the relevant parts):

mission-reducer.ts:

import * as MissionActionsFile from "./mission-actions";

export type State = .... // doesnt matter

const initialState: State = // doesnt matter as well

export function reducer(state: State = initialState, action: 
    MissionActionsFile.Actions): State {

    switch(action.type) {

        case MissionActionsFile.ADD_SUCCESS: 
        {
            const mission: Mission = action.payload; // This line generates the 
            // following error: 
            // Type 'string | number | Mission | MissionRealTime | ...' is not 
            // assignable 
            // to type 'Mission'. Type 'string' is not assignable to type 'Mission'

            return {}; // This doesnt matter too
        }

        // There are more cases and a deafult one, but that doesn't matter
    }
}

mission-actions.ts:

// I will only give the actual action defenition for AddSuccess and for AddFailed since we are using AddSuccess in the example and 
// addFailed is an example where the payload type is string, which according to the error is the problem, though
// there are more action types and other payload types with other classes as the quoted error implies.

export const ADD_SUCCESS: string = "[Mission] AddSuccess";
export const ADD_FAILED: string = "[Mission] AddFailed";

export class AddSuccessAction implements Action {
    public readonly type: string = ADD_SUCCESS;

    constructor(public payload: Mission) {}
}

export class AddFailedAction implements Action {
    public readonly type:string = ADD_FAILED;

    constructor(public payload: string) {}
}

// More actions are defined here but im not gonna copy all

...

export type Actions =
    AddSuccessAction |
    AddFailedAction;

So to conclude, I do see why it make sense for typescript to think the action payload type might be string | Mission | ... but in the ngrx example that I based this on, it seems typescript knows there to infer the specific type in the case, but for me that doesn't work for a reason im not understanding, might be something to do with me using typescript 2.4.1? not sure, need help with this

2

There are 2 answers

0
Aluan Haddad On BEST ANSWER

The problem is that the behavior of the code and its type annotations are at cross purposes with one another.

Actually, I would say that the code is excessively annotated.

Union types and the case analysis that they enable work via type inference and control flow based type analysis, two of TypeScript's most powerful features. The process by which the language eliminates possibilities from the union is known as narrowing.

As the code suggests, a union type can be broken down into its constituents by performing a value test against a property known as the discriminant.

A discriminant is a property whose type has a finite set of possible values with each value generally corresponding to a case of the union.

The type string is not a valid discriminant, but the type "hello world" is because, as a supertype of all possible strings, a union of string types including string collapses to just string. For example, the type string | "hello world" is exactly the type string.

When we define a const or a readonly property in TypeScript, the compiler infers its type to be the type of the initializing literal.

Consider:

const kind = "first";

Here the type of kind is not string but "first".

Similarly, given

class Kindred {
  readonly kind = "first";
}

The type of the property kind is not string but "first".

In our code, the discriminant is a property named type that is defined by each constituent of the union.

But while, you have correctly provided unique values for each member, you have over-specified them with type annotations that prevent narrowing.

What you have:

export class AddSuccessAction {
    public readonly type: string = ADD_SUCCESS;

    constructor(public payload: Mission) {}
}

export class AddFailedAction {
    public readonly type: string = ADD_FAILED;

    constructor(public payload: string) {}
}

What you want:

export class AddSuccessAction {
    readonly type = ADD_SUCCESS;

    constructor(public payload: Mission) {}
}

export class AddFailedAction {
    readonly type = ADD_FAILED;

    constructor(public payload: string) {}
}

A working switch

switch (action.type) {
  // action.type is `"[Mission] AddSuccess" | "[Mission] AddFailed"`
  // action.payload is `string | Mission`
  case missionActions.ADD_SUCCESS:
    // action.type is `"[Mission] AddSuccess"`
    // action.payload is `Mission`
    const mission = action.payload;
}

Why this matters:

A string literal type is a common and idiomatic way discriminate the possibilities of a union but, by declaring the implementing properties as being of type string, a type that is a super type of all string literal types, we inhibited type inference and thereby prevented narrowing. Note that string is not a type that can be narrowed from.

In general, it is best to leverage TypeScript's type inference when a value has an initializer. In addition to enabling the expected scenario, you will be impressed by the amount of bugs that the type inferencer will catch. We should not tell the compiler more than it needs to know unless we intentionally want a type that is more general than what it infers. With --noImplicitAny it will always let us know when we need to specify extra information in the form of type annotations.

Remarks:

You can technically specify the types and still retain the narrowing behavior by specifying the literal value as the type of the const or the readonly property. However, this increases maintenance costs and is rather redundant.

For example, the following is valid:

export const ADD_FAILED: "[Mission] AddFailed" = "[Mission] AddFailed";

but you are just repeating yourself unnecessarily.

In their answer, ilyabasiuk provides some great reference links on the use of literal types specifically with ngrx and how it evolved over recent iterations of the TypeScript language.

To understand why literal types are inferred in immutable positions, consider that it enables powerful static analysis and thus better error detection.

Consider:

type Direction = "N" | "E" | "S" | "W";

declare function getDirection(): Direction;

const currentDirection = getDirection();

if (currentDirection === "N") { // currentDirection is "N" | "E" | "S" | "W"

}
// Error: this is impossible, unreachable code as currentDirection is "E" | "S" | "W"
// the compiler knows and will give us an error here.
else if (currentDirection === "N") {

}
0
ilyabasiuk On

Here is really good description of migration, that you are trying to do https://github.com/ngrx/example-app/pull/88#issuecomment-272623083

You have to do next changes:

  • delete type definition for action const string;
  • delete type definition for type property;
  • add /* tslint:disable:typedef*/ in the beginning of file if your tslint configured to not allow missed type definition (yep it looks not very good, but if I have to choose between strong rules for type definition in action files an support types in reducers I will definitely choose second one);

So You will get something like:

/* tslint:disable:typedef*/
export const ADD_SUCCESS = "[Mission] AddSuccess";
export const ADD_FAILED = "[Mission] AddFailed";

export class AddSuccessAction implements Action {
   public readonly type = ADD_SUCCESS;

   constructor(public payload: Mission) {}
}

export class AddFailedAction implements Action {
   public readonly type = ADD_FAILED;

   constructor(public payload: string) {}
}

// More actions are defined here but im not gonna copy all

...

export type Actions =
    AddSuccessAction |
    AddFailedAction;

Aluan Haddad has rpovided good explanation why it works so in post above.