origamitower / folktale

[not actively maintained!] A standard library for functional programming in JavaScript
https://folktale.origamitower.com/
MIT License
2.04k stars 102 forks source link

(docs) Add tentative type annotations to Task methods #94

Closed justin-calleja closed 7 years ago

justin-calleja commented 7 years ago
  1. The constructor type annotations are in need of guidance.
  2. Special attention needs to be given to and and or type annotations.
    • and, got from here. Was going to go with: (Task a b).(Task c d) => (Task a _) or (Task c _) or (Task _ (b, d))) - why is the rejected part of both Tasks the same type a?
justin-calleja commented 7 years ago

Also, from here, I could not figure out the difference between fat arrow (=>) and arrow (->).

robotlolita commented 7 years ago

The function notation is (arg1, …, argN) => resultType, so only => arrows are used.

As for the type of or/and, the error type is the same for simplicity. While in JS you can use taskA.or(taskB) and have the error types in taskA and taskB be different, for simplicity we assume a more restricted system where logical type unions are less prevalent.

Ideally this'd encourage people to keep their types a bit more consistent, too, as something like (Task String Number).or(Task String Number) leads to simpler code (the type is always the same) than something like (Task String String).or(Task Error (Array Number)), where one'd have to use typeof and other checks, and write different branches for each case.

So, for that reason, we kinda arbitrarily restrict the types to be a bit more consistent:

or :: (Task a b).(Task a b) => Task a b
and :: (Task a b).(Task a c) => Task (a, c)

These types should also include the resources. Each task has an unique resource associated with it, which is consumed when you run that task (automatically by the TaskExecution). There's no real type notation for this concept yet (linear types), so they can be simplified by just putting the resource type on the LHS and RHS.

The constructor's type would be:

Task :: forall value, reason, resources:
  new ( ({ resolve: (value) => Void, reject: (reason) => Void, cancel: () => Void }) => resources
  , (resources) => Void
  , (resources) => Void
  ) => Task value reason resources 

And then each method transforming a task just keeps the same resource:

Task.map :: forall v1, v2, e, r: (Task v1 e r).((v1) => v2) => Task v1 e r

Methods combining tasks yield a combination of both resources:

Task.or :: forall v, e, r1, r2: (Task v e r1).(Task v e r2) => Task v e (r1 and r2)
justin-calleja commented 7 years ago

@robotlolita thanks! The changes based on your feedback are WIP.

justin-calleja commented 7 years ago

btw, re documenting Task based on Result (gitter discussion) - I have started with type annotations and looking at tests.

Next, I'll be looking at docs/source/en/data/result. If I'm comfortable with writing docs for Task based on that, I'll open a separate PR for that (that is what I think you were referring to in Gitter - but I didn't even know about the whole re-work of Task in Folktale 2... so need to get familiar before even giving it a shot). - mentioning this in case you have other priorities on what to work on ;)

justin-calleja commented 7 years ago

@robotlolita

Shouldn't this:

and :: (Task a b).(Task a c) => Task (a, c)

be:

and :: (Task a b).(Task a c) => Task a (b, c)

?

safareli commented 7 years ago

and :: (Task a b).(Task a c) => Task a (b, c) is correct

justin-calleja commented 7 years ago

Thanks @safareli !

robotlolita commented 7 years ago

Ah, yes, sorry, my and type was incorrect :)

justin-calleja commented 7 years ago

@robotlolita np

I was going to bring this up in my next push, but it might take a bit longer than expected so:

I noticed you've switched the rejected and resolved values in the type annotations.

i.e. data.Task was Task rejectedValue resolvedValue

but in your e.g. above it's Task v1 e r where I take it that v1 is resolved value, e is rejected value, and r is resources.

I've started the re-write of the type annotations using this new way (resolved value first) - as I figured, since Tasks are now created differently, perhaps it makes more sense to have the resolved values first.

Also, I personally like having resolution types first - but wanted to clear this up. In any case, it won't be a big deal to change again in case the above was just a typo.

Note, one thing that came to mind as a possible "inconsistency":

bimap takes the rejectionTransformation first which may be a bit misleading when the type of Task takes a resolution type as a first param (not a rejected one).

justin-calleja commented 7 years ago

@robotlolita I'm not sure the changes I made are exactly what you had in mind.

Some things to look out for:

  1. Make sure I properly translated: "transforming of tasks keep the same resource" and "combining of tasks yield combination of both" in the changes.
  2. I assumed TaskExecution would also take the resources in its type so run returns a TaskExecution v e r.
  3. Again, as mentioned in previous comment above, the type of the resolved value now comes first.
  4. re of and rejected, there doesn't seem to be any resources in the type but I kept r there for consistency. Not sure if this is the right way to annotate those methods.
robotlolita commented 7 years ago

That's pretty much it. In the places where there're two independent tasks, and the result is one of them, each task gets its own resource, and the resulting type has the resource of the task returned.

The resolved value part was an error on my part, sorry :< It doesn't really matter as long as the types line up, but as you mentioned it becomes inconsistent with things like bimap, fold and stuff that use more positional arguments.

justin-calleja commented 7 years ago

@robotlolita np I'll make the changes re the resolved value position.