Add createReducer - a typesafe reducer factory using object map (including getType() support) #106

Ok good small-news: I fix the error on the create-reducers.ts part by enforcing the definition and removing the any part f779b6d

Hey, I suppose this issue is related right? https://github.com/piotrwitek/react-redux-typescript-guide/issues/121

So you want to include createReducer function into the library right?

I implemented it but there is an issue with computed properties assertions which I believe is a bug in TypeScript.

Maybe you can investigate it further in TypeScript repo issue as I want to focus on clearing out the technical debt before adding new features.

Implemented here: https://github.com/piotrwitek/react-redux-typescript-guide/commit/6502fd4e7fda78b5aa30d8e6435eb8092f9baefa#diff-8ed224eb6b7ff91fd900f963dc88b328

Ah true! I didn't see this issue in your other repository.

That's indeed the idea for the moment (I have another suggestion to improve typesafe-actions, but probably need an improvement from the TypeScript team).

Ok, I don't know if you already check my repository, but we have more or less the same problem.

If I use the full path string constant as a key, it works like a charn, but if I use getType, there are some errors.

Yeah I agree, from the example above I think the problem is not in the getType, but IMO it is a type assertion bug in TypeScript. Please check in TypeScript repo.

Yes, I'm trying to figure out a small test example for them 👍

@MasGaNo please use this: http://www.typescriptlang.org/play/#src=%2F%2F%20PLEASE%20ENABLE%20%3D%3E%20noImplicitAny%20%0D%0A%0D%0Atype%20Keys%20%3D%20'a'%20%7C%20'b'%20%7C%20'c'%3B%0D%0Atype%20Shape%20%3D%20%7B%20%5BK%20in%20Keys%5D%3F%3A%20K%20extends%20string%20%3F%20(key%3A%20K)%20%3D%3E%20K%20%3A%20never%20%7D%3B%0D%0A%0D%0Aconst%20getC%20%3D%20()%20%3D%3E%20'c'%20as%20'c'%3B%0D%0A%0D%0Aexport%20const%20obj%3A%20Shape%20%3D%20%7B%0D%0A%20%20%5B'a'%5D%3A%20key%20%3D%3E%20key%2C%20%2F%2F%20OK%20%3D%3E%20inferred%20correctly%0D%0A%20%20%5B'b'%20as%20'b'%5D%3A%20key%20%3D%3E%20key%2C%20%2F%2F%20Error%20%3D%3E%20inference%20broken%20%22any%22%20type%0D%0A%20%20%5BgetC()%5D%3A%20key%20%3D%3E%20key%2C%20%2F%2F%20Error%20%3D%3E%20inference%20broken%20%22any%22%20type%0D%0A%7D%3B%0D%0A

// PLEASE ENABLE => noImplicitAny 

type Keys = 'a' | 'b' | 'c';
type Shape = { [K in Keys]?: K extends string ? (key: K) => K : never };

const getC = () => 'c' as 'c';

export const obj: Shape = {
  ['a']: key => key, // OK => inferred correctly
  ['b' as 'b']: key => key, // Error => inference broken "any" type
  [getC()]: key => key, // Error => inference broken "any" type
};

Ah! nice 🙂

I will post it right now in there repository 👍

Thanks a lot @MasGaNo! Please link to this issue so we can track it here.

Here we go: Microsoft/TypeScript#29718

Blocked by TypeScript bug, we'll get back to this idea when it's fixed upstream.

Well, I hope it will be really fixed for TypeScript 3.4.0, as they are currently already working on the 3.3.3 (btw, almost complete).

Wait&see.

For the moment, there is still the workaround with constant, so it remains a good progress against the big switch statement.

a convenient example from redux-act

import { createReducer } from 'redux-act';
import * as act from './actions';

const someReducer = createReducer<SomeState>((on) => {
    on(act.someAction1, (state, initialOptions) => {
        return {
            ...state
        };
    });
    on(act.someAction2, (state, selectedFilterId) => {
        return selectedFilterId !== state.selectedFilterId
            ? {
                  ...state,
                  selectedFilterId,
              }
            : state;
    });
});

but not type safe :( (or I failed)

Hi @rifler It's indeed an interested approach.

But personally, I prefer to keep the ObjectMap approach because I like the idea to have my IDE tells me what are the non-implemented reducer, and help me to avoid duplicate redefinition.

Here I don't know if I overwrite the previous implementation or if it will be extended.

At least with this kind of implementation could be more useful IMHO:

type ReducerRegisterGlob<T> = {handlerRegister: ReducerRegister<T>};
type ReducerRegister<T> = <P extends T>(value: P, callback: Function) => ReducerRegisterGlob<Exclude<T, P>>;

function createReducers<T>(callback: (handlerRegister: ReducerRegister<T>) => void) {
    const reducers: any = {};

    function getNext<R>(): ReducerRegister<R> {
        return <P extends R>(value: P, callback: Function) => {
            reducers[value] = callback;
            return getNextGlob<Exclude<R, P>>();
        }
    }

    function getNextGlob<R>() {
        return {
            handlerRegister: getNext<R>()
        };
    }

    callback(getNext<T>());

    return reducers;
}

const enum Actions {
    Action1 = 'action1',
    Action2 = 'action2'
};

const reducers = createReducers<Actions>((handlerRegister) => {
    handlerRegister(Actions.Action1, () => {}) // 'action1'|'action2'
    .handlerRegister(Actions.Action2, () => {}) // 'action2'
    .handlerRegister(); // Error -> 'never' 
});

But you still cannot avoid your developer to perform something like:

const reducers = createReducers<Actions>((handlerRegister) => {
    handlerRegister(Actions.Action1, () => {}); // 'action1'|'action2'
    handlerRegister(Actions.Action2, () => {}); // 'action1'|'action2' again
    handlerRegister(Actions.Action1, () => {}); // 'action1'|'action2' again
});

Or... If you really want to constraint your developer, you can always use this approach:

type NoNever<T, A> = T extends never ? never : A;
type _ReducerRegister<T> = {
    register: NoNever<T, <P extends T>(value: P, callback: Function) => ReducerRegister<Exclude<T, P>>>;
    commit: () => any;
};
type ReducerRegister<T> = OmitByValue<_ReducerRegister<T>, never>;

function createReducers<T>() {
    const reducers: any = {};

    const commit = () => {
        return reducers;
    };

    function getNext<R>(): ReducerRegister<R> {
        return {
            // @ts-ignore
            register: <P extends R>(value: P, callback: Function) => {
                reducers[value] = callback;
                return getNext<Exclude<R, P>>();
            },
            commit
        };
    }

    return getNext<T>();
}

const enum Actions {
    Action1 = 'action1',
    Action2 = 'action2'
};

const reducers = createReducers<Actions>()
    .register(Actions.Action1, () => { }) // 'action1'|'action2'
    .register(Actions.Action2, () => { }) // 'action2'
    .commit(); // register is not available anymore

Here you can avoid at least the duplicate registration of your action. But it's definitely more verbose. I'm still looking for the Map approach 🙂

@issuehunt has funded $50.00 to this issue.

• Submit pull request via IssueHunt to receive this reward.
• Want to contribute? Chip in to this issue via IssueHunt.
• Checkout the IssueHunt Issue Explorer to see more funded issues.
• Need help from developers? Add your repository on IssueHunt to raise funds.

@piotrwitek et. al

I was able to find an approach that seems to work using a conditional type for the ReducerMap. I've done some basic testing against a reducer with 8 actions. I'm exclusively using PayloadActions in this reducer, so that's what you see here. I could expand this to support other action types in the branches of the conditionals (kinda ugly, maybe there's a nicer way to handle that). I'll open a PR with this, but would appreciate thoughts esp. on how to get rid of the any typecast that is also in the example posted above.

type Action = ActionType<typeof actions>;
type ReducerMap<S, A> = A extends PayloadAction<infer T, infer P>
  ? { [key in T]: (state: S, action: PayloadAction<T, P>) => S }
  : never;

function createReducer<S, A extends { type: string }>(
  initialState: S,
  handlers: ReducerMap<State, Action>,
) {
  return function reducer(state = initialState, action: A): S {
    if (handlers.hasOwnProperty(action.type)) {
      return (handlers as any)[action.type](state, action);
    } else {
      return state;
    }
  };
}

EDIT for some context: This accomplishes linking the specific action type key to the specific action, therefore in your reducer map object it will automatically differentiate, eg., payload types. The tradeoff is that the ReducerMap type definition will have to enumerate each type of action supported. I can't really see a way around that.

EDIT 2: @piotrwitek I guess I should ask before assuming... do you want the ReducerMap type in this library or are trying to keep reducer-related types out of this?

Hey @joefiorini 🙂

I don't know why you need the any key word, in my implementation, I really don't need it:

/**
 * Create a reducers from a defined handlerMap
 * @param initialState 
 * @param handlersMap 
 */
export function createReducers<T extends ActionType<ActionCreator<StringType>>, S>(
    initialState: S,
    handlersMap: {
        [P in GetActionType<T>]?: ActionReducer<S, T, P>;
    }
) {
    return function <P extends GetActionType<T>>(state: S = initialState, action: GetAction<T, P>) {
        if (handlersMap.hasOwnProperty(action.type)) {
            return handlersMap[action.type]!(state, action);
        }
        return state;
    }
}

Then, this solution works perfectly as soon as you specify explicitly the type of your action:

createReducers<ExampleStoreActionsType, ExampleStoreState>(ExampleStoreState, {
    "Example/ActionWithPayload": (state, action) => {
        action.payload.username
        return {
            ...state,
            username: action.payload.username,
            password: action.payload.password
        };
    }
});

But the thing is: I really don't like the constant approach 🙂 And that's precisely the concern of this issue, I want to have the action-helper approach instead:

createReducers<ExampleStoreActionsType, ExampleStoreState>(ExampleStoreState, {
    [getType(ExampleStoreActions.ActionWithPayload)]: (state, action) => {
        action.payload.username
        return {
            ...state,
            username: action.payload.username,
            password: action.payload.password
        };
    }
});

And here, it doesn't work:

But it's because of a limitation of TypeScript. That's why we created an issue in their side: Microsoft/TypeScript#29718

So don't hesitate to upvote, and share this issue with lot of people, as the TypeScript team can reconsider the priority of this feature.

@joefiorini I have tested your implementation but it doesn't fix the issues we had with my previous createReducers implementation that I did here: https://github.com/piotrwitek/typesafe-actions/issues/106#issuecomment-460104654

Could you tell us what do you think are the benefits of your solution and how it is relevant for this library? If action helpers still don't work (as @MasGaNo mentionted) which is an actual deal breaker here.

@piotrwitek Totally missed the goal of having it work with the getType helper. Sorry about that. Looking back it's clearly spelled out in the first comment, I guess I didn't totally understand the core issue at first. Sorry about that!

This solves my biggest problem which is having the exact payload type inside each reducer map function (the types in redux-starter-kit did not do this). I'm personally okay with the constant approach since I still get a type error if I spell the type name wrong.

I'm going to try out @MasGaNo's solution in my app, since that seems a bit simpler than mine, and if it solves the same problems I'll use that.

@piotrwitek To help clear up anyone else getting confused about this issue, what do you think of renaming it to something along the lines of "Support getType helper in typesafe reducer factories" since this is about more than just "adding a typesafe reducer factory using object map"?

I have a new API proposal ready for this feature, here are the highlights of new main features:

import * as actions from './'

// NEW FEATURE: extend internal typesafe-actions RootAction type 
// so you never have to use generic types in your application 😮 
// it's as simple as below one-liner
declare module 'typesafe-actions' {
  export type RootAction = ActionType<typeof actions>;
}

// now just import helper functions from "typesafe-actions" as always
import { createReducer, getType } from 'typesafe-actions'

const initialState = 0;

// Notice here you don't need to provide Action type to createReducer (magic!),
// because it's derived from the above declare section 😉 
export const counterReducer = createReducer(initialState)
  // you can pass single or multiple action [using array] creators instances
  // and the action arg will be constrained in reducer function
  .handleAction(actions.add, (state, action) => {
    return state;
  })
  // it also works with type constants, here 'ADD' constant will not be accepted because it's handled above (removing handled actions from accepted argument)
  .handleAction('INCREMENT', (state, action) => {
    return state + 1;
  })
  //.handleAction <= exshausive checking! you cannot use "handleAction" anymore because all available actions are handled


counterReducer(0, increment()); // => 1
counterReducer(0, add(4)); // => 4

Extended proposal with a new feature to create reducer factory using object map, you can even compose different reducers together with type complete safety:

const counterReducer = createReducer(initialState)
  .handleAction(add, (state, action) => state + action.payload)
  .handleAction(increment, (state, _) => state + 1);

const rootReducer = createReducer(initialState, {
 ADD: counterReducer.reducers.ADD,
 INCREMENT: counterReducer.reducers.INCREMENT,
})

All is already implemented and extensively tested, so should be rock solid and production ready.

I just need to add documentation and examples and will release today 🚀

Hey @piotrwitek

Hehe, you reused finally chaining-handle approach 🙂

Ok, I'm waiting for your release, and I will try it on my-side.

I'm still more in favor of the objectMapCreator approach, but until TypeScript implement the missing feature, it could be definitely a good approach, for the Typing safety.

// NEW FEATURE: extend internal typesafe-actions RootAction type 
// so you never have to use generic types in your application 😮 
// it's as simple as below one-liner
declare module 'typesafe-actions' {
  export type RootAction = ActionType<typeof actions>;
}

Really nice 👍

I never consider this approach!

Will make some try with that 😄

Hey @MasGaNo, I think you missed a very important point, I made an implementation that is actually covering both approaches :)

There is the second optional argument in createReducer to provide objectMapCreator, take a look again this time the simple implementation, everything is completely typesafe ⚔️🛡, and you can chain it even further with additional handlers. It's basically an extension mechanism.

const rootReducer = createReducer(initialState, {
 ADD: (state, action) => state + action.payload,
 INCREMENT: (state, _) => state + 1,
})

What do you think?

I worked across all typescript issues and it's even working with getType helper You can take a look at all the test cases here for more examples: https://github.com/piotrwitek/typesafe-actions/blob/c319c3a7b43671bcdd38215ad13139aabc5afab9/src/create-reducer.spec.ts

Hey @MasGaNo, I think you missed a very important point, I made an implementation that is actually covering both approaches :)

Yes right 😄

Very exciting to be able to test it!

Unfortunately, I cannot do it now, need to wait tomorrow 😅

I really like the Array approach to solve the problem of "multi-case" from the "switch... case" approach.

Currently, I store the reducerHandler into a variable and I pass it, but here, it's really a nice approach 👍

And you are still opened to let developer modify the generic parameters TState and TAllActions.

So yeah, definitely, looks very good! 👍

I will definitely provide you my feedback tomorrow 👍

@piotrwitek has rewarded $35.00 to @piotrwitek. See it on IssueHunt

• :moneybag: Total deposit: $50.00
• :tada: Repository reward(20%): $10.00
• :wrench: Service fee(10%): $5.00