Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
MasGaNo opened this issue Apr 22, 2019 · 8 comments · Fixed by #142
Closed

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

MasGaNo opened this issue Apr 22, 2019 · 8 comments · Fixed by #142

Comments

@MasGaNo
Copy link

MasGaNo commented Apr 22, 2019

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

  • Typesafe-Actions Version: 4.1.0
  • TypeScript Version: 3.4.3

PS: Good job for this release 👍

@piotrwitek
Copy link
Owner

piotrwitek commented Apr 22, 2019

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
Copy link
Owner

piotrwitek commented Apr 22, 2019

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

@MasGaNo
Copy link
Author

MasGaNo commented Apr 22, 2019

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
Copy link
Owner

piotrwitek commented Apr 22, 2019

@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
Copy link
Author

MasGaNo commented Apr 22, 2019

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

@MasGaNo
Copy link
Author

MasGaNo commented Apr 22, 2019

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
Copy link
Owner

@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
Copy link
Author

MasGaNo commented Apr 22, 2019

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.

@piotrwitek piotrwitek changed the title action type when providing initialHandlers for createReducer Object map style can not use getType to get type to be used as map key Nov 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants