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

isActionOf doesn't narrow properly with multiple actions #63

Closed
cjol opened this issue Jul 26, 2018 · 11 comments · Fixed by #94
Closed

isActionOf doesn't narrow properly with multiple actions #63

cjol opened this issue Jul 26, 2018 · 11 comments · Fixed by #94
Labels

Comments

@cjol
Copy link

cjol commented Jul 26, 2018

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

Hey,
Could you please attach typescript version and tsconfig?

@cjol
Copy link
Author

cjol commented Jul 27, 2018

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

@cjol nice gist, cool trick with declaration output ;)
I'm guessing the issue may be related to this: #44

I'll try to investigate later today.

@cjol
Copy link
Author

cjol commented Jul 27, 2018

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

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

@IssueHuntBot
Copy link

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

@p-sebastian
Copy link

p-sebastian commented Sep 17, 2018

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

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

vincentjames501 pushed a commit to vincentjames501/typesafe-actions that referenced this issue Dec 19, 2018
piotrwitek pushed a commit that referenced this issue Dec 26, 2018
…#63 (#94)

`isActionOf` when given an array doesn't yield the correct type assertions. Here's a minimalist example:

```typescript
const action1 = createAction('Action 1', resolve => (value: number) => resolve({ value }));
const action2 = createAction('Action 2', resolve => (value: number) => resolve({ value }));

const action: any = action1(1);
if (isActionOf([action1, action2], action)) {
  // This should be valid because `payload` exists on both action1 and action2 types but the actual resolution was before this change `action1 | action2 | {type: string}` and is now correctly `action1 | action2`
  console.log(action.payload.value);
}
```

Thank you for your contribution! 👍

Please makes sure that these checkboxes are checked before submitting your PR, thank you!

* [x] Rebase before creating a PR to keep commit history clean
* [x] Clear node_modules and reinstall all the dependencies: `npm run reinstall`
* [x] Run checker npm script: `npm run check`

Extra checklist:
* [x] Always add some description and refer to all the related issues using `#` tag

For bugfixes:
 * [x] Add at least one unit test to cover the bug that have been fixed
@piotrwitek
Copy link
Owner

Resolved by #94

@IssueHuntBot
Copy link

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

@IssueHuntBot
Copy link

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

  • 💰 Total deposit: $40.00
  • 🎉 Repository reward(20%): $8.00
  • 🔧 Service fee(10%): $4.00

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants