chore: address remaining issues

- address jsdom use case as per
   ipfs/js-ipfs#3238
- remove onDownloadProgress since we don't need it

Author: Gozala

src/http.js

@@ -70,8 +70,8 @@ const defaults = {
  * @prop {function(URLSearchParams): URLSearchParams } [transformSearchParams]
  * @prop {function(any): any} [transform] - When iterating the response body, transform each chunk with this function.
  * @prop {function(Response): Promise<void>} [handleError] - Handle errors
- * @prop {function({total:number, loaded:number, lengthComputable:boolean}):void} [onUploadProgress] - Can be passed to track upload progress
- * @prop {function({total:number, loaded:number, lengthComputable:boolean}):void} [onDownloadProgress] - Can be passed to track download progress
+ * @prop {function({total:number, loaded:number, lengthComputable:boolean}):void} [onUploadProgress] - Can be passed to track upload progress.
+ * Note that if this option in passed undelying request will be performed using `XMLHttpRequest` and response will not be streamed.
  */
 
 class HTTP {

src/http/fetch.browser.js

@@ -2,14 +2,14 @@
 /* eslint-env browser */
 
 const { TimeoutError, AbortError } = require('./error')
+const Response = require('./response')
 
 /**
  * @typedef {RequestInit & ExtraFetchOptions} FetchOptions
  * @typedef {Object} ExtraFetchOptions
  * @property {number} [timeout]
  * @property {URLSearchParams} [searchParams]
  * @property {function({total:number, loaded:number, lengthComputable:boolean}):void} [onUploadProgress]
- * @property {function({total:number, loaded:number, lengthComputable:boolean}):void} [onDownloadProgress]
  * @property {string} [overrideMimeType]
  * @returns {Promise<Response>}
  */
@@ -42,15 +42,16 @@ const fetchWithProgress = (url, options = {}) => {
     options.signal.onabort = () => request.abort()
   }
 
-  if (options.onDownloadProgress) {
-    request.onprogress = options.onDownloadProgress
-  }
-
   if (options.onUploadProgress) {
     request.upload.onprogress = options.onUploadProgress
   }
 
-  request.responseType = 'blob'
+  // Note: Need to use `arraybuffer` here instead of `blob` because `Blob`
+  // instances coming from JSDOM are not compatible with `Response` from
+  // node-fetch (which is the setup we get when testing with jest because
+  // it uses JSDOM which does not provide a global fetch
+  // https://github.com/jsdom/jsdom/issues/1724)
+  request.responseType = 'arraybuffer'
 
   return new Promise((resolve, reject) => {
     /**
@@ -97,7 +98,7 @@ const fetchWithProgress = (url, options = {}) => {
 const fetchWithStreaming = fetch
 
 const fetchWith = (url, options = {}) =>
-  (options.onDownloadProgress != null || options.onUploadProgress != null)
+  (options.onUploadProgress != null)
     ? fetchWithProgress(url, options)
     : fetchWithStreaming(url, options)
 

src/http/fetch.node.js

@@ -29,17 +29,9 @@ const { Request, Response, Headers } = nodeFetch
  * @param {FetchOptions} [options]
  * @returns {Promise<Response>}
  */
-const fetch = async (url, options = {}) => {
-  const { onDownloadProgress } = options
+const fetch = (url, options = {}) =>
+  nodeFetch(url, withUploadProgress(options))
 
-  const response = await nodeFetch(url, withUploadProgress(options))
-
-  if (onDownloadProgress) {
-    return withDownloadProgress(response, onDownloadProgress)
-  } else {
-    return response
-  }
-}
 exports.fetch = fetch
 exports.Request = Request
 exports.Headers = Headers
@@ -94,40 +86,16 @@ const iterateBodyWithProgress = async function * (body, onUploadProgress) {
   } else if (Buffer.isBuffer(body)) {
     const total = body.byteLength
     const lengthComputable = true
-    onUploadProgress({ total, loaded: 0, lengthComputable })
     yield body
     onUploadProgress({ total, loaded: total, lengthComputable })
   } else {
     const total = 0
     const lengthComputable = false
     let loaded = 0
-    onUploadProgress({ total, loaded, lengthComputable })
     for await (const chunk of body) {
       loaded += chunk.byteLength
       yield chunk
       onUploadProgress({ total, loaded, lengthComputable })
     }
   }
 }
-
-/**
- * Takes node-fetch response and tracks download progress for it.
- * @param {Response} response
- * @param {function(LoadProgress):void} onDownloadProgress
- * @returns {Response}
- */
-const withDownloadProgress = (response, onDownloadProgress) => {
-  /** @type {Readable} */
-  // @ts-ignore - Unlike standard Response, in node-fetch response body is
-  // node Readable stream.
-  const { body } = response
-  const length = parseInt(response.headers.get('Content-Length'))
-  const lengthComputable = !isNaN(length)
-  const total = isNaN(length) ? 0 : length
-  let loaded = 0
-  body.on('data', (chunk) => {
-    loaded += chunk.length
-    onDownloadProgress({ lengthComputable, total, loaded })
-  })
-  return response
-}

src/http/response.js

@@ -0,0 +1,11 @@
+'use strict'
+
+// JSDOM has `XMLHttpRequest` but it does not have a `fetch` or `Response` so
+// we workaround by pulling in node-fetch.
+// See: https://github.com/jsdom/jsdom/issues/1724
+if (typeof Response !== 'undefined') {
+  /* eslint-env browser */
+  module.exports = Response
+} else {
+  module.exports = require('node-fetch').Response
+}

test/http.spec.js

@@ -155,27 +155,19 @@ describe('http', function () {
 
   it('progress events', async () => {
     let upload = 0
-    let download = 0
     const body = new Uint8Array(1000000 / 2)
     const request = await HTTP.post(`${process.env.ECHO_SERVER}/echo`, {
       body,
       onUploadProgress: (progress) => {
         expect(progress).to.have.property('lengthComputable').to.be.a('boolean')
         expect(progress).to.have.property('total', body.byteLength)
-        expect(progress).to.have.property('loaded').to.be.a('number')
+        expect(progress).to.have.property('loaded').to.be.greaterThan(0)
         upload += 1
-      },
-      onDownloadProgress: (progress) => {
-        expect(progress).to.have.property('lengthComputable').to.be.a('boolean')
-        expect(progress).to.have.property('total').to.be.a('number')
-        expect(progress).to.have.property('loaded').to.be.a('number')
-        download += 1
       }
     })
     await all(request.iterator())
 
     expect(upload).to.be.greaterThan(0)
-    expect(download).to.be.greaterThan(0)
   })
 
   it('makes a GET request with unprintable characters', async function () {