Notifications for xhr.upload.onprogress

[demurgos]

Hi,
Currently, q-xhr uses deferred.notify to track the progression of the download of the response. It would be nice to be able to track the upload when available.

Angular's $http being backed by $q, they don't have the deferred.notify API so it's not possible to copy their implementation. (it's the goal of your lib if I'm not mistaken ?)
Still, reacting to progress is a popular feature so here is a proposition for the implementation:

• To avoid to break compatibility with applications expecting Q.xhr({...}).progress to only trigger on download progress, applications have to explicitly ask for upload progress notifications with the new config property watchUpload (boolean).

• It's possible to set Q.xhr.defaults.watchUpload = true

• Since there will be two sorts of progress calls, it should be possible to differentiate them. Both type of progress objects are instances of ProgressEvent. To differentiate them, it's possible to use:

  Q
    .xhr({url: '/foo', method: 'GET', watchUpload: true})
    .progress(function(progress) {
      if(window.XMLHttpRequestUpload && progress.target instanceof XMLHttpRequestUpload) {
        // upload progress
      }else{
        // download progress
      }
    })

  window.XMLHttpRequestUpload is a quick feature detection to avoid to throw on progress.target instanceof undefined when executed on older browsers.

  This test on the user side does not seem like a good idea (hard to maintain, boilerplate) so I add the flag upload when the progress event is received by the upload.onprogress callback. This way, the above example becomes simply:

  Q
    .xhr({url: '/foo', method: 'GET', watchUpload: true})
    .progress(function(progress) {
      if(progress.upload) {
        // upload progress
      }else{
        // download progress
      }
    })

What do you think about it ? (Maybe the names of the option and flag should change ?)

[nathanboktae]

Thanks for the pull request! I like it - a couple of things:

so it's not possible to copy their implementation. (it's the goal of your lib if I'm not mistaken ?)

No it's not. $http is a basis and a few points I did not explicitly implement are on in the readme.

I would prefer to remove the flag and take a major version bump. This is a nice feature and I don't see a reason to turn it off, definitely not by default.

Also documentation and a test (integration is good but unit only is fine) please. I see the build is failing on the saucelabs part - I'll check to see if my saucelabs key is still good but I am not worried about that.

[demurgos]

Thank you for your quick answer, I'll complete the pull request tomorrow.

About $http, I wasn't sure if the goal was to stick with their API or not: thanks for clarification.

I'll drop watchUpload, but what about the easy way to differentiate between upload & download progress events ? Should I still augment the original event with progress.upload=true ?

[nathanboktae]

Should I still augment the original event with progress.upload=true ?

Yes, I like that approach.

You can bump to 1.0.0 btw - I was meaning to do that for some time. The project is stable.

[demurgos]

I updated my changes but I'm not so familiar with mocha & coffee script so I screw up the tests. Could you take a look at it ?

[nathanboktae]

Yup I can update the tests. The changes are solid. thanks!