piotrwitek / typesafe-actions

Typesafe utilities for "action-creators" in Redux / Flux Architecture
https://codesandbox.io/s/github/piotrwitek/typesafe-actions/tree/master/codesandbox
MIT License
2.41k stars 98 forks source link

Object map style can not use getType to get type to be used as map key #140

Closed MasGaNo closed 5 years ago

MasGaNo commented 5 years ago

Description

Hey @piotrwitek Just try out the latest version and it's a real improvement. I like this idea, it's very flexible and it simplify more the way to organize file.

I just have a small issue with the initialHandlers approach, the type of the action is not provided anymore. Works perfectly with the handleAction approach.

Related issues

106

Steps to Reproduce

Consider we have this action:

export const ExampleStoreActions = {
  First: createStandardAction("ExampleActions/First")<{
    foo: string
    bar: string
  }>(),
}
export const ExampleStoreReducers = createReducer({foo: ''})
    .handleAction(ExampleStoreActions.First, (state, action) => {
        action.payload.bar; // Works perfectly
        return {
            ...state
        }
    });
createReducer({foo: ''}, {
    [getType(ExampleStoreActions.First)]: (state, action) => {
        // action.payload. // no typing hint for action
        return {
            ...state
        }
    }
});

createReducer({foo: ''}, {
    "ExampleActions/First": (state, action) => {
        // action.payload. // neither here
        return {
            ...state
        }
    }
});

CodeSandbox to help you reproduce issue in isolation https://codesandbox.io/s/64ow9qxl3z

Expected behavior

Having the action correctly type like for handleAction.

Suggested solution(s)

Can investigate. Correct me if I'm wrong, for the getType approach, we need to wait the fix from TypeScript. But for explicit constant approach, should display the correct typing, shouldn't it?

Project Dependencies

PS: Good job for this release 👍

piotrwitek commented 5 years ago

Hey @MasGaNo Thanks for the report, ~it doesn't work because you have not followed tutorial.~

~If you're not using RootAction extension method, you need to add generic type arguments explicitely~

piotrwitek commented 5 years ago

@MasGaNo Sorry my bad, I was too quick, I just noticed it is there :)

MasGaNo commented 5 years ago

Yeah, sorry, I tried to summarize, but yeah, I provided the RootAction, and this is my point when I said it's a good idea to keep it flexible with the RootAction approach or the generic one 🙂

But yeah, which both approach, I face the same problem for action type.

piotrwitek commented 5 years ago

@MasGaNo you're right I have reproduced it and added a new test case, I think I can make it work but cannot confirm right now as I need some time to investigate.

MasGaNo commented 5 years ago

Sure no problem 🙂 I have some time before my next meeting, I'm trying to figure out something also 😉

MasGaNo commented 5 years ago

Quick workaround:

// typesafe-actions/dist/create-reducer.d.ts

// ...
declare type GetAction<T extends Action, P extends T['type']> = T extends { type: P } ? T : never;

declare type InitialHandler<TState, TRootAction extends Action> = {
    [P in TRootAction['type']]?: (state: TState, action: GetAction<TRootAction, P>) => TState;
}

export declare function createReducer<TState, TRootAction extends Action = RootAction>(initialState: TState, initialHandlers?: InitialHandler<TState, TRootAction> /*Record<RootAction['type'], (state: TState, action: RootAction) => TState>*/): Reducer<TState, TRootAction> & {
    handlers: {
        readonly [x: string]: (state: TState, action: any) => TState;
    };
    readonly handleAction: HandleActionChainApi<TState, TRootAction, TRootAction>;
};

I replaced the Record type for initalHandler by what I used in my implementation, and it works for constant, but we come back with the problem of getType and the implicit any: image

piotrwitek commented 5 years ago

@MasGaNo thanks it works great 👍 Fix is on the way!

Regarding getType, we need to open a separate issue tracking a previously known TS compiler bug: Microsoft/TypeScript#29718

MasGaNo commented 5 years ago

I think a way to get ride the need of getType (and so, the problem of using it in the handlerMap approach) is to use the future Symbol.toPrimitive and/or Symbol.toStringTag: Microsoft/TypeScript/issues/4538 and Microsoft/TypeScript/issues/2361

Could be interesting to keep an eye on it.