Expose option to listen for onprogress event

[deprecate]

Expose option to listen for onprogress event

[brunobasto]

I guess we could have some shorthand function so that we wouldn't have to pass in all those params at once. Something like:

Ajax.config({
	url,
	onprogress: (event) => {
		console.log(`Loaded ${event.loaded}% so far.`);
	}
}).request();

Could be implemented like:

static config(options) {
	return {
		request: () => Ajax.request(options.url, options.method, options.body, ...)
		post: () => Ajax.request(options.url, 'POST', options.body, ...)
	}
}

[jbalsas]

I was also looking at Rob's idea, and while I don't dislike it, I don't think we want to do that. This opens the door for metal-ajax not working at all.

For example:

Ajax.config({
	url,
	beforeRequest: (xhr) => {
		xhr.onload = () => {};
	}
}).request();

This is likely to override our own onload method and make the promise never resolve. Similar overrides can happen for the different params, headers and configuration we pass. I guess it would be better to shield the xhr object and expose the functionality we need in a different way...

I think I lean towards implementing a ProgressPromise inside metal-promise on top of CancellablePromise. Then, this could be used like:

Ajax.request(...)
    .progress(value => console.log(value))
    .then(...)

[jbalsas]

Hey @brunobasto, what do you want exposed? Do you have an idea of how the usage and API would look like? Can you add some more context about the use case?

Since Ajax.request is promise-based, it's not 100% straightforward...

One option would be to attach the xhr instance to the returned promise so you could do whatever you want with it. However, this would open the door for endless side-effects, so I'm not so sure that's such a good idea.

If we wanted to be more strict, we'd need something like progress-promise, similar to jQuery deferred.progress. I haven't seen this spec'd or proposed anywhere though, so it would be quite custom at this point...

@Robert-Frampton, @eduardolundgren, @ipeychev... thoughts?

[brunobasto]

Hey @jbalsas

I just opened this one for discussion purposes. I haven't thought about how to approach this yet. This could be useful for people using the API in general, but the use case I had in mind was to have the senna loading bar show the real progress of the request instead of a continuous animation. Similar to what GitHub is doing I think.

The approach that would require the least amount of change would be to just add another parameter to Ajax.request called opt_onprogressFn. Although I think we already have too many parameters in there.

[robframpton]

I like the idea of creating a new method for passing in params as a config object, there are too many params on the existing request method.

I think the onprogress param would be fine, but what if we created a more general solution so that developers could access/modify the xhr instance without attaching it to the promise? That way we don't have to continue adding params for features such as this.

Ajax.config({
	url,
	beforeRequest: (xhr) => {
		xhr.onprogress = myFn;
	}
}).request();

[jbalsas]

Hey @brunobasto, as @Robert-Frampton points out, I don't think the config solution fixes the onprogress issue, so I would create a separate issue for that.

In that line, the impression I got from past talks was that metal-ajax was meant to be a low-level transport module that we could use to build higher abstractions. That's what WeDeploy does for its client API. I feel that this higher abstractions could be achieved somewhere else, maybe by exposing WeDeploy's client or creating a similar API client for Liferay Portal and other projects.

@eduardolundgren?

[jbalsas]

Another reference: https://github.com/sindresorhus/p-progress

[jbalsas]

Let's try to squeeze this in for 3.0.0, @Robert-Frampton! 🎉

[robframpton]

Hey @jbalsas,

Since it's not too complicated, I thought it would be best to add progress functionality to metal-promise in the form of a ProgressPromise class. Once we have that published it should be pretty trivial to add progress support for metal-ajax.

deprecate/metal-promise#10

[ipeychev]

@jbalsas
In this case, could you release v3.0 with what we have so far to unblock WeDeploy API?

[jbalsas]

Hey @Robert-Frampton, I've just released [email protected], so this should be ready to be implemented here 🎉

I've agreed with @ipeychev to wait until tomorrow to see if we can release [email protected] with both, React16 and ProgressPromise to avoid unnecessary releases, so if you can send that during the day, we'll merge and release tomorrow 💃

[jbalsas]

Merged to develop and scheduled for 3.1.0!!