m.request: Add abort

[sdemjanenko]

It would be awesome if I could abort a running request. A use case would be if I start a request and then want to navigate out of the controller that started the request.

[lhorie]

Technically this is possible with the config option, but, yeah, it's kinda clunky:

var Aborter = function() {
  var transport;
  this.config = function(xhr) {
    transport = xhr;
  }
  this.abort = function() {
    transport.abort();
  }
}

var aborter = new Aborter();
m.request({method: "GET", url: "/foo", config: aborter.config});
aborter.abort();

I'll mark this as an enhancement request

[lhorie]

Note to self: as per convo in #62, need to be able to support the use case of potentially expensive requests that live through route changes.

[lhorie]

I was thinking about this last night, and it occurred to me that we could just use m.prop to achieve the same effect as above.

var transport = m.prop();
m.request({method: "GET", url: "/foo", config: transport});
transport().abort();

This allows us to just expose the raw xhr, which then can be used for arbitrary low-level computations (e.g. aborting, attaching onprogress handlers, etc). Does that seem clean enough?

I'm reluctant to add an API point at the promise level because it would create an ugly abstraction leak related to the backwards ripple effect of aborting a downstream promise (there was a long discussion on this topic somewhere in the Angular issues page last year, and I'm not a huge fan of their solution)

[sdemjanenko]

I think that should be fine.

[Raynos]

@lhorie why not have m.request return an xhr object AND a promiselike.

That's what my var rawBrowserXHRObject = xhr(opts, cb) function does. It allows people access to the raw object without special casing.

Maybe return { then: fn, xhr: rawXhr }

[lhorie]

@Raynos Hmm, that's basically a variation of the backwards ripple issue I mentioned above, e.g.

var root = m.request(args)

var a = root
  .then(doSomething)
  .then(doAnother)

var b = root
  .then(doSomethingElse)
  .then(doMoreStuff)

//later
b.xhr.abort() //this rejects a

If you're clever enough, you can probably figure out that the xhr property refers to the root of the promise chain, and is not a property of the current promise per se. This deduction can be made more easily than in the Angular case I mentioned (because they were calling it cancel, which is semantically confusing), but it's still a bit treacherous: You're less likely to think about the sibling chain A when your attention is on promise B (whereas explicitly working with a root reference implies you are aware of implications on all downstreams).

Btw, I'm not sure if what you meant was to make this type mixin non-infectious, in which case, you'd likely be doing things like:

var ctrl = function() {
  var usersReq = m.request(args)
  this.users = usersReq.then(filterByWhatever)

  //later
  usersReq.xhr.abort()
}

I can see the xhr property being less verbose for the super simple case, but I'm not super convinced that the idiom above is much better than the no-code-changes approach when we chain promises.

In either case, I'm not sure I like the idea of m.request returning a modified promise. m.request is generally supposed to be an implementation detail that is abstracted away by the model layer. If a controller calls promise.xhr.abort(), then we get poor separation of concerns - we then need to do ugly feature sniffing everywhere if we want to, say, change that model API to return an m.deferred promise instead of an m.request one (because it pulls data from a client-side cache or whatever). Exposing the xhr object via the config/m.prop combo forces the developer to expose abort as a separate API point, which is better, imho.

[sdemjanenko]

What if m.request returned the xhr object. This would force people into a pattern like:

var cars = m.props([]);
var xhr = m.request({GET:"/cars"}).then(cars);

It would be a breaking change and it would make the simple case of loading a prop a bit more verbose. I think this syntax would be a bit more expected though. Im used to request functions returning the xhr object (e.g. jQuery.ajax)

[Raynos]

@lhorie promises shouldnt propagate the xhr object a.xhr === null; b.xhr === null; root.xhr === { ... }

the xhr is just a value that you can read if you really must. It has nothing to do with promises.

[lhorie]

@sdemjanenko I think that would be too much code to add and it suffers from the same issues I mentioned above. Promises are stateful and return new promises when chained, so I'd basically need to clone the xhr object somehow every time.

Also, I don't want to lose the assignment syntax for data binding, I think it's really convenient and more likely to be used than raw xhr operations (given that the assignment syntax is covered prominently in the guide).

var users = m.request(args)

@Raynos ok, that's what I thought.

[Raynos]

the important part is exposing low level details in the least optionated way possible.

It doesnt have to be easy or convenient, just possible. If people want to write a ChainedAndXhrPromise so they can do

ChainedAndXhrPromise(m.request(...))
  .then(foo)
  .then(bar)
  .xhr.agawjkgahwjkg

then they can go do that outside of mithril.

[lhorie]

@Raynos it is possible, fwiw:

function requestWithXhr(options) {
  var xhr
  options.config = function(x) {xhr = x};
  var req = m.request(options);
  req.xhr = xhr;
  return req;
}

ChainedAndXhrPromise(requestWithXhr(args))
  .then(foo)
  .then(bar)
  .xhr.agawjkgahwjkg