developit/unistore

Typing of setState and ActionFn

shabbyrobe opened this issue · 1 comments

I would like to propose some enhancements to the TypeScript typings for Unistore, but I thought raising an issue might be a better place to start rather than a PR just in case my proposal is completely wrong!

I think setState can potentially be improved by changing it like so:

    setState(update: K, overwrite?: true, action?: Action<K>): void;
    setState(update: Partial<K>, overwrite?: false, action?: Action<K>): void;

This correctly raises an error when you try to call setState in overwrite mode without a full object, and allows you to call setState in update mode without a full object. The drawback is that the error that gets raised when you accidentally use (Partial<K>, true) is somewhat unhelpful, but perhaps some documentation on the method in the typing file might be sufficient? The error in that situation is this:

Type error: Argument of type 'true' is not assignable to parameter of type 'false | undefined'. TS2345

It would definitely be better if the error was something like "overload could not be resolved with the arguments you supplied" or something. Maybe that's one for the TypeScript issue tracker though.

The other possible improvement I noticed was that ActionFn could possibly be changed like so:

-    export type ActionFn<K> = (state: K) => object;
+    export type ActionFn<K> = (state: K) => Partial<K>;

This would allow the return value to be checked for intersection with state.

I'm not 100% sure these suggestions are accurate and wanted to kick off a discussion here just in case I'm breaking something for somebody!

Actually, just taking another look at the source; perhaps the action changes maybe need to apply to Action<K> as well, with the return type being Partial<K> | Promise<Partial<K>> | void?