Skip to content
This repository has been archived by the owner on Jul 30, 2018. It is now read-only.

Poor typing usability #21

Closed
novemberborn opened this issue Sep 30, 2016 · 2 comments
Closed

Poor typing usability #21

novemberborn opened this issue Sep 30, 2016 · 2 comments

Comments

@novemberborn
Copy link
Contributor

While working on dojo/examples#56 I found it nearly impossible to correctly type actions.

There is an assumption that the options passed to the do() implementation have an event object, and that object has a target property. That makes sense when actions are used as event listeners but often actions call other actions, passing an arbitrary object.

Furthermore, the do() method is assumed to return a Task which resolves with the (same type as) the target.

Correctly changing the typing of an action requires three types (DoOptions, TargettedEventObject and ActionState). This requires a fair amount of understanding of the action internals.

Typing an action which handles keypress events is nearly impossible since the KeyboardEvent type is not compatible with the TargettedEventObject. It has a target of course (an HTMLElement), but the do() implementation may return an entirely different object.


I'd like to see an interface closer to this:

interface ActionMixin<T, O> {
  do(options?: O): Task<T>;
}

type Action<T, O> = Stateful<ActionState> & ActionMixin<T, O>;

The default createAction export should create Action<any, any> instances. It should take type arguments, e.g. createAction<any, KeyboardEvent>() should create an action that can handle keyboard events. In the TodoMVC example, a createAction<any, Item>() should create an action that is called with todo items.

We could export other constructors to make it easier to create the current "targeted event" actions.


I'm not sure whether ActionState deserves to be a generic. It's tempting but we can't provide default values for generics, and the more arguments Action takes the harder it is to customize the typing.

@kitsonk
Copy link
Member

kitsonk commented Oct 6, 2016

  • Promise not Task
  • Callers should NOT care about the return value, no matter what.
  • .do() executor should always return Promise<void>
  • Type the .do() arguments to be more generic and flexible
  • Work better with dojo-app by typing registryProvider in factory options and .configure()
  • Generic registryProvider

@dylans dylans added this to the 2017.02 milestone Jan 12, 2017
@dylans dylans modified the milestones: 2017.03, 2017.02 Feb 19, 2017
@dylans dylans modified the milestones: 2017.04, 2017.03 Mar 25, 2017
@dylans dylans modified the milestones: 2017.05, 2017.04 Apr 8, 2017
@dylans dylans modified the milestones: 2017.05, 2017.06 May 5, 2017
@dylans dylans modified the milestones: 2017.06, 2017.08 Jul 4, 2017
@dylans dylans modified the milestones: 2017.08, 2017.09 Aug 4, 2017
@kitsonk
Copy link
Member

kitsonk commented Oct 9, 2017

Closing as we would address this if we would ever revive this package.

@kitsonk kitsonk closed this as completed Oct 9, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants