Changing XHR to Fetch API

[hmdros]

@arielj, @toppa and I are submitting this PR for this issue: #1193

We had a few differences between XHR and Fetch, so we couldn't match functionality 100%.
But we think it's fine from what we saw on the code.

• Now we abort the request with AbortController
• Fetch API doesn't have a onProgress callback or equivalent
• Fetch API doesn't support sync request

We couldn't find usages of the xhr argument on callback methods. We weren't sure about the arguments.

[simon04]

Thanks for your work. I appreciate the modernisation. For this core functionality, we should take extra care to maintain compatibility and not introduce any bugs. Please see my comments.

[simon04]

Here are some usages for the error handler and progress callback:

• devdocs/assets/javascripts/app/db.coffee

  Lines 253 to 257 in 676f4f6

  	ajax
  	url: entry.fileUrl()
  	dataType: 'html'
  	success: onSuccess
  	error: onError

• devdocs/assets/javascripts/app/update_checker.coffee

  Lines 14 to 17 in 14049da

  	ajax
  	url: $('script[src*="application"]').getAttribute('src')
  	dataType: 'application/javascript'
  	error: (_, xhr) => @onUpdateReady() if xhr.status is 404

• devdocs/assets/javascripts/models/doc.coffee

  Lines 67 to 70 in 14049da

  	ajax
  	url: @indexUrl()
  	success: callback
  	error: onError

• devdocs/assets/javascripts/models/doc.coffee

  Lines 114 to 119 in 14049da

  	ajax
  	url: @dbUrl()
  	success: success
  	error: error
  	progress: onProgress
  	timeout: 3600

[simon04]

Here's one usage of abort:

• devdocs/assets/javascripts/views/content/entry_page.coffee

  Lines 112 to 114 in 93a42cc

  	if @xhr
  	@xhr.abort()
  	@xhr = @entry = null

[simon04]

I think response.ok and response.status (200 <= xhr.status < 300) need to be checked.

[arielj]

hey @simon04 we were reviewing your comments and the code, and we have some concerns if this move from xhr to fetch is actually beneficial or not. I can describe a bit of what we found:

• fetch has no equivalent of a progress callback, the workarounds involve adding a lot of code wrapping the fetch requests into another object like https://dev.to/tqbit/how-to-monitor-the-progress-of-a-javascript-fetch-request-and-cancel-it-on-demand-107f or https://github.com/prioe/node-fetch-progress, but we are not sure if that's the direction you want to go with this code, it will be way more complex
• the way the abort works with fetch requests requires a lot of changes in logic because fetch does not return a request object that can be aborted, it returns a promise, so the abort callback would have to be passed as an option from the caller (it requires changing entry_page.coffee, entry.coffee, db.coffee, and ajax.coffee just to pass around an abort function)

Also, we noticed one thing that seems to be a bug in the current code, this error callback

devdocs/assets/javascripts/app/update_checker.coffee

Line 17 in 14049da

error: (_, xhr) => @onUpdateReady() if xhr.status is 404

is expecting the xhr as the second argument, but that's actually passed as the third argument here

devdocs/assets/javascripts/lib/ajax.coffee

Line 97 in 14049da

options.error?.call options.context, type, xhr, options

We want to raise these concerns before continuing with this change, we are not sure if it's really worth it (the current code works just fine and some of the code around it was designed for how xhr works)