Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Http toTask missing #81

Open
eriklott opened this issue Apr 20, 2018 · 9 comments
Open

Http toTask missing #81

eriklott opened this issue Apr 20, 2018 · 9 comments

Comments

@eriklott
Copy link

I was hoping to chain a few http requests into a task when I noticed that Elm's Http.toTask function hasn't been adopted (or is missing).

I attempted to split the current Http.send function into 2 parts myself:

  1. Http.toTask - converts the request into a task
  2. Http.send - under the hood, basically does Http.toTask req |> Cmd.attempt msg

... and ran into a problem. Because of the way that Http.send is also being used to track progress events, I don't believe the send function can technically be pulled apart into a task - correct me if I'm wrong. See the following problem lines:

https://github.com/OvermindDL1/bucklescript-tea/blob/master/src/tea_http.ml#L108-L117

I'm torn. If we can't chain api requests into a single task, we'll need to rethink how we transition pages in our SPA (arggg). At the same time, I always found Elm's progress events awkward and disconnected.

Is there a solution that I'm not seeing?

@OvermindDL1
Copy link
Owner

It can be pulled apart but it will definitely require a lot of alterations to the overall code, basically rewriting a good chunk of it, but it's not too hard. Might be better to make another function that acts like send but returns a task instead for now, but hmm... This one was more made to get the functionality in rather than copy the Elm api of it, does need more work...

At the same time, I always found Elm's progress events awkward and disconnected.

Very true, do you have ideas for a better interface for that all?

@eriklott
Copy link
Author

eriklott commented Apr 21, 2018

That's tough. Conceptually, it seems to me that as soon as we start dealing with Http requests as Tasks, progress events don't seem to fit well into that story. First, a task chain could consist of mixed infrastructure types -e.g. db Task, http Task, 'b Task (making stuff up here), and I'm not sure what "progress" means in that case. Second, if I've chained a bunch of tasks together, I think I only care about the success or failure of the chain as a whole. I can't think of a scenario where I'd want the progress of a single task within a chain of tasks...

So if the possibility of receiving progress events is removed from http request tasks, then by association, they're also removed from the Commands created from those tasks, e.g. Cmd.attempt. I guess that leaves us with creating a specialized function that includes progress reporting, maybe something like this:

// create a task from an http request
val to_task : request -> task

// create a command from an http request. ( under the hood creates a task via to_task, and calls Cmd.attempt )
val send : request -> (result -> msg) -> msg Cmd

// a new result type for the `send_with_progress` function
type result_with_progress =
   | Ok 'a
   | Progress of {progress: float ; total: float}
   | Error ...

// create a command from an http request. The resulting messages include progress.
val send_with_progress : request -> (result_with_progress -> msg) -> msg Cmd

Thoughts?

@OvermindDL1
Copy link
Owner

That's tough. Conceptually, it seems to me that as soon as we start dealing with Http requests as Tasks, progress events don't seem to fit well into that story.

Ahh, super good point... I wonder if there is a good way that can be thought of to let tasks send event (though then that breaks the 'immutability' style of things)...

A subscription is the 'elmy' abstraction for something that can send updates. I do have a note of Might still want to make a subscription variant though... to do specifically that (wasn't Elm lacking that ability as well) and just let the user parallize and so forth internally for whatever style they are wanting (while retaining the current Elm'y API as well for quick usages). Might just need to finish making that then. :-)

Thoughts?

Useful API, but a message cannot be sent to the main application from a task until it completes. A Cmd could do that, but it's not expected that it 'could' even do that as it generally only returns a single message, though we definitely could technically, just expectedly I'm unsure if it is the proper course? Thoughts?

@eriklott
Copy link
Author

Useful API, but a message cannot be sent to the main application from a task until it completes

Definitely. I hope it didn't seem like I was suggesting that.

// a new result type for the `send_with_progress` function
type result_with_progress =
   | Ok 'a
   | Progress of {progress: float ; total: float}
   | Error ...

// create a command from an http request. The resulting messages include progress.
val send_with_progress : request -> (result_with_progress -> msg) -> msg Cmd

Just to clarify, the send_with_progress function would return a command directly, with no task intermediary, much like your current Html.send does.

Another alternative: send_with_progress could take a request, and message for the result, and a message for progress, returning a command:

type progress =
    { bytes : int
    ; bytesExpected : int
    }

val send_with_progress : request -> (result -> 'msg) -> (progress -> 'msg) -> 'msg Cmd

This is more in line with how you're using the Html.Progress module, but explicitly merges the intention into a single function.

The other way is to use Subs explicitly like Elm does - example. This bothers me too. It was my feeling that Subs were for listening to events in the outside infrastructure, NOT initiating them. Using Subs to MAKE http requests feels like a break in that concept. At least with some type of send_with_progress function, we're being explicit.

@OvermindDL1
Copy link
Owner

Indeed, I wish that the 'Router' concept was exposed from Elm (it's only exposed to Elm's mis-named effect systems), perhaps it might be worth doing that, then the user can send whatever message they want any time they want, though how to expose it other than passing it in to the update function itself (and maybe subscriptions and view and init?). Elm is just such a limited API, I really want to break from it (and further modules of course will, but the current base set of modules should match), but for now we should probably just copy what it does, subscriptions... It might be worth making a Cmd version of it that gives back multiple events, even though that seems to break the 1-to-1 Cmd-to-Msg concept, but if it is documented as such, perhaps especially via an explicit argument that the user has to set up to cause it to happen, then it should be good?

@eriklott
Copy link
Author

Whether the final api uses Subs or Cmds, the conceptual outcome is the same: the infrastructure initiates a request, and spews events (progress, ok, err) back to the app in the form of a union msg (or multiple msg types if you choose that direction - personally I prefer the union).

It might be worth making a Cmd version of it that gives back multiple events, even though that seems to break the 1-to-1 Cmd-to-Msg concept

IMHO I don't think there is a 1-to-1 concept. It just so happens that the overwhelming majority of commands work out this way. Once the app sends a Cmd to the infrastructure, what the infrastructure chooses to do with that request is up to it, as long as it obeys the contract and returns a message back to the app using the intended type.

@OvermindDL1
Copy link
Owner

IMHO I don't think there is a 1-to-1 concept. It just so happens that the overwhelming majority of commands work out this way. Once the app sends a Cmd to the infrastructure, what the infrastructure chooses to do with that request is up to it, as long as it obeys the contract and returns a message back to the app using the intended type.

Hmm, true, this would likely be the better API overall...

@neochrome
Copy link
Contributor

Is there any progress on this yet?
It would be super nice to have to possibility to chain http reqests etc as tasks :)
Right now I'll just bounce through some messages instead, but that get boring quite fast 😄

@OvermindDL1
Copy link
Owner

I think it was mostly just a conversation to discuss ideas but no PR or work done on it yet, a PR is welcome, especially from someone that uses the feature so I know it is implemented in a useful way? :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants