deox
deox copied to clipboard
the-dr-lazy
Make createActionCreator simpler by removing `resolve` callback from executor
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.
@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.
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 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.
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.
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
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 error message caused by specifying extra properties feel strange.
What makes you think so?
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(https://github.com/microsoft/TypeScript/issues/241) is fixed.
However, { type: string; payload: string; } & "{ payload?: any, meta?: any }" feel a little forced.
never feel better.
Type '{ type: string; payload: string; }' is not assignable to type 'never'.
Type '{ type: string; payload: string; }' is not assignable to type '"{ payload?: any, meta?: any }"'.
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 :+1: on the error message that you think is better.
I will try to create a backward-compatible playground soon.
@LouizFC The backward-compatible version Playground.