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 99 forks source link

New API - v2.0 #22

Closed piotrwitek closed 6 years ago

piotrwitek commented 6 years ago

Road to v2.0: New API

I'm working on New API using the new language feature (conditional types) coming to TypesScript in the next release (v2.8)

I have already published working version that is compatible with recent typescript@rc release

> to install yarn add typesafe-actions@next

Some changes are inspired by the suggestion from the past and I would really appreciate some feedback from their authors:

The implementation of a new API is taking place on the next branch, and will be released on the release of TypeScript v2.8.

all of the examples are working and have test's against them so they should be rock solid

Finished

ActionsUnion

conditional mapped type, will provide much cleaner and experience to get Union Type of actions


import { ActionsUnion } from 'typesafe-actions';
import * as actions from './todos-duck-module';

type RootAction = ActionsUnion;

- ActionsUnion works nicely with nested actions from new `async` action creator (check below)
```ts
const actions = {
    async: buildAction('GET_USER').async<number, { name: string }, string>(),
};
type RootAction = ActionsUnion<typeof actions>;
// { type: "GET_USER" & "REQUEST"; payload: number; } | { type: "GET_USER" & "SUCCESS"; payload: { name: string; }; } | { type: "GET_USER" & "FAILURE"; payload: string; }

ReturnType

getting type declaration of action will now be possible to derive from action creator on call site without the need to declare tons of boilerplate before-hand:


import { ReturnType, createAction } from 'typesafe-actions';

const increment = createAction('INCREMENT');

function incrementReducer(state: State, action: ReturnType) { ...


#### `buildAction`
`import { buildAction } from 'typesafe-actions'`
> function will accept action type and return an object with multiple methods to create different action creators described below:

- `buildAction('INCREMENT').empty()`
```ts
const increment = buildAction('INCREMENT').empty();
expect(increment()).toEqual({ type: 'INCREMENT' }); //  { type: 'INCREMENT' }
expect(getType(increment)).toBe('INCREMENT'); // 'INCREMENT'
type User = { name: string };
const fetchUsers = buildAction('LIST_USERS').async<void, User[], string>();
expect(fetchUsers.request())
    .toEqual({ type: 'LIST_USERS_REQUEST' }); // { type: 'LIST_USERS' & 'REQUEST' }
expect(fetchUsers.success([{ name: 'Piotr' }]))
    .toEqual({ type: 'LIST_USERS_SUCCESS', payload: [{ name: 'Piotr' }] }); // { type: 'LIST_USERS' & 'SUCCESS'; payload: User[] }
expect(fetchUsers.failure('error message'))
    .toEqual({ type: 'LIST_USERS_FAILURE', payload: 'error message' }); // { type: 'LIST_USERS' & 'FAILURE'; payload: string }
expect(getType(fetchUsers.request)).toBe('LIST_USERS_REQUEST'); // 'LIST_USERS' & 'REQUEST'
expect(getType(fetchUsers.success)).toBe('LIST_USERS_SUCCESS'); // 'LIST_USERS' & 'SUCCESS'
expect(getType(fetchUsers.failure)).toBe('LIST_USERS_FAILURE'); // 'LIST_USERS' & 'FAILURE'

Breaking changes

🎉NO BREAKING CHANGES 🎉

Known Issues

Migration to 1.2.0

Here is an migration example of example in the docs:

// migration to v1.2 (complete type-safety is still preserved) const add = buildAction('ADD').payload;


- advanced Flux Standard Action
```ts
// current v1.0
const notify = createAction('NOTIFY',
  (username: string, message?: string) => ({
    type: 'NOTIFY',
    payload: `${username}: ${message || ''}`,
    meta: { username, message },
  }),
);
// migration to v1.2 (complete type-safety is still preserved)
const notify = buildAction('NOTIFY').fsa<{ username: string; message?: string }>(
    ({ username, message }) => `${username}: ${message || ''}`,
    ({ username, message }) => ({ username, message })
);
flq commented 6 years ago

This all looks very useful to me. With regard to

buildAction('INCREMENT').payload<P>()

The following one we also use quite often:

a1 = buildAction('FAIL').error();
a1(new Error("Damn"));
piotrwitek commented 6 years ago

@flq thanks for feedback, could you also add what would be expected action shape from calling a1 and it's type signature? :) More specifically, are you're interested in setting error: boolean property of action like in the FSA standard?

flq commented 6 years ago

See, I never stop learning, I wasn't quite aware of said standard. In that context it'd probably have the same signature as payload<P>, but the resulting action also sets error to true.

piotrwitek commented 6 years ago

yes .payload<Error>() would be enough, I could add logic in action creator to check if payload is instanceOf Error and in this case add an error property with the value true to the action and of course change the type signature accordingly

farzadmf commented 6 years ago

Looking forward to using this new API. Just one thing, it maybe a stupid question, but can you please explain what fsa stands for? Is it flux-specific? (I haven't ever used flux)

piotrwitek commented 6 years ago

@farzadmf as @flq mentioned above it stands for Flux Standard Action: https://github.com/redux-utilities/flux-standard-action, most redux related libraries are following it more or less

duro commented 6 years ago

@piotrwitek Do you think it would be useful to add a way to set the async suffix that is used? For example if I wanted the suffix the be /@REQUEST instead of _REQUEST

Could make creating generic middleware for promisifying these action creators more sane.

Let me know if I'm not making sense.

duro commented 6 years ago

@piotrwitek Not sure if I found a bug, or am just doing something wrong.

Given the following code:

setToken: buildAction(SessionActionType.SET_TOKEN).fsa<{
  token: string
  resolve?: () => void
  reject?: (error: Error) => void
}>(
  ({ token }) => ({ token }),
  ({ resolve, reject }) => ({ resolve, reject })
)

I am getting the following type errors in VSCode using TS2.8

actions_ts_ _freebird-native

actions_ts_ _freebird-native

duro commented 6 years ago

I was able to get my use case to work with the following:

setToken: buildAction(SessionActionType.SET_TOKEN).fsa<
  { token: string; resolve: () => void; reject: (error: Error) => void },
  { token: string },
  { resolve: () => void; reject: (error: Error) => void }
>(
  ({ token }) => ({ token: 'fgfg' }),
  ({ resolve, reject }) => ({ resolve, reject })
)

Now I seem to be having some oddities around using the ActionsUnion with .async() created actions. Since that call returns three methods, instead of what would normally satisfy a the Redux.Action type, the compiler is complaining.

Any thoughts there?

piotrwitek commented 6 years ago

@duro fsa is WIP, for the moment it works differently like this:

setToken: buildAction(SessionActionType.SET_TOKEN).fsa(
  ({ token }: { token: string }) => ({ token }),
  ({ token }) => ({ resolve: ..., reject: ... })
)

But I plan to change it to using only the generic type for payload, and the rest will use inference

duro commented 6 years ago

@piotrwitek Did you see my last comment https://github.com/piotrwitek/typesafe-actions/issues/22#issuecomment-376672914. I was able to get it working with three type args.

That said, see the second half of the comment. New issue is with use of .async() and ActionsUnion

I'm no TS wiz, but perhaps for that case, we could have a named interface for the return of the .async and do an additional conditional that digs one level deeper if that type?

piotrwitek commented 6 years ago

Yes I saw it and working on it :) Trying to modify mapped with conditional type

piotrwitek commented 6 years ago

@duro Ok got it working :)

const actions = {
      empty: buildAction('INCREMENT').empty(),
      payload: buildAction('ADD').payload<number>(),
      fsa: buildAction('SHOW_NOTIFICATION').fsa((message: string) => message),
      async: buildAction('GET_USER').async<number, { name: string }, string>(),
    };
    type RootAction = ActionsUnion<typeof actions>;
// { type: "INCREMENT"; } | { type: "ADD"; payload: number; } | { type: "SHOW_NOTIFICATION"; payload: string; } | { type: "GET_USER" & "REQUEST"; payload: number; } | { type: "GET_USER" & "SUCCESS"; payload: { name: string; }; } | { type: "GET_USER" & "FAILURE"; payload: string; }

I'll do new release in a few minutes

duro commented 6 years ago

@piotrwitek Awesome! Can't wait to see it. I was fiddling with it over here, and I wonder how close I was to the right track.

piotrwitek commented 6 years ago

@duro I deployed a new version to npm

duro commented 6 years ago

@piotrwitek Works great! And I was getting close. Your library has taught me more about TS that the docs :)

manvesh commented 6 years ago

@piotrwitek buildAction doesn't seem to be returning the right shape for the action (in empty/payload/fsa) with the latest RC.2.

screen shot 2018-03-28 at 10 29 39 pm
piotrwitek commented 6 years ago

@manvesh it works fine I tested. Please check in console with tsc compiler, you have probably selected default ts version in vscode, not from node_modules

CzBuCHi commented 6 years ago

i like all your good work on this. 👍

I have one question: Why is used this syntax?

const actions = {
      empty: buildAction('INCREMENT').empty(),
      payload: buildAction('ADD').payload<number>(),
      fsa: buildAction('SHOW_NOTIFICATION').fsa((message: string) => message),
      async: buildAction('GET_USER').async<number, { name: string }, string>(),
};

im asking, because it look little wierd to me create simple js object by calling 2 functions only to make ts happy ...

PS: not sure if typescript can infer return type based on number of type parameters passes in - something like this:

buildAction('FOO')  // no type params -> buildAction('FOO').empty();
buildAction<T>('BAR')  // single param with single argument -> buildAction('BAR').payload<T>();
buildAction<T>('FSA', pc, mc)  // single param with 3 arguments -> buildAction('FSA').fsa<T>(pc,mc);
buildAction<R, S, F>('RSF') // three params -> buildAction('RSF').async<R,S,F>();

if this works i think it would be cleaner syntax (or at least less typing syntax :) )

piotrwitek commented 6 years ago

@CzBuCHi I tried that already and I couldn't make it work, seems like it's not possible but I'm not 100% sure If that is possible I would consider using it

CzBuCHi commented 6 years ago

@piotrwitek ah okay ... i will try to experiment with it when i get some time and get result here ...

manvesh commented 6 years ago

@piotrwitek I was able to use actions that are not async.

But as soon as I use an async action in ActionUnion, I'm not able to use getType correctly. - All the payloads are checked against request type of the async action.

Errors on using async -

screen shot 2018-03-29 at 9 41 53 am

For example, error on refreshToken is

[ts]
Property 'refreshToken' does not exist on type '{} | { isAuthenticated: boolean; displ...'.
  Property 'refreshToken' does not exist on type '{}'.

Works fine without async -

screen shot 2018-03-29 at 9 38 47 am

I'm not sure if I'm doing something wrong, or if async usage is currently WIP.

piotrwitek commented 6 years ago

@manvesh I think I don't have a type tests for this scenario yet, so yes it's WIP, I'll look to fix it tomorrow :)

runeh commented 6 years ago

This new is looking really nice! Tried converting some of my project, ran into an issue with actionCreators that take a boolean:

Given

const actions = {
  putLoadingState: buildAction('PUT_LOADING_STATE').payload<boolean>(),
}

I get this error from the callsite where I try to invoke putLoadingState(true):

Cannot invoke an expression whose type lacks a call signature. Type '((payload: true) => PayloadAction<"PUT_LOADING_STATE", true>) | ((payload: false) => PayloadActio...' has no compatible call signatures.

Is this expected to work, or have I misunderstood the API?

runeh commented 6 years ago

I added a test here: https://github.com/runeh/typesafe-actions/tree/next . It fails when running npm run test but succeeds when running npm run test:watch . Weird.

piotrwitek commented 6 years ago

@runeh thanks a lot for help! You found 2 nice improvements to the project :)

piotrwitek commented 6 years ago

@runeh the fix is already deployed to npm rc.3 thanks for adding the test case I cherry-picked from your repo :) Enjoy!

runeh commented 6 years ago

@piotrwitek That's awesome! Thanks.

kuzvac commented 6 years ago

@piotrwitek can you suggest properly usage async buildAction with ActionsUnion? I create gist with code i use, when i try to access action.payload in case getType(authActions.fetchAuth.request): i get an error:

Error:(49, 25) TS2339: Property 'payload' does not exist on type 'EmptyAction<"GET_LOGGED_USER"> | PayloadAction<"AUTH" & "REQUEST", { pending: boolean; email: str...'. Property 'payload' does not exist on type 'EmptyAction<"GET_LOGGED_USER">'.

Looks like, i doing something wrong...

piotrwitek commented 6 years ago

@kuzvac there is an issue with Algebraic Data Types implementation in TS with regard to Product Type not beign accepted as proper discriminant property, I'm trying to work this out. I will update the proposal with a warning about this issue

joeferraro commented 6 years ago

@kuzvac there is an issue with Algebraic Data Types implementation in TS with regard to Product Type not beign accepted as proper discriminant property, I'm trying to work this out. I will update the proposal with a warning about this issue

@piotrwitek is this the same issue affecting @manvesh above ^^?

kuzvac commented 6 years ago

Ok, no problem, thank you.

piotrwitek commented 6 years ago

@joeferraro exactly the same

runeh commented 6 years ago

@piotrwitek I tried out a couple of other cases from our codebase that doesn't currently work. Error is the same as with the boolean case from yesterday. Tests are here: https://github.com/runeh/typesafe-actions/commit/b4c4efc7460b5faf88bdecd4f8c71ab8c32c64e2 .

Not sure if these cases are something the type system can deal with, but if not, it's probably useful to document.

piotrwitek commented 6 years ago

Hi @runeh, It should be fixed already, I have incorporated a new generic solution to handle all possible union combinations, please test the recent release rc4!

I added your newest test cases and it passed just fine!

It's not pushed here yet but released to npm

runeh commented 6 years ago

@piotrwitek oh nice. Works for me as well now. I must have not updated to the latest properly or something. :+1:

joeferraro commented 6 years ago

@piotrwitek thanks so much for this great library. Question for you as I attempt to implement a generic way to handle actions. Should I be able to do the following?

import { AxiosResponse } from 'axios';
import { Dispatch } from 'redux';
import { ThunkAction } from 'redux-thunk';
import { AsyncCreator, buildAction } from 'typesafe-actions';

import { PDCReduxState } from '../reducers';
import * as api from '../lib/api';

export class MyClass<T> {

  private entity: string;
  private listAction: AsyncCreator<string, void, T[], string>;
  private getAction: AsyncCreator<string, void, T, string>;

  constructor(entity: string) {
    this.entity = entity;
    this.listAction = buildAction(`${entity}/LIST`).async<void, T[], string>();
    this.getAction = buildAction(`${entity}/GET`).async<void, T, string>();
  }

  list(): ThunkAction<Promise<void>, PDCReduxState, void> {
    return async (dispatch: Dispatch<PDCReduxState>): Promise<void> => {
      try {
        dispatch(this.listAction.request()); // <-- this compiles
        const response: AxiosResponse<Array<T>> = await api.get(`/${this.entity}`);
        if (response.status === 200) {
          dispatch(this.listAction.success(response.data)); // <-- this compiles
          return Promise.resolve();
        } else {
          throw new Error(`Request failed: ${response.statusText}`);
        }
      } catch (e) {
        dispatch(this.listAction.failure(e)); // <-- this compiles
        return Promise.reject(e);
      }
    };
  }

  get(id: string): ThunkAction<Promise<void>, PDCReduxState, void> {
    return async (dispatch: Dispatch<PDCReduxState>): Promise<void> => {
      try {
        dispatch(this.getAction.request()); // <-- this compiles
        const response: AxiosResponse<T> = await api.get(`/${this.entity}/${id}`);
        if (response.status === 200) {
          dispatch(this.getAction.success(response.data)); // <-- *** this does NOT compile
          return Promise.resolve();
        } else {
          throw new Error(`Request failed: ${response.statusText}`);
        }
      } catch (e) {
        dispatch(this.getAction.failure(e)); // <-- this compiles
        return Promise.reject(e);
      }
    };
  }
}
dispatch(this.getAction.success(response.data));

will not compile with the error:

[ts] Cannot invoke an expression whose type lacks a call signature. 
Type 'EACreator<string & "SUCCESS"> | 
((payload: T) => PayloadAction<string & "SUCCESS", T>)' has no compatible call signatures.

I've tried with 1.2.0-rc.1, 1.2.0-rc.2, and 1.2.0-rc.4

Any suggestions would be much appreciated ... I'm not sure whether this is a bug or a general limitation of TypeScript.

piotrwitek commented 6 years ago

I have updated version to use a major bump (v2.0.0-0), because of requirements on TS 2.8 conditional types. From now on please install using next tag:

> yarn add typesafe-actions@next

TotallWAR commented 6 years ago

Is getReturnOfExpression removed? Could you say am im doing right? What should i do with:

const returnsOfActions = Object.values(widgetContentActions).map(getReturnOfExpression);
export type Action = typeof returnsOfActions[number];

My reducer is:

import {combineReducers} from 'redux';
import {getType} from 'typesafe-actions';
import {getReturnOfExpression} from 'utility-types';

import {WidgetContent} from '../../typings/content';
import * as widgetContentActions from './actions';
import {RootState} from 'Features/root-reducer';
import {RootAction} from 'Features/root-action';

const returnsOfActions = Object.values(widgetContentActions).map(getReturnOfExpression);
export type Action = typeof returnsOfActions[number];

export type State = {
    readonly widgetContent: WidgetContent | undefined;
};

export const contentReducer = combineReducers<State, Action>({
    widgetContent: (state, action) => {
        switch (action.type) {
            case getType(widgetContentActions.contentSetAction):
                return Object.assign({}, state, action.payload);

            default:
                return state;
        }
    }
});

My actions file:

import {buildAction} from 'typesafe-actions';
import {CONTENT_GET, CONTENT_SET} from '../../constants/action-types';
import {WidgetContent} from '../../typings/content';

export const contentGetAction = buildAction(CONTENT_GET).payload<string>();
export const contentSetAction = buildAction(CONTENT_SET).payload<WidgetContent | object>();

My epic:

import 'rxjs/add/operator/map';
import 'rxjs/add/operator/mergeMap';
import {Epic} from 'redux-observable';

import {RootState} from 'Features/root-reducer';

import {
    contentSetAction,
} from './actions';

import {loadContent} from './service';
import {WidgetContent} from '../../typings/content';
import {RootAction} from 'Features/root-action';
import {isActionOf} from 'typesafe-actions';

export const contentGetEpic: Epic<RootAction, RootState> = (action$, store) =>
    action$.filter(isActionOf(contentSetAction))
        .mergeMap((action) =>
            loadContent(action.payload)
                .map((payload: WidgetContent) =>
                    contentSetAction(payload))
        );

EDIT: I found getReturnOfExpression in utility-types package.

Methuselah96 commented 6 years ago

v2.0.0-0 doesn't seems to have some of the bug fixes that 1.2.0-rc.4 had (specifically the union payload).

piotrwitek commented 6 years ago

@TotallWAR please don't use getReturnOfExpression, it's deprecated.

You can use the new cleaner approach:

import { ActionsUnion } from 'typesafe-actions';

...
import {WidgetContent} from '../../typings/content';
import * as widgetContentActions from './actions';
import {RootState} from 'Features/root-reducer';
import {RootAction} from 'Features/root-action';

export type Action = ActionsUnion<typeof widgetContentActions>;
...
piotrwitek commented 6 years ago

@Methuselah96 I tested typesafe-actions@2.0.0-0 in project just now and is working fine, double-check it you not forgotten to use @next tag when installing

Methuselah96 commented 6 years ago

@piotrwitek Don't know what I did, but it seems to be working now. Thanks.

Methuselah96 commented 6 years ago

@piotrwitek Should tslib be under dependencies instead of devDependecies? I'm getting errors saying that it can't resolve tslib when trying to run tests, so I had to install it manually.

piotrwitek commented 6 years ago

@Methuselah96 yes correct, is should be dependencies :) thanks

EDIT: fix published as v2.0.0-1

farzadmf commented 6 years ago

Dear @piotrwitek , I would really appreciate if you can provide a "tutorial" for this new API (similar to the one you already have in the Github README for the current API).

piotrwitek commented 6 years ago

@farzadmf sure I'll try to do it, let's track it in this issue: #25

chasecaleb commented 6 years ago

Playing around with this API and I overall I really like, although I'm a little stuck: is there a way to add logic buildAction(..).async<R, S, F>(), kind of like how fsa(..) takes functions to create the payload and meta?

Use case: Handling async API requests in a cross-cutting way. Currently, I have action creator functions that delegate to this utility method:

interface ApiRequestAction<T extends string> {
    readonly type: T;
    readonly payload: { readonly url: string, readonly init?: RequestInit },
    readonly meta: { readonly statusType: "REQUEST" | "RESPONSE" },
}
const createApiRequest = <T extends string>(
    type: T,
    url: string,
    init?: RequestInit
): ApiRequestAction<T> => ({
    meta: { statusType: "REQUEST" },
    payload: { url, init },
    type
});

For instance:

const submitSearch: (values: SearchFormValues) = (
    values: SearchFormValues
) =>
    createApiRequest("SEARCH_API", "/api/search", {
        body: JSON.stringify(values),
        method: "POST"
    });

Using redux-saga, I have a saga listening for actions with meta: { statusType: "REQUEST" } that calls fetch(..) and then dispatches a similar action containing the response. Overall I like the type safety and elegance of your api much more, but I do like having the request logic in my action creators and a single saga that handles all api requests.

Do you have any suggestions for how to approach this with your api? The best I've come up with so far is writing an extra action creator factory function like this (untested, but you get the idea):

interface RequestPayload {
    readonly url: string,
    readonly init?: RequestInit
}
const actions = {
    search: buildAction("SEARCH_API").async<RequestPayload, Response, string>()
};
const createSearch = (values: SearchFormValues) =>
    actions.search.request({
        init: {
            body: JSON.stringify(values),
            method: "POST"
        },
        url: "/api/search"
    });
cjol commented 6 years ago

I agree that the syntax is a bit ugly; I would much rather have a bunch of separate functions that I could call like buildPayloadAction<number>("FOO"). I had a play to understand why this wasn't possible, and I thought I would write up my findings for future reference.

The problem is that there are actually two generic type arguments for a function like buildPayloadAction - one for the payload type P (number in the example above), and another for the string literal type T ("FOO" in the example above). In our case, we want users to be able to specify P, but not need to specify T, because we can infer it from the function parameter. Unfortunately, in TypeScript inference is currently all or nothing, which means that we cannot have a single function where only one of the generic params is inferred. The workaround is to split the generic params over two functions, as piotrwitek has done here.

Someone else reached the same conclusion to their problem (stated here) and explained their results in a little more detail here.

In terms of function naming, I think my preference would still be to have several specialised top-level functions, so things could look more like:

emptyAction("FOO").build();
payloadAction("FOO").withType<number>();
asyncAction("FOO").withTypes<RequestPayload, Response, string>();

But I realise that doesn't save very much, and might still be controversial.

piotrwitek commented 6 years ago

Hi @cjol, thanks for your summary and feedback!

This is exactly the reason why I went with such API, moreover I was also considering switching to a specialized functions approach as you suggested (empty action API won't be necessary because you could do payloadAction("FOO").withType<void>()).

I was thinking to clean the new API to this form, which would handle all the use-cases from initial proposal:

const noPayload = buildAction('NO_PAYLOAD').withType<void>(); // noPayload()
const createUser = buildAction('CREATE_USER').withType<PayloadType>();
const createUser = buildAction('CREATE_USER').withTypes<PayloadType, MetaType>();
const notify = buildAction('NOTIFY').withMappers<InputParameterType>( // output types are inferred
  ({ username, message }) => `${username}: ${message || ''}`, // payload mapper
  ({ timestamp }) => ({ timestamp }) // meta mapper
);

const fetchUser = buildAsyncAction(
  'FETCH_USER_REQUEST',
  'FETCH_USER_SUCCESS',
  'FETCH_USER_FAILURE'
).withTypes<string, User, Error>();

const fetchUser = buildAsyncAction(
  'FETCH_USER_REQUEST',
  'FETCH_USER_SUCCESS',
  'FETCH_USER_FAILURE'
).withMappers<string, User, Error>(
  id => ({ id }), // request mapper
  ({ firstName, lastName }) => `${firstName} ${lastName}` // success mapper
  ({ message }) => message // error mapper
);

PS: I saw TS team were discussing on a design meeting possibility to add a partially specified type arguments with inference, so when this feature arrives we will iterate and update API to the new and cleaner form (by removing withType chain call)(https://github.com/Microsoft/TypeScript/pull/22368#issuecomment-377715476)