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

Generics on createAction don't narrow #92

Closed ethanroday closed 5 years ago

ethanroday commented 5 years ago

Let's say I have a slice of state storing user settings that looks like this:

interface UserSettingsState {
  settingA: string;
  settingB: number;
}

I want to create an action to set a given setting. My first attempt was this:

const setUserSetting = createAction("SET_USER_SETTING",
  <K extends keyof UserSettingsState>(resolve) =>
    (setting: K, newValue: UserSettingsState[K]) => resolve({ setting, newValue })
);

However, this didn't quite have the behavior I expected:

const action1 = setUserSetting("settingA", "foo"); // Allowed, as expected
const action2 = setUserSetting("settingA", 0); // Expected a type error, but this is allowed,
                                               // since the second parameter is not narrowed
// Also, both `action1` and `action2` have type `any`.

I then tried explicitly passing the type parameter down, like this:

const setUserSetting = createAction("SET_USER_SETTING",
  <K extends keyof UserSettingsState>(resolve) =>
    <_K extends K>(setting: _K, newValue: UserSettingsState[_K]) => resolve({ setting, newValue })
);

But this was even worse - both arguments and the return type were any.

I know that this is doable with a normal higher-order function (not that there'd be any reason for this to exist; just to illustrate that the passing down is doable):

const setUserSetting = <K extends keyof UserSettingsState>() =>
  <_K extends K>(setting: _K, newValue: UserSettingsState[_K]) => ({ setting, newValue });
setUserSetting("settingA", "foo"); // Allowed, as expected
setUserSetting("settingA", 0); // Error, as expected

How can I do this with typesafe-actions?

piotrwitek commented 5 years ago

Second attempt is the correct one. I can guess it doesn't work because it wasn't possible in typescript@2.8.3 which the current "TA" version is based on.

You can confirm that by checking if your higher-order function example works with 2.8.3. If it doesn't work then it might be possible that when I update "TA" to recent 3.2.2 and refactor to tuple types then it might start to work.

ethanroday commented 5 years ago

Actually, it seems to be working okay in 2.8.3:

image

Something else maybe?

IssueHuntBot commented 5 years ago

@issuehuntfest has funded $20.00 to this issue. See it on IssueHunt

piotrwitek commented 5 years ago

Solved:

it('with higher-order function', () => {
    interface UserSettingsState {
      settingA: string;
      settingB: number;
    }

    const setUserSetting = <K extends keyof UserSettingsState>(
      setting: K,
      newValue: UserSettingsState[K]
    ) =>
      createAction('SET_USER_SETTING', resolve => () =>
        resolve({ setting, newValue })
      )();

    // @dts-jest:pass:snap -> { type: "SET_USER_SETTING"; payload: { setting: "settingA"; newValue: string; }; }
    setUserSetting('settingA', 'foo');
    // @dts-jest:fail:snap -> Argument of type '0' is not assignable to parameter of type 'string'.
    setUserSetting('settingA', 0); // Error as expected

    // @dts-jest:pass:snap -> { type: "SET_USER_SETTING"; payload: { setting: "settingB"; newValue: number; }; }
    setUserSetting('settingB', 0);
    // @dts-jest:fail:snap -> Argument of type '"foo"' is not assignable to parameter of type 'number'.
    setUserSetting('settingB', 'foo'); // Error as expected
  });
IssueHuntBot commented 5 years ago

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

ethanroday commented 5 years ago

Thanks for the fix! Works like a charm.

mAAdhaTTah commented 5 years ago

This solution produces an action creator that doesn't work with getType though. Is this infeasible without the weird wrapping code?

piotrwitek commented 5 years ago

@mAAdhaTTah you cannot solve it without a wrapper because the generic type argument is stored there

Below change will make it compatible with all action helpers:

const setUserSetting = <K extends keyof UserSettingsState>(
      setting: K,
      newValue: UserSettingsState[K]
    ) => {

      const actionCreator = createAction('SET_USER_SETTING', resolve => () =>
        resolve({ setting, newValue })
      )();

      return Object.assign(actionCreator, {
        getType: () =>'SET_USER_SETTING',
      }) as typeof actionCreator & { getType?: () => 'SET_USER_SETTING' };
}

You could generalize above function to an action creator function that would accept type parameter as well.

kylemh commented 4 years ago

This action actually gets dispatched, but without type-safety when passing props to the would-be rendered modal. I hate this code, but it's a stop-gap; please don't hurt me ♥️

export const setModal = createAction(
  'SET_MODAL',
  (modal: ModalComponent<any> | null, options: ModalOptions = {}) => ({ modal, options }),
)();

Trying to emulate the examples above is not working for me 😞 ... The actions type is dispatched, but it doesn't seem like the dispatch resolves.

1) Here's the one that's not compatible with getType():

export const setModal = <InnerProps>(modal: ModalComponent<InnerProps> | null, options: ModalOptions = {}) => {
  return createAction('SET_MODAL', resolve => () => resolve({ modal, options }))();
};

2) Here's the one that works with getType():

export const setModal = <InnerProps>(modal: ModalComponent<InnerProps> | null, options: ModalOptions = {}) => {
  const actionCreator = createAction('SET_MODAL', resolve => () => resolve({ modal, options }))();

  return Object.assign(actionCreator, {
    getType: () => 'SET_MODAL',
  }) as typeof actionCreator & { getType?: () => 'SET_MODAL' };
};

Any recommendations?