Make createActionCreator simpler by removing `resolve` callback from executor #122

the-dr-lazy · 2019-09-11T17:30:00Z

From TypeScript 3.0 we are able to infer function's parameters with their names. So now we can remove resolve callback from executor in createActionCreator to make the API simpler.

Example:

// (name: string) => { type: "FETCH_TODOS"; }
createActionCreator("FETCH_TODOS");

// (name: string) => { type: "ADD_TODO"; payload: string; }
createActionCreator("ADD_TODO", (name: string) => ({ payload: name }));

There is one caveat point that has been encountered in #108 and that's type property in return type of callable (2nd argument of proposed createActionCreator). One possible solution for the mentioned problem which I think is ideal is to set the type of type property to undefiend.

// Types of property 'type' are incompatible.
// Type 'string' is not assignable to type 'undefined'.ts(2345)
createActionCreator("ADD_TODO", (name: string) => ({ type: 'MISTAKE', payload: name }));

We can call the callable's return type TypelessAction as it's type should be undefined.

The text was updated successfully, but these errors were encountered:

the-dr-lazy · 2019-09-11T17:41:30Z

@LouizFC @Haaxor1689 @rlesniak can we have you guys here?

the-dr-lazy · 2019-09-11T17:56:02Z

I created a codesandbox as a demo for the proposed API.

the-dr-lazy · 2019-09-11T18:08:33Z

I clicked "close issue" button by mistake. :D

LouizFC · 2019-09-13T11:47:28Z

@thebrodmann I think that the current API is fine and removes the boilerplate of setting error: true on Error payloads.

But also I am not agains't it. The only "resistance" I have on this change is by my experience with #106 . I found very difficult to extract and modify return types correctly, but if you get it right I have no objections.

Edit: Also, backwards compatibility
Edit2: Also, typescript playground now supports types from libraries, I think it is better than codesandbox when showcasing types.

the-dr-lazy · 2019-09-13T12:43:26Z

I think that the current API is fine and removes the boilerplate of setting error: true on Error payloads.

The only thing can be returned by callable is an object by payload and meta properties then we create an action by them.

Also, backwards compatibility

Yeah, unfortunately, this is not backward compatible but I think it makes the createActionCreator API more simple and also gives us an opportunity to make #108 API simpler. I will try to create a playground for createActionCreatorFactory with the proposed API.

typescript playground now supports types from libraries, I think it is better than codesandbox when showcasing types.

Ah, thank you for that, It's. This playground can show the proposed API.

the-dr-lazy · 2019-09-13T13:49:16Z

Also in this playground the extra properties are not allowed.

LouizFC · 2019-09-16T15:09:25Z

The proposed API looks fine, my only nitpick is: If you try to preview the arguments both on playground or IDE, the arguments for the function will show up as ...args: any[] (instead of, for example, name: string for ADD_TODO). I think it is a bug with the typescript language server, because you can't pass anything other than string to ADD_TODO, which is correct.

As I expressed earlier, I think we should not mess with createActionCreator because it will break backwards compatibility. I know it is a simple change, but it will require a huge amount of refactoring on my side.

the-dr-lazy · 2019-09-17T08:49:59Z

If you try to preview the arguments both on playground or IDE, the arguments for the function will show up as ...args: any[] (instead of, for example, name: string for ADD_TODO).

Actually it should work! Is your TypeScript version above 3.0?

I think we should not mess with createActionCreator because it will break backwards compatibility.

Maybe we can be backward compatible. I will try to create a backward-compatible playground soon.

LouizFC · 2019-09-17T19:25:53Z

Actually it should work! Is your TypeScript version above 3.0?

It "works", when pressing Ctrl to see the types I get them right, also the types are applied correctly, but when you "request" the argument types, they come as ...args: any or empty, as I will show you below:

When you press Ctrl + Q on Webstorm

When you press a new parenthesis in front of a function on the playground

But this is not a blocking issue, probably just a bug in typescript language server. I will try to report it somehow

Maybe we can be backward compatible

Thank you. I think this is the best way

kotarella1110 · 2019-12-13T09:39:38Z

Hey @thebrodmann !

Also in this playground the extra properties are not allowed.

The error message caused by specifying extra properties feel strange.

I think it is better to change return value type of callable function from "{ payload?: any, meta?: any }" to never.

from:

callable?: (...args: TArguments) => TReturn & (Exclude<keyof TReturn, keyof TypelessAction> extends never ? TReturn : "{ payload?: any, meta?: any }"

to:

callable?: (...args: TArguments) => TReturn & (Exclude<keyof TReturn, keyof TypelessAction> extends never ? TReturn : never)

What do you think about this?

the-dr-lazy · 2019-12-13T16:55:32Z

The error message caused by specifying extra properties feel strange.

What makes you think so?

kotarella1110 · 2019-12-16T04:33:23Z

The ideal error and error message are as follows.

error:

error message:

The ideal return value type of callable function is TypelessAction , but the current return value type is { type: string; payload: string; } & "{ payload?: any, meta?: any }"
I understand that the implementation is difficult because of the use of generics.
Even if the return value type can be set to TypelessAction, it will be difficult to implement unless this issue(microsoft/TypeScript#241) is fixed.

However, { type: string; payload: string; } & "{ payload?: any, meta?: any }" feel a little forced.
never feel better.

the-dr-lazy · 2019-12-16T05:27:27Z

Type '{ type: string; payload: string; }' is not assignable to type 'never'.

the-dr-lazy · 2019-12-16T05:27:35Z

Type '{ type: string; payload: string; }' is not assignable to type '"{ payload?: any, meta?: any }"'.

the-dr-lazy · 2019-12-16T05:31:34Z

Type '{ type: string; payload: string; }' is not assignable to type '"{ payload?: any, meta?: any }"'.

I think this error message is more descriptive than the former one. BTW, I can't make a decision on it in this way. So let's vote on it. Please add a 👍 on the error message that you think is better.

the-dr-lazy · 2020-03-04T17:05:18Z

I will try to create a backward-compatible playground soon.

@LouizFC
The backward-compatible version Playground.

the-dr-lazy mentioned this issue Sep 11, 2019

Implement ActionCreatorFactory and ReducerFactory #108

Closed

the-dr-lazy closed this as completed Sep 11, 2019

the-dr-lazy reopened this Sep 11, 2019

the-dr-lazy changed the title ~~Make createActionCreator simpler by remove resolve callback from executor argument~~ Make createActionCreator simpler by removing resolve callback from executor argument Sep 12, 2019

the-dr-lazy changed the title ~~Make createActionCreator simpler by removing resolve callback from executor argument~~ Make createActionCreator simpler by removing resolve callback from executor Sep 12, 2019

the-dr-lazy added good first issue Good for newcomers help wanted Extra attention is needed labels Dec 5, 2019

the-dr-lazy mentioned this issue May 15, 2020

Explain or remove resolve => x => resolve(x) pattern #148

Closed

the-dr-lazy mentioned this issue Oct 27, 2020

handleAction allows you to return a state object with any additional properties #123

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make createActionCreator simpler by removing `resolve` callback from executor #122

Make createActionCreator simpler by removing `resolve` callback from executor #122

the-dr-lazy commented Sep 11, 2019 •

edited

Loading

the-dr-lazy commented Sep 11, 2019

the-dr-lazy commented Sep 11, 2019 •

edited

Loading

the-dr-lazy commented Sep 11, 2019

LouizFC commented Sep 13, 2019 •

edited

Loading

the-dr-lazy commented Sep 13, 2019 •

edited

Loading

the-dr-lazy commented Sep 13, 2019 •

edited

Loading

LouizFC commented Sep 16, 2019

the-dr-lazy commented Sep 17, 2019

LouizFC commented Sep 17, 2019

kotarella1110 commented Dec 13, 2019

the-dr-lazy commented Dec 13, 2019

kotarella1110 commented Dec 16, 2019 •

edited

Loading

the-dr-lazy commented Dec 16, 2019

the-dr-lazy commented Dec 16, 2019

the-dr-lazy commented Dec 16, 2019

the-dr-lazy commented Mar 4, 2020

Make createActionCreator simpler by removing resolve callback from executor #122

Make createActionCreator simpler by removing resolve callback from executor #122

Comments

the-dr-lazy commented Sep 11, 2019 • edited Loading

the-dr-lazy commented Sep 11, 2019

the-dr-lazy commented Sep 11, 2019 • edited Loading

the-dr-lazy commented Sep 11, 2019

LouizFC commented Sep 13, 2019 • edited Loading

the-dr-lazy commented Sep 13, 2019 • edited Loading

the-dr-lazy commented Sep 13, 2019 • edited Loading

LouizFC commented Sep 16, 2019

the-dr-lazy commented Sep 17, 2019

LouizFC commented Sep 17, 2019

kotarella1110 commented Dec 13, 2019

the-dr-lazy commented Dec 13, 2019

kotarella1110 commented Dec 16, 2019 • edited Loading

the-dr-lazy commented Dec 16, 2019

the-dr-lazy commented Dec 16, 2019

the-dr-lazy commented Dec 16, 2019

the-dr-lazy commented Mar 4, 2020

Make createActionCreator simpler by removing `resolve` callback from executor #122

Make createActionCreator simpler by removing `resolve` callback from executor #122

the-dr-lazy commented Sep 11, 2019 •

edited

Loading

the-dr-lazy commented Sep 11, 2019 •

edited

Loading

LouizFC commented Sep 13, 2019 •

edited

Loading

the-dr-lazy commented Sep 13, 2019 •

edited

Loading

the-dr-lazy commented Sep 13, 2019 •

edited

Loading

kotarella1110 commented Dec 16, 2019 •

edited

Loading