From 873e52d0d67b0f8470e6290c6fbc35c571464aaf Mon Sep 17 00:00:00 2001 From: Ben Lesh Date: Tue, 9 Feb 2021 00:17:16 -0600 Subject: [PATCH] feat(ajax): Add option for streaming progress (#6001) * feat(ajax): New progress event streaming options - Adds configuration for `includeDownloadProgress` and `includeUploadProgress` that will add streaming download and upload events (respectively) to the resulting observable. - Removes two tests that are no longer relevant. - Adds some additional documentation. - Updates ajax tests to support EventTarget APIs on the mock XHR impl. - Attempts to reduce the size of the ajax implementation. * docs(ajax): Deprecate `progressSubscriber` --- api_guard/dist/types/ajax/index.d.ts | 13 +- spec/observables/dom/ajax-spec.ts | 442 ++++++++++++++++++++++----- src/internal/ajax/AjaxResponse.ts | 53 +++- src/internal/ajax/ajax.ts | 57 +++- src/internal/ajax/types.ts | 25 +- 5 files changed, 484 insertions(+), 106 deletions(-) diff --git a/api_guard/dist/types/ajax/index.d.ts b/api_guard/dist/types/ajax/index.d.ts index 6bb7e1a02b..e99fdb0a6a 100644 --- a/api_guard/dist/types/ajax/index.d.ts +++ b/api_guard/dist/types/ajax/index.d.ts @@ -6,6 +6,8 @@ export interface AjaxConfig { createXHR?: () => XMLHttpRequest; crossDomain?: boolean; headers?: Readonly>; + includeDownloadProgress?: boolean; + includeUploadProgress?: boolean; method?: string; password?: string; progressSubscriber?: PartialObserver; @@ -43,13 +45,20 @@ export interface AjaxRequest { } export declare class AjaxResponse { - readonly originalEvent: Event; + readonly loaded: number; + readonly originalEvent: ProgressEvent; readonly request: AjaxRequest; readonly response: T; readonly responseType: XMLHttpRequestResponseType; readonly status: number; + readonly total: number; + readonly type: string; readonly xhr: XMLHttpRequest; - constructor(originalEvent: Event, xhr: XMLHttpRequest, request: AjaxRequest); + constructor( + originalEvent: ProgressEvent, + xhr: XMLHttpRequest, + request: AjaxRequest, + type?: string); } export interface AjaxTimeoutError extends AjaxError { diff --git a/spec/observables/dom/ajax-spec.ts b/spec/observables/dom/ajax-spec.ts index 1effd753fd..e4458e2e95 100644 --- a/spec/observables/dom/ajax-spec.ts +++ b/spec/observables/dom/ajax-spec.ts @@ -863,7 +863,7 @@ describe('ajax', () => { { uploadProgressTimes: 3 } ); - expect(spy).to.be.called.callCount(3); + expect(spy).to.be.called.callCount(4); }); it('should emit progress event when progressSubscriber is specified', function () { @@ -894,7 +894,7 @@ describe('ajax', () => { { uploadProgressTimes: 3 } ); - expect(spy).to.be.called.callCount(3); + expect(spy).to.be.called.callCount(4); }); }); @@ -948,85 +948,6 @@ describe('ajax', () => { delete root.XMLHttpRequest.prototype.ontimeout; }); - it('should work fine when XMLHttpRequest onprogress property is monkey patched', function () { - Object.defineProperty(root.XMLHttpRequest.prototype, 'onprogress', { - set: function (fn: (e: ProgressEvent) => any) { - const wrapFn = (ev: ProgressEvent) => { - const result = fn.call(this, ev); - if (result === false) { - ev.preventDefault(); - } - }; - this['_onprogress'] = wrapFn; - }, - get() { - return this['_onprogress']; - }, - configurable: true, - }); - - ajax({ - url: '/flibbertyJibbet', - progressSubscriber: { - next: () => { - // noop - }, - error: () => { - // noop - }, - complete: () => { - // noop - }, - }, - }).subscribe(); - - const request = MockXMLHttpRequest.mostRecent; - - expect(() => { - (request.upload as any).onprogress('onprogress'); - }).not.throw(); - - delete root.XMLHttpRequest.prototype.onprogress; - delete root.XMLHttpRequest.prototype.upload; - }); - - it('should work fine when XMLHttpRequest onerror property is monkey patched', function () { - Object.defineProperty(root.XMLHttpRequest.prototype, 'onerror', { - set(fn: (e: ProgressEvent) => any) { - const wrapFn = (ev: ProgressEvent) => { - const result = fn.call(this, ev); - if (result === false) { - ev.preventDefault(); - } - }; - this['_onerror'] = wrapFn; - }, - get() { - return this['_onerror']; - }, - configurable: true, - }); - - ajax({ - url: '/flibbertyJibbet', - }).subscribe({ - error() { - /* expected */ - }, - }); - - const request = MockXMLHttpRequest.mostRecent; - - try { - request.onerror('onerror'); - } catch (e) { - expect(e.message).to.equal('ajax error'); - } - - delete root.XMLHttpRequest.prototype.onerror; - delete root.XMLHttpRequest.prototype.upload; - }); - describe('ajax.patch', () => { it('should create an AjaxObservable with correct options', () => { const expected = { foo: 'bar', hi: 'there you' }; @@ -1111,9 +1032,358 @@ describe('ajax', () => { }); }); }); + + describe('with includeDownloadProgress', () => { + it('should emit download progress', () => { + const results: any[] = []; + + ajax({ + method: 'GET', + url: '/flibbertyJibbett', + includeDownloadProgress: true, + }).subscribe({ + next: (value) => results.push(value), + complete: () => results.push('done'), + }); + + const mockXHR = MockXMLHttpRequest.mostRecent; + mockXHR.respondWith( + { + status: 200, + total: 5, + loaded: 5, + responseType: 'json', + responseText: JSON.stringify({ boo: 'I am a ghost' }), + }, + { uploadProgressTimes: 5, downloadProgressTimes: 5 } + ); + + const request = { + async: true, + body: undefined, + crossDomain: true, + headers: { + 'x-requested-with': 'XMLHttpRequest', + }, + includeDownloadProgress: true, + method: 'GET', + responseType: '', + timeout: 0, + url: '/flibbertyJibbett', + withCredentials: false, + }; + + expect(results).to.deep.equal([ + { + type: 'download_loadstart', + responseType: '', + response: undefined, + loaded: 0, + total: 5, + request, + status: 0, + xhr: mockXHR, + originalEvent: { type: 'loadstart', loaded: 0, total: 5 }, + }, + { + type: 'download_progress', + responseType: '', + response: undefined, + loaded: 1, + total: 5, + request, + status: 0, + xhr: mockXHR, + originalEvent: { type: 'progress', loaded: 1, total: 5 }, + }, + { + type: 'download_progress', + responseType: '', + response: undefined, + loaded: 2, + total: 5, + request, + status: 0, + xhr: mockXHR, + originalEvent: { type: 'progress', loaded: 2, total: 5 }, + }, + { + type: 'download_progress', + responseType: '', + response: undefined, + loaded: 3, + total: 5, + request, + status: 0, + xhr: mockXHR, + originalEvent: { type: 'progress', loaded: 3, total: 5 }, + }, + { + type: 'download_progress', + responseType: '', + response: undefined, + loaded: 4, + total: 5, + request, + status: 0, + xhr: mockXHR, + originalEvent: { type: 'progress', loaded: 4, total: 5 }, + }, + { + type: 'download_progress', + responseType: '', + response: undefined, + loaded: 5, + total: 5, + request, + status: 0, + xhr: mockXHR, + originalEvent: { type: 'progress', loaded: 5, total: 5 }, + }, + { + type: 'download_load', + loaded: 5, + total: 5, + request, + originalEvent: { type: 'load', loaded: 5, total: 5 }, + xhr: mockXHR, + response: { boo: 'I am a ghost' }, + responseType: 'json', + status: 200, + }, + 'done', // from completion. + ]); + }); + + it('should emit upload and download progress', () => { + const results: any[] = []; + + ajax({ + method: 'GET', + url: '/flibbertyJibbett', + includeUploadProgress: true, + includeDownloadProgress: true, + }).subscribe({ + next: (value) => results.push(value), + complete: () => results.push('done'), + }); + + const mockXHR = MockXMLHttpRequest.mostRecent; + mockXHR.respondWith( + { + status: 200, + total: 5, + loaded: 5, + responseType: 'json', + responseText: JSON.stringify({ boo: 'I am a ghost' }), + }, + { uploadProgressTimes: 5, downloadProgressTimes: 5 } + ); + + const request = { + async: true, + body: undefined, + crossDomain: true, + headers: { + 'x-requested-with': 'XMLHttpRequest', + }, + includeUploadProgress: true, + includeDownloadProgress: true, + method: 'GET', + responseType: '', + timeout: 0, + url: '/flibbertyJibbett', + withCredentials: false, + }; + + expect(results).to.deep.equal([ + { + type: 'upload_loadstart', + loaded: 0, + total: 5, + request, + status: 0, + response: undefined, + responseType: '', + xhr: mockXHR, + originalEvent: { type: 'loadstart', loaded: 0, total: 5 }, + }, + { + type: 'upload_progress', + loaded: 1, + total: 5, + request, + status: 0, + response: undefined, + responseType: '', + xhr: mockXHR, + originalEvent: { type: 'progress', loaded: 1, total: 5 }, + }, + { + type: 'upload_progress', + loaded: 2, + total: 5, + request, + status: 0, + response: undefined, + responseType: '', + xhr: mockXHR, + originalEvent: { type: 'progress', loaded: 2, total: 5 }, + }, + { + type: 'upload_progress', + loaded: 3, + total: 5, + request, + status: 0, + response: undefined, + responseType: '', + xhr: mockXHR, + originalEvent: { type: 'progress', loaded: 3, total: 5 }, + }, + { + type: 'upload_progress', + loaded: 4, + total: 5, + request, + status: 0, + response: undefined, + responseType: '', + xhr: mockXHR, + originalEvent: { type: 'progress', loaded: 4, total: 5 }, + }, + { + type: 'upload_progress', + loaded: 5, + total: 5, + request, + status: 0, + response: undefined, + responseType: '', + xhr: mockXHR, + originalEvent: { type: 'progress', loaded: 5, total: 5 }, + }, + { + type: 'upload_load', + loaded: 5, + total: 5, + request, + status: 0, + response: undefined, + responseType: '', + xhr: mockXHR, + originalEvent: { type: 'load', loaded: 5, total: 5 }, + }, + { + type: 'download_loadstart', + responseType: '', + response: undefined, + loaded: 0, + total: 5, + request, + status: 0, + xhr: mockXHR, + originalEvent: { type: 'loadstart', loaded: 0, total: 5 }, + }, + { + type: 'download_progress', + responseType: '', + response: undefined, + loaded: 1, + total: 5, + request, + status: 0, + xhr: mockXHR, + originalEvent: { type: 'progress', loaded: 1, total: 5 }, + }, + { + type: 'download_progress', + responseType: '', + response: undefined, + loaded: 2, + total: 5, + request, + status: 0, + xhr: mockXHR, + originalEvent: { type: 'progress', loaded: 2, total: 5 }, + }, + { + type: 'download_progress', + responseType: '', + response: undefined, + loaded: 3, + total: 5, + request, + status: 0, + xhr: mockXHR, + originalEvent: { type: 'progress', loaded: 3, total: 5 }, + }, + { + type: 'download_progress', + responseType: '', + response: undefined, + loaded: 4, + total: 5, + request, + status: 0, + xhr: mockXHR, + originalEvent: { type: 'progress', loaded: 4, total: 5 }, + }, + { + type: 'download_progress', + responseType: '', + response: undefined, + loaded: 5, + total: 5, + request, + status: 0, + xhr: mockXHR, + originalEvent: { type: 'progress', loaded: 5, total: 5 }, + }, + { + type: 'download_load', + loaded: 5, + total: 5, + request, + originalEvent: { type: 'load', loaded: 5, total: 5 }, + xhr: mockXHR, + response: { boo: 'I am a ghost' }, + responseType: 'json', + status: 200, + }, + 'done', // from completion. + ]); + }); + }); }); -class MockXMLHttpRequest { +// Some of the older versions of node we test on don't have EventTarget. +class MockXHREventTarget { + private registry = new Map void>>(); + + addEventListener(type: string, handler: (e: ProgressEvent) => void) { + let handlers = this.registry.get(type); + if (!handlers) { + this.registry.set(type, (handlers = new Set())); + } + handlers.add(handler); + } + + removeEventListener(type: string, handler: (e: ProgressEvent) => void) { + this.registry.get(type)?.delete(handler); + } + + dispatchEvent(event: ProgressEvent) { + const { type } = event; + const handlers = this.registry.get(type); + if (handlers) { + for (const handler of handlers) { + handler(event); + } + } + } +} +class MockXMLHttpRequest extends MockXHREventTarget { static readonly DONE = 4; /** @@ -1165,9 +1435,11 @@ class MockXMLHttpRequest { onprogress: (e: ProgressEvent) => any; // @ts-ignore: Property has no initializer and is not definitely assigned ontimeout: (e: ProgressEvent) => any; - upload: XMLHttpRequestUpload = {}; + + upload: XMLHttpRequestUpload = new MockXHREventTarget() as any; constructor() { + super(); MockXMLHttpRequest.recentRequest = this; MockXMLHttpRequest.requests.push(this); if (MockXMLHttpRequest.noResponseProp) { @@ -1284,6 +1556,8 @@ class MockXMLHttpRequest { // TODO: create a better default event const e: any = eventObj || { type: name }; + this.dispatchEvent({ type: name, ...eventObj }); + if (this['on' + name]) { this['on' + name](e); } @@ -1293,6 +1567,8 @@ class MockXMLHttpRequest { // TODO: create a better default event const e: any = eventObj || {}; + this.upload.dispatchEvent({ type: name, ...eventObj }); + if (this.upload['on' + name]) { this.upload['on' + name](e); } diff --git a/src/internal/ajax/AjaxResponse.ts b/src/internal/ajax/AjaxResponse.ts index 14683046c1..0c7c171364 100644 --- a/src/internal/ajax/AjaxResponse.ts +++ b/src/internal/ajax/AjaxResponse.ts @@ -8,18 +8,38 @@ import { getXHRResponse } from './getXHRResponse'; * - DO NOT create instances of this class directly. * - DO NOT subclass this class. * + * It is advised not to hold this object in memory, as it has a reference to + * the original XHR used to make the request, as well as properties containing + * request and response data. + * * @see {@link ajax} */ export class AjaxResponse { /** The HTTP status code */ readonly status: number; - /** The response data */ + /** The response data, if any. Note that this will automatically be converted to the proper type.*/ readonly response: T; /** The responseType from the response. (For example: `""`, "arraybuffer"`, "blob"`, "document"`, "json"`, or `"text"`) */ readonly responseType: XMLHttpRequestResponseType; + /** + * The total number of bytes loaded so far. To be used with {@link total} while + * calcating progress. (You will want to set {@link includeDownloadProgress} or {@link includeDownloadProgress}) + * + * {@see {@link AjaxConfig}} + */ + readonly loaded: number; + + /** + * The total number of bytes to be loaded. To be used with {@link loaded} while + * calcating progress. (You will want to set {@link includeDownloadProgress} or {@link includeDownloadProgress}) + * + * {@see {@link AjaxConfig}} + */ + readonly total: number; + /** * A normalized response from an AJAX request. To get the data from the response, * you will want to read the `response` property. @@ -31,9 +51,34 @@ export class AjaxResponse { * @param xhr The `XMLHttpRequest` object used to make the request. This is useful for examining status code, etc. * @param request The request settings used to make the HTTP request. */ - constructor(public readonly originalEvent: Event, public readonly xhr: XMLHttpRequest, public readonly request: AjaxRequest) { - this.status = xhr.status; - this.responseType = xhr.responseType; + constructor( + /** + * The original event object from the raw XHR event. + */ + public readonly originalEvent: ProgressEvent, + /** + * The XMLHttpRequest object used to make the request. + * NOTE: It is advised not to hold this in memory, as it will retain references to all of it's event handlers + * and many other things related to the request. + */ + public readonly xhr: XMLHttpRequest, + /** + * The request parameters used to make the HTTP request. + */ + public readonly request: AjaxRequest, + /** + * The event type. This can be used to discern between different events + * if you're using progress events with {@link includeDownloadProgress} or + * {@link includeUploadProgress} settings in {@link AjaxConfig}. + */ + public readonly type = 'download_load' + ) { + const { status, responseType } = xhr; + this.status = status ?? 0; + this.responseType = responseType ?? ''; this.response = getXHRResponse(xhr); + const { loaded, total } = originalEvent; + this.loaded = loaded; + this.total = total; } } diff --git a/src/internal/ajax/ajax.ts b/src/internal/ajax/ajax.ts index e078b89f1f..4de5b3ff34 100644 --- a/src/internal/ajax/ajax.ts +++ b/src/internal/ajax/ajax.ts @@ -268,8 +268,13 @@ export const ajax: AjaxCreationMethod = (() => { return create; })(); +const DOWNLOAD = 'download'; +const LOADSTART = 'loadstart'; +const PROGRESS = 'progress'; +const LOAD = 'load'; + export function fromAjax(config: AjaxConfig): Observable> { - return new Observable>((destination) => { + return new Observable((destination) => { // Normalize the headers. We're going to make them all lowercase, since // Headers are case insenstive by design. This makes it easier to verify // that we aren't setting or sending duplicates. @@ -315,7 +320,7 @@ export function fromAjax(config: AjaxConfig): Observable> { withCredentials: false, method: 'GET', timeout: 0, - responseType: 'json' as XMLHttpRequestResponseType, + responseType: '' as XMLHttpRequestResponseType, // Override with passed user values ...config, @@ -343,7 +348,16 @@ export function fromAjax(config: AjaxConfig): Observable> { // Otherwise the progress events will not fire. /////////////////////////////////////////////////// - const progressSubscriber = config.progressSubscriber; + const { progressSubscriber, includeDownloadProgress = false, includeUploadProgress = false } = config; + + const createResponse = (direction: 'upload' | 'download', event: ProgressEvent) => + new AjaxResponse(event, xhr, _request, `${direction}_${event.type}`); + + const addProgressEvent = (target: any, type: string, direction: 'upload' | 'download') => { + target.addEventListener(type, (event: ProgressEvent) => { + destination.next(createResponse(direction, event)); + }); + }; xhr.ontimeout = () => { const timeoutError = new AjaxTimeoutError(xhr, _request); @@ -351,20 +365,32 @@ export function fromAjax(config: AjaxConfig): Observable> { destination.error(timeoutError); }; + if (includeUploadProgress) { + [LOADSTART, PROGRESS, LOAD].forEach((type) => addProgressEvent(xhr.upload, type, 'upload')); + } + if (progressSubscriber) { - xhr.upload.onprogress = (e: ProgressEvent) => { - progressSubscriber.next?.(e); - }; + [LOADSTART, PROGRESS].forEach((type) => xhr.upload.addEventListener(type, (e: any) => progressSubscriber?.next?.(e))); } - xhr.onerror = (e: ProgressEvent) => { - progressSubscriber?.error?.(e); - destination.error(new AjaxError('ajax error', xhr, _request)); + if (includeDownloadProgress) { + [LOADSTART, PROGRESS].forEach((type) => addProgressEvent(xhr, type, DOWNLOAD)); + } + + const emitError = (status?: number) => { + const msg = 'ajax error' + (status ? ' ' + status : ''); + destination.error(new AjaxError(msg, xhr, _request)); }; - xhr.onload = (e: ProgressEvent) => { + xhr.addEventListener('error', (e) => { + progressSubscriber?.error?.(e); + emitError(); + }); + + xhr.addEventListener(LOAD, (event) => { + const { status } = xhr; // 4xx and 5xx should error (https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html) - if (xhr.status < 400) { + if (status < 400) { progressSubscriber?.complete?.(); let response: AjaxResponse; @@ -372,18 +398,19 @@ export function fromAjax(config: AjaxConfig): Observable> { // This can throw in IE, because we end up needing to do a JSON.parse // of the response in some cases to produce object we'd expect from // modern browsers. - response = new AjaxResponse(e, xhr, _request); + response = createResponse(DOWNLOAD, event); } catch (err) { destination.error(err); return; } + destination.next(response); destination.complete(); } else { - progressSubscriber?.error?.(e); - destination.error(new AjaxError('ajax error ' + xhr.status, xhr, _request)); + progressSubscriber?.error?.(event); + emitError(status); } - }; + }); } const { user, method, async } = _request; diff --git a/src/internal/ajax/types.ts b/src/internal/ajax/types.ts index 5becc29b7c..47d653852e 100644 --- a/src/internal/ajax/types.ts +++ b/src/internal/ajax/types.ts @@ -166,12 +166,33 @@ export interface AjaxConfig { createXHR?: () => XMLHttpRequest; /** - * An observer for watching the progress of an HTTP request. Will - * emit progress events, and completes on the final load event, will error for + * An observer for watching the upload progress of an HTTP request. Will + * emit progress events, and completes on the final upload load event, will error for * any XHR error or timeout. * * This will **not** error for errored status codes. Rather, it will always _complete_ when * the HTTP response comes back. + * + * @deprecated If you're looking for progress events, please try {@link includeDownloadProgress} and + * {@link includeUploadProgress}. This will be removed in version 8. */ progressSubscriber?: PartialObserver; + + /** + * If `true`, will emit all download progress and load complete events as {@link AjaxProgressEvents} + * from the observable. The final download event will also be emitted as a {@link AjaxDownloadCompleteEvent} + * + * If both this and {@link includeUploadProgress} are `false`, then only the {@link AjaxResponse} will + * be emitted from the resulting observable. + */ + includeDownloadProgress?: boolean; + + /** + * If `true`, will emit all upload progress and load complete events as {@link AjaxUploadProgressEvents} + * from the observable. The final download event will also be emitted as a {@link AjaxDownloadCompleteEvent} + * + * If both this and {@link includeDownloadProgress} are `false`, then only the {@link AjaxResponse} will + * be emitted from the resulting observable. + */ + includeUploadProgress?: boolean; }