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

isActionOf doesn't narrow properly with multiple actions #63

Closed cjol closed 5 years ago

cjol commented 6 years ago

Here's my minimal demo:


import { createAction, ActionsUnion } from "typesafe-actions";
import { filter, tap } from "rxjs/operators";

const actions = {
  toggle: createAction("todos/TOGGLE", resolve => {
    return (id: string) => resolve(id);
  }),
  add: createAction("todos/ADD", resolve => {
    return (title: string) => resolve({ title, id: 5, completed: false });
  }),
  dummy: createAction("todos/DUMMY", resolve => () => resolve({foo: true}))
};

export type TestActions = ActionsUnion<typeof actions>;

(action$: ActionsObservable<TestActions>) =>
  action$.pipe(
    filter(isActionOf([actions.add, actions.toggle])),
    tap(x => x.type) // x.type:  "todos/TOGGLE" | "todos/ADD" | "todos/DUMMY"
  );

const myFilter = isActionOf([actions.add, actions.toggle]);
(action$: ActionsObservable<TestActions>) =>
  action$.pipe(
    filter(myFilter),
    tap(x => x.type) // x.type:  string | "todos/ADD" | "todos/TOGGLE"
  );

In the first case, no narrowing is done at all. In the second case (which I discovered by trying to inspect the result of isActionOf directly), some narrowing seems to be done, but it's also unioning with an empty action, so that x is basically unusable. The type given to myFilter is:

const myFilter: (action: {
    type: string;
}) => action is {
    type: "todos/TOGGLE";
    payload: string;
} | {
    type: "todos/ADD";
    payload: {
        title: string;
        id: number;
        completed: boolean;
    };
} | {
    type: string;
}

Am I doing something wrong?

piotrwitek commented 6 years ago

Hey, Could you please attach typescript version and tsconfig?

cjol commented 6 years ago

Yes - I can reproduce with typescript @ latest version (2.9.2), and this config (I've also included the package.json so you can see other dep versions), and typescript's declaration file in case that gives anything away.

piotrwitek commented 6 years ago

@cjol nice gist, cool trick with declaration output ;) I'm guessing the issue may be related to this: https://github.com/piotrwitek/typesafe-actions/issues/44

I'll try to investigate later today.

cjol commented 6 years ago

Thanks for looking into this. The output doesn't seem to change if I set strictFunctionTypes: false, (or even strict: false), but I don't really understand what the issue is in #44, so I'll leave it up to you for now!

HorusGoul commented 6 years ago

Also got this issue, in my case, using isActionOf and then tap(action => ...) doesn't work, but, using mergeMap(action => ...) does.

IssueHuntBot commented 6 years ago

@boostio funded this issue with $20. Visit this issue on Issuehunt

p-sebastian commented 6 years ago

For anyone else having this issue, @cjol @piotrwitek. I created this isOfTypes function, that will take Type Constants as attributes and return the action type;

But it requires to be passed the CONSTANTS union type in the declaration of the function, I tried for hours to make it generalized and take constants as a type attribute but couldn't

So if you have:

// ...types.ts
export const AUTH_REQUEST = '@auth/request';
export const JOB_START_REQUEST = '@job/start_request';
export const AUTH_SUCCESS = '@auth/success';

// ...actions.ts
export const authenticate = (body: Api.Auth.IReq) => action(AUTH_REQUEST, { ...body, method: 'login' });
export const authSuccess = (res: Api.Auth.IRes) => action(AUTH_SUCCESS, res);
export const startJob = (body: Api.Jobs.IStartJobReq) => action(JOB_START_REQUEST, { ...body, method: 'job-start' });
// FUNCTION
import { AUTH_REQUEST,  AUTH_SUCCESS, JOB_START_REQUEST } from '../types.ts';

// IMPORTANT must be declared
type CONSTANTS = typeof AUTH_REQUEST | typeof AUTH_SUCCESS | typeof JOB_START_REQUEST;

export function isOfTypes<T extends CONSTANTS, Action extends { type: string }>(...actionTypes: T[]) {

  const assertFn = (action: Action): action is Action extends { type: T } ? Action : never => {
    return actionTypes.some(constant => action.type === constant);
  };

  return assertFn;
}

So on the epic we would have

// ...epic.ts
...
const organizeEpic: Epic<ApiActionsType, any, RootState> = (action$, state$) => 
  action$.pipe(
    /// Here is the function
    filter(isOfTypes(AUTH_REQUEST, JOB_START_REQUEST)),
    withLatestFrom(state$),
    // action here is of type defined below
    switchMap(([action, state]) => concat(
      // start spinner
      of(spinnerStart()),
      of(ajaxRequest(attachDefaults(state, action.payload)))
    ))
  );

// action is of type:
var action: {
    type: "@auth/request";
    payload: {
        ...
    };
} | {
    type: "@job/start_request";
    payload: {
        ...
    };
}
IssueHuntBot commented 5 years ago

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

piotrwitek commented 5 years ago

Resolved by https://github.com/piotrwitek/typesafe-actions/pull/94

IssueHuntBot commented 5 years ago

@vincentjames501 has submitted a pull request. See it on IssueHunt

IssueHuntBot commented 5 years ago

@piotrwitek has rewarded $28.00 to @vincentjames501. See it on IssueHunt