neurosnap / robodux

caching in redux made simple
MIT License
101 stars 7 forks source link

Suggestion: simpler Actions interfaces #7

Closed Dudeonyx closed 5 years ago

Dudeonyx commented 5 years ago

I forked this project and made some changes to how the actions are typed

Here it is

I published a temporary alt package to be used for testing as robodux-alt

1

First of all the action interfaces are simpler because in the all action types are of the form actionName: (payload: string) => Action<string> The only thing that changes is the payload type so why not simply specify actionName: string and convert it to the first form internally

2

Second if an actions interface is provided it is used to typecheck the actions field when creating the slice

interface SliceState {
  test: string;
  wow: number;
}

interface IState {
  hi: SliceState;
}

interface Actions {    // simpler interface specifying only the payload type
  set: SliceState;
  setWow: number;
  setTest: string;
  reset: null;             // <- null is used to indicate that no payload is expected
};

const defaultState = {
  test: '',
  wow: 0,
};

const { actions, selectors, reducer } = robodux<SliceState, Actions, IState>({
  slice: 'hi',
  actions: {                        // the action names are autocompleted
    set: (state, payload) => payload,   // state is automatically typed as SliceState and payload as SliceState(from the Actions interface)
    setWow: (state, payload)=>{ state.wow = payload }  // payload is typed as a number
   setTest: (state, payload) => {state.test = payload}   // payload is string
    reset: (state) => defaultState,        //  state is typed but a typeerror is shown if a payload is added
  },
  initialState: defaultState,
});

actions.set     // is of type (payload: SliceState)=> Action<SliceState>

actions.setWow  // is of type (payload: number) => Action<number>

actions.setTest  // is of type (payload: string) => Action<string>

actions.reset     // is of type () => Action

actions.set()      // throws a typeerror: expected 1 argument of type SliceState

actions.reset(defaultState)      // throws a typeerror: expected 0 arguments

actions.setWow('hello')    // typeerror: argument of type number expected

actions.setTest(5)   //typeerror: argument of type string expected

actions.set({test: 'yolo', wow: 99})  // works perfectly

actions.reset()       // works perfectly 

3

Third I had to do some weird typing so that if an actions interface is not provided the action creators are typed as (payload?: any) => Action<any>

Note: all of the above were done without editing the actual code at all, just the types. The code below required heavy editing and is mostly experimental

4

Fourth I refactored the code a bit(meaning a lot) so I could create a createSliceAlt that detects if the initial state is an object and automatically create additional selectors for each key in it

The additional selectors are name in the form: get${slice}${key}

So if you have an intial state object like above in a slice called hi and with keys test and wow, the selectors created would be: getHi, getHiWow and getHiTest And they do exactly what their names suggest.

However Trying to correctly get the return types of the selectors is proving to be difficult, likely impossible as Typescript currently is. Right now the type of the any of the selectors in this case for example would be (state: State) => State | SliceState | string | number

It has a return type including every single type in the state, even those from other slices.

I'm not satisfied with this hence why I created a different function instead but I refactored the original create functions sub-functions to reduce code duplication.

neurosnap commented 5 years ago

Any updates? How can I help?

Dudeonyx commented 5 years ago

Any updates? How can I help?

Ah, I'm actually done, Full type inference!, I got side tracked forking redux-starter-kit, converting it to typescript and incorporating my changes.

For now I'll make a PR with the new typings. let's discuss the other(somewhat breaking) changes and possible benefits another time

neurosnap commented 5 years ago

10