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

v5.0.0 - Rewrite and simplify `createAction` API #143

Closed
piotrwitek opened this issue Apr 22, 2019 · 24 comments
Closed

v5.0.0 - Rewrite and simplify `createAction` API #143

piotrwitek opened this issue Apr 22, 2019 · 24 comments

Comments

@piotrwitek
Copy link
Owner

@piotrwitek piotrwitek commented Apr 22, 2019

Issuehunt badges

Is your feature request related to a real problem or use-case?

Currently, createAction API is not optimal and require some unnecessary boilerplate code (historically it was created pre TS v3.0 so there was a lot of limitations having an impact on API shape):

const action = createAction('TYPE', resolve => {
    return (name: string, id: string) => resolve(id);
  });

Describe a solution including usage in code example

Today with recent TypeScript features I can finally rewrite the API to something simpler, more intuitive and more consistent across the board.

This will resolve few existing feature requests:

createAction

type User = { id: number, name: string };
const user: User = { id: 1, name: 'Piotr' };

const action1 = createAction('TYPE1')<string>();
action1(user.name); // => { type: 'TYPE1', payload: 'Piotr' }

const action2 = createAction(
  'TYPE1',
  (user: User) => user.name, // payload creator
)<Payload>();
action2(user); // => { type: 'TYPE1', payload: 'Piotr' }

const actionWithMeta1 = createAction('TYPE2')<string, number>();
actionWithMeta1(user.name, user.id); // => { type: 'TYPE2', payload: 'Piotr', meta: 1 }

const actionWithMeta2 = createAction(
  'TYPE2',
  (user: User) => user.name, // payload creator
  (user: User) => user.id, // optional meta creator
)<string, number>();
actionWithMeta2(user); // => { type: 'TYPE2', payload: 'Piotr', meta: 1 }

createAsyncAction

const action = createAsyncAction(
  'REQ_TYPE', 'SUCCESS_TYPE', 'FAILURE_TYPE', 'CANCEL_TYPE',
)<Request, Response, Error, undefined>();

const action = createAsyncAction(
  ['REQ_TYPE', (req: Request) => req], // request payload creator
  ['SUCCESS_TYPE', (res: Response) => res], // success payload creator
  ['FAILURE_TYPE', (err: Error) => err], // failure payload creator
  'CANCEL_TYPE', // optional cancel payload creator
)();

const actionWithMeta = createAsyncAction(
  'REQ_TYPE', 'SUCCESS_TYPE', 'FAILURE_TYPE', 'CANCEL_TYPE',
)<[Request, Meta], Response, [Error, Meta], undefined>();

const actionWithMeta = createAsyncAction(
  ['REQ_TYPE', (req: Request) => req, (req: Request) => 'meta'], // request payload & meta creators
  ['SUCCESS_TYPE', (res: Response) => res], // success payload creator
  ['FAILURE_TYPE', (err: Error) => err, (err: Error) => 'meta'], // failure payload & meta creators
  'CANCEL_TYPE', // optional cancel payload creator
)();

createCustomAction

All remaining non-standard use-cases you will be able to cover with createCustomAction

const action1 = createCustomAction('TYPE1', (name: string) => ({ payload: name }));

const action2 = createCustomAction('TYPE2', (name: string, id: string) => ({
  payload: name, meta: id,
}));

Who does this impact? Who is this for?

All TypeScript users

Additional context (optional)

createAction and createStandardAction will be deprecated because new createAction will be able to replace their function.

In v5, the old API will be kept in deprecated import so incremental migration is possible:

import { deprecated } from "typesafe-actions";

const { createAction, createStandardAction } = deprecated;

IssueHunt Summary

Backers (Total: $200.00)

Become a backer now!

Or submit a pull request to get the deposits!

Tips


IssueHunt has been backed by the following sponsors. Become a sponsor

@piotrwitek piotrwitek added this to the v5.0.0 milestone Apr 22, 2019
@piotrwitek piotrwitek self-assigned this Apr 22, 2019
@piotrwitek piotrwitek pinned this issue Apr 22, 2019
@piotrwitek piotrwitek changed the title Rewrite and simplify `createAction` API using generic tuple types v5.0.0 - Rewrite and simplify `createAction` API using generic tuple types Apr 22, 2019
@piotrwitek piotrwitek changed the title v5.0.0 - Rewrite and simplify `createAction` API using generic tuple types v5.0.0 - Rewrite and simplify `createAction` API Apr 23, 2019
@IssueHuntBot

This comment has been minimized.

Copy link

@IssueHuntBot IssueHuntBot commented Apr 23, 2019

@IssueHunt has funded $150.00 to this issue.


@sjsyrek

This comment has been minimized.

Copy link
Contributor

@sjsyrek sjsyrek commented Apr 26, 2019

This is not related to #116, right?

@piotrwitek

This comment has been minimized.

Copy link
Owner Author

@piotrwitek piotrwitek commented Apr 26, 2019

createAction and createCustomAction will be deprecated.

@sjsyrek It means it would become unnecessary

It's still just a proposal, It's not finalized yet, open to discussion.

@piotrwitek

This comment has been minimized.

Copy link
Owner Author

@piotrwitek piotrwitek commented Apr 28, 2019

I updated the proposal with improved createAsyncAction API that will resolve 2 existing feature requests:

@davidgovea

This comment has been minimized.

Copy link

@davidgovea davidgovea commented May 10, 2019

This looks great!

Edit - is there a way to set the error: boolean parameter of a FSA in this approach? These purpose-built payload and meta creation functions are more specific than the general "mapper" approach from before -- maybe I'm missing something

@piotrwitek

This comment has been minimized.

Copy link
Owner Author

@piotrwitek piotrwitek commented May 10, 2019

Hey @davidgovea, please help me to figure out this feature.

I need to understand your use-case and how you would like to use this feature from a user perspective.
Could you send me a real usage example of how you're using the error flag with a current v4 API?
Please include actions, reducer and any other code that is invoking actions and setting error flag (I need to see what parameters you're passing to action creator with their types).

Thanks

@paolostyle

This comment has been minimized.

Copy link

@paolostyle paolostyle commented May 16, 2019

Would something like this work with the new API?

const action = createAsyncAction(
  'REQ_TYPE',
  ['SUCCESS_TYPE', res => {
    // do something with res
  }],
  'FAILURE_TYPE'
)<[Request, Meta], Response, [Error, Meta]>;

If not I guess I could simply pass identity function with the type in 1st and 3rd param, but something like this would also be pretty useful.

@piotrwitek

This comment has been minimized.

Copy link
Owner Author

@piotrwitek piotrwitek commented May 16, 2019

Hey @paolostyle, that's an interesting idea, I think that might be possible. I haven't thought of it and I really appreciate your feedback.

I'll be working on the implementation this weekend so I'll give it a shot.

If that would work I think I would drop the second variant and use only the version from your proposal as it's basically solving both use-cases with one function which is a superior API.

Challenge 1:

One limitation is it'll only allow mapper functions with a single argument. I think it might still be ok because the previous version of async action has the same limitation and practically no one was complaining about it, so I guess most users can live without it.

Possible solution:

const action = createAsyncAction(
  'REQ_TYPE',
  ['SUCCESS_TYPE', (res, meta) => {
    // do something with res
  }],
  'FAILURE_TYPE'
)<[Request, Meta], (res: Response, meta: Meta) => Results, [Error, Meta]>;
@bboydflo

This comment has been minimized.

Copy link

@bboydflo bboydflo commented May 30, 2019

will v5 release fix createAction to handle optional payload/meta? I think currently it doesn't.

// this keeps the correct type
export const decrement1 = (payload?: number) => action(DECREMENT, payload)

// while this doesn't
export const decrement2 = createAction(DECREMENT, action => {
  return (payload?: number) => action(payload);
});
@piotrwitek

This comment has been minimized.

Copy link
Owner Author

@piotrwitek piotrwitek commented May 30, 2019

@bboydflo yes you are correct, createAction is currently not supporting optional payload type correctly, it'll be fixed in new API.

@jaridmargolin

This comment has been minimized.

Copy link

@jaridmargolin jaridmargolin commented Aug 20, 2019

Hi @piotrwitek - These changes look great. I wanted to quickly check-in and see how progress was coming.

Thank you for all the hard work :)

@piotrwitek

This comment has been minimized.

Copy link
Owner Author

@piotrwitek piotrwitek commented Sep 20, 2019

Just returned from a Summer Break, will come back to this real soon!

@jstheoriginal

This comment has been minimized.

Copy link

@jstheoriginal jstheoriginal commented Oct 7, 2019

Not sure if this is isolated to 5.0.0-3 or not (if you suspect it's not just let me know and I'll make a new issue), but createReducer allows you to add extra items than what's on the defined state. It properly type-checks keys defined in the state type for the reducer...it just allows extra keys (not the end of the world, unless someone makes a spelling mistake).

Example (the extra property has no typescript errors reported..doing anything invalid to the other two properties properly shows errors):

export type AppState = {
  previous: AppStateStatus | null;
  current: AppStateStatus;
};

export type AppStateReducerState = DeepReadonly<{
  appState: AppState;
}>;

export const defaultAppReducerState: AppStateReducerState = {
  appState: {
    previous: null,
    current: 'active',
  },
};

export const appStateReducer = {
  appState: createReducer(defaultAppReducerState.appState).handleAction(
    setAppState,
    (state, { payload }) => ({
      previous: state.current,
      current: payload,
      extra: 'why is this allowed?',
    }),
  ),
};
@piotrwitek

This comment has been minimized.

Copy link
Owner Author

@piotrwitek piotrwitek commented Oct 7, 2019

@jstheoriginal It's a good find, thanks for reporting.
It would be great to have it as a separate issue to make sure it's a known problem.

@mAAdhaTTah

This comment has been minimized.

Copy link
Contributor

@mAAdhaTTah mAAdhaTTah commented Oct 16, 2019

@jstheoriginal Not specific to 5.0. I remember this coming up when I attempted to use createReducer once before. Did you open an issue? If not, I can.

@jonrimmer

This comment has been minimized.

Copy link

@jonrimmer jonrimmer commented Oct 17, 2019

Hey, thanks for the great library! It makes working with using react-redux with TS a lot better. I was wondering if you'd seen the TypeScript action creator code in NgRx, the Angular redux library? It does a good job of avoiding boilerplate, while retaining type safety for things like reducers. Might be a source of inspiration.

@issuehunt-app

This comment has been minimized.

Copy link

@issuehunt-app issuehunt-app bot commented Oct 17, 2019

@jonrimmer has funded $50.00 to this issue.


@piotrwitek

This comment has been minimized.

Copy link
Owner Author

@piotrwitek piotrwitek commented Oct 17, 2019

Hey @jonrimmer,
Thanks for adding to the funding, that's really appreciated!
Regarding your suggestion, I haven't seen it, could you link to some more advanced usage examples/scenarios that are including payloads, meta, async actions so I could compare with new API proposal and check for possible improvements? Thanks!

@jonrimmer

This comment has been minimized.

Copy link

@jonrimmer jonrimmer commented Oct 17, 2019

No problem! The actual creators are fairly simple, they work like your examples above. The main thing I noticed is that they've managed to get type inference on the reducer functions without having to register root actions or extend the module, e.g.

const scoreboardReducer = createReducer(
  initialState,
  on(loadTodos, state => ({ ...state, todos })),
  on(ScoreboardPageActions.awayScore, state => ({ ...state, away: state.away + 1 })),
  on(ScoreboardPageActions.resetScore, state => ({ home: 0, away: 0 })),
  on(ScoreboardPageActions.setScores, (state, { game }) => ({ home: game.home, away: game.away }))
);

So, similar to createReducer() and handleAction() in the current version of typesafe-actions, just slightly smoother. Not sure how they do it though!

@piotrwitek

This comment has been minimized.

Copy link
Owner Author

@piotrwitek piotrwitek commented Oct 17, 2019

@jonrimmer actually that automatic inference is easy to add, what differentiate typesafe-actions from all other libraries is the fact that createReducer knows about all valid actions in the system (that's why you register RooAction) and informs you in real-time which actions are already handled in this reducer and which not allowing for quicker understanding of the bigger picture without a need to browse through the files containing declarations of actions. I can guess this is most certainly not the case in the above example because it is impossible without extra type parameter providing the constraint type.

@piotrwitek

This comment has been minimized.

Copy link
Owner Author

@piotrwitek piotrwitek commented Oct 28, 2019

  • Feature is completed and will be published today as beta v5.1.0-0.
  • Migration guide is ready #191
  • Documentation will be updated later, so for now please use examples found in the first comment #189
@Sir-hennihau

This comment has been minimized.

Copy link

@Sir-hennihau Sir-hennihau commented Dec 11, 2019

For anyone coming from search engines with the following bug:

argument 1 is invalid it should be an action-creator instance from typesafe-actions

My mistake was that I didn't see the docs were not updated yet. I solved it by using the new createAction() API given by @piotrwitek in the first post of this thread.

Example:

export const updateProviders = createAction(
  "UPDATE_PROVIDERS",
  (provider: Provider) => provider,
)<Provider>();

Go and checkout the other breaking changes https://github.com/piotrwitek/typesafe-actions/releases which are not in the official docs yet, too.

@mAAdhaTTah

This comment has been minimized.

Copy link
Contributor

@mAAdhaTTah mAAdhaTTah commented Dec 12, 2019

Maybe this should be a new issue, but I'm not sure I understand the benefit/point of the extra function call in createAction. Given the above example:

export const updateProviders = createAction(
  "UPDATE_PROVIDERS",
  (provider: Provider) => provider,
)<Provider>();

The generic Provider is... provided... twice. Does this actually help the typing? I'm curious why this is necessary.

@piotrwitek

This comment has been minimized.

Copy link
Owner Author

@piotrwitek piotrwitek commented Dec 24, 2019

@mAAdhaTTah good question.
Looking at the above example you don't want to use extra function call in that case, because it's not designed for it. There is a simpler way to do that:

export const updateProviders = createAction(
  "UPDATE_PROVIDERS",
)<Provider>();

But it can be useful in other more complex cases, especially when you want to set the constraints for the types you expect.

Consider this:

const deleteUser = createAction(
  'TYPE1',
  (user: User) => user.id,
)<string>();

deleteUser(userObject) // { type: 'TYPE1', payload: string }

This will create an action creator that will accept argument user of type User, but the payload will be string.
Hope that is clear!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.