From e158f2532fd59136abf0f21119d669c4d53f5e93 Mon Sep 17 00:00:00 2001 From: Philipp von Weitershausen Date: Sun, 17 Apr 2016 13:33:39 -0700 Subject: [PATCH] Make XMLHttpRequest and XMLHttpRequest.upload proper EventTargets --- Libraries/Network/XMLHttpRequest.android.js | 10 +- Libraries/Network/XMLHttpRequest.ios.js | 17 +- Libraries/Network/XMLHttpRequestBase.js | 171 ++++++++++-------- .../__tests__/XMLHttpRequestBase-test.js | 81 ++++++++- 4 files changed, 186 insertions(+), 93 deletions(-) diff --git a/Libraries/Network/XMLHttpRequest.android.js b/Libraries/Network/XMLHttpRequest.android.js index 165b6d316e29..099df7526f05 100644 --- a/Libraries/Network/XMLHttpRequest.android.js +++ b/Libraries/Network/XMLHttpRequest.android.js @@ -26,7 +26,14 @@ function convertHeadersMapToArray(headers: Object): Array
{ } class XMLHttpRequest extends XMLHttpRequestBase { - sendImpl(method: ?string, url: ?string, headers: Object, data: any, timeout: number): void { + sendImpl( + method: ?string, + url: ?string, + headers: Object, + data: any, + useIncrementalUpdates: boolean, + timeout: number, + ): void { var body; if (typeof data === 'string') { body = {string: data}; @@ -40,7 +47,6 @@ class XMLHttpRequest extends XMLHttpRequestBase { } else { body = data; } - var useIncrementalUpdates = this.onreadystatechange ? true : false; var requestId = RCTNetworking.sendRequest( method, url, diff --git a/Libraries/Network/XMLHttpRequest.ios.js b/Libraries/Network/XMLHttpRequest.ios.js index d79e38019f48..dd4ee0dd63d3 100644 --- a/Libraries/Network/XMLHttpRequest.ios.js +++ b/Libraries/Network/XMLHttpRequest.ios.js @@ -17,13 +17,14 @@ var RCTNetworking = require('RCTNetworking'); var XMLHttpRequestBase = require('XMLHttpRequestBase'); class XMLHttpRequest extends XMLHttpRequestBase { - constructor() { - super(); - // iOS supports upload - this.upload = {}; - } - - sendImpl(method: ?string, url: ?string, headers: Object, data: any, timeout: number): void { + sendImpl( + method: ?string, + url: ?string, + headers: Object, + data: any, + incrementalUpdates: boolean, + timeout: number, + ): void { if (typeof data === 'string') { data = {string: data}; } else if (data instanceof FormData) { @@ -35,7 +36,7 @@ class XMLHttpRequest extends XMLHttpRequestBase { url, data, headers, - incrementalUpdates: this.onreadystatechange ? true : false, + incrementalUpdates, timeout }, this.didCreateRequest.bind(this) diff --git a/Libraries/Network/XMLHttpRequestBase.js b/Libraries/Network/XMLHttpRequestBase.js index 1c9633da2d96..5fbf9182d259 100644 --- a/Libraries/Network/XMLHttpRequestBase.js +++ b/Libraries/Network/XMLHttpRequestBase.js @@ -13,6 +13,8 @@ var RCTNetworking = require('RCTNetworking'); var RCTDeviceEventEmitter = require('RCTDeviceEventEmitter'); + +const EventTarget = require('event-target-shim'); const invariant = require('fbjs/lib/invariant'); const utf8 = require('utf8'); const warning = require('fbjs/lib/warning'); @@ -35,74 +37,81 @@ const SUPPORTED_RESPONSE_TYPES = { '': true, }; +const REQUEST_EVENTS = [ + 'abort', + 'error', + 'load', + 'loadstart', + 'progress', + 'timeout', + 'loadend', +]; + +const XHR_EVENTS = REQUEST_EVENTS.concat('readystatechange'); + +class XMLHttpRequestEventTarget extends EventTarget(...REQUEST_EVENTS) { + onload: ?Function; + onloadstart: ?Function; + onprogress: ?Function; + ontimeout: ?Function; + onerror: ?Function; + onloadend: ?Function; +} + /** * Shared base for platform-specific XMLHttpRequest implementations. */ -class XMLHttpRequestBase { +class XMLHttpRequestBase extends EventTarget(...XHR_EVENTS) { - static UNSENT: number; - static OPENED: number; - static HEADERS_RECEIVED: number; - static LOADING: number; - static DONE: number; + static UNSENT: number = UNSENT; + static OPENED: number = OPENED; + static HEADERS_RECEIVED: number = HEADERS_RECEIVED; + static LOADING: number = LOADING; + static DONE: number = DONE; - UNSENT: number; - OPENED: number; - HEADERS_RECEIVED: number; - LOADING: number; - DONE: number; + UNSENT: number = UNSENT; + OPENED: number = OPENED; + HEADERS_RECEIVED: number = HEADERS_RECEIVED; + LOADING: number = LOADING; + DONE: number = DONE; - onreadystatechange: ?Function; + // EventTarget automatically initializes these to `null`. onload: ?Function; - upload: any; - readyState: number; - responseHeaders: ?Object; - responseText: string; - status: number; - timeout: number; - responseURL: ?string; + onloadstart: ?Function; + onprogress: ?Function; ontimeout: ?Function; onerror: ?Function; + onloadend: ?Function; + onreadystatechange: ?Function; - upload: ?{ - onprogress?: (event: Object) => void; - }; + readyState: number = UNSENT; + responseHeaders: ?Object; + responseText: string = ''; + status: number = 0; + timeout: number = 0; + responseURL: ?string; + + upload: XMLHttpRequestEventTarget = new XMLHttpRequestEventTarget(); _requestId: ?number; _subscriptions: [any]; - _aborted: boolean; + _aborted: boolean = false; _cachedResponse: Response; - _hasError: boolean; + _hasError: boolean = false; _headers: Object; _lowerCaseResponseHeaders: Object; - _method: ?string; + _method: ?string = null; _response: string | ?Object; _responseType: ResponseType; _sent: boolean; - _url: ?string; - _timedOut: boolean; + _url: ?string = null; + _timedOut: boolean = false; + _incrementalEvents: boolean = false; constructor() { - this.UNSENT = UNSENT; - this.OPENED = OPENED; - this.HEADERS_RECEIVED = HEADERS_RECEIVED; - this.LOADING = LOADING; - this.DONE = DONE; - - this.onreadystatechange = null; - this.onload = null; - this.upload = undefined; /* Upload not supported yet */ - this.timeout = 0; - this.ontimeout = null; - this.onerror = null; - + super(); this._reset(); - this._method = null; - this._url = null; - this._aborted = false; - this._timedOut = false; - this._hasError = false; } _reset(): void { @@ -205,30 +214,30 @@ class XMLHttpRequestBase { this._requestId = requestId; this._subscriptions.push(RCTDeviceEventEmitter.addListener( 'didSendNetworkData', - (args) => this._didUploadProgress.call(this, ...args) + (args) => this._didUploadProgress(...args) )); this._subscriptions.push(RCTDeviceEventEmitter.addListener( 'didReceiveNetworkResponse', - (args) => this._didReceiveResponse.call(this, ...args) + (args) => this._didReceiveResponse(...args) )); this._subscriptions.push(RCTDeviceEventEmitter.addListener( 'didReceiveNetworkData', - (args) => this._didReceiveData.call(this, ...args) + (args) => this._didReceiveData(...args) )); this._subscriptions.push(RCTDeviceEventEmitter.addListener( 'didCompleteNetworkResponse', - (args) => this._didCompleteResponse.call(this, ...args) + (args) => this._didCompleteResponse(...args) )); } _didUploadProgress(requestId: number, progress: number, total: number): void { - if (requestId === this._requestId && this.upload && this.upload.onprogress) { - var event = { + if (requestId === this._requestId) { + this.upload.dispatchEvent({ + type: 'progress', lengthComputable: true, loaded: progress, total, - }; - this.upload.onprogress(event); + }); } } @@ -321,7 +330,14 @@ class XMLHttpRequestBase { this.setReadyState(this.OPENED); } - sendImpl(method: ?string, url: ?string, headers: Object, data: any, timeout: number): void { + sendImpl( + method: ?string, + url: ?string, + headers: Object, + data: any, + incrementalEvents: boolean, + timeout: number + ): void { throw new Error('Subclass must define sendImpl method'); } @@ -333,7 +349,15 @@ class XMLHttpRequestBase { throw new Error('Request has already been sent'); } this._sent = true; - this.sendImpl(this._method, this._url, this._headers, data, this.timeout); + const incrementalEvents = this._incrementalEvents || !!this.onreadystatechange; + this.sendImpl( + this._method, + this._url, + this._headers, + data, + incrementalEvents, + this.timeout + ); } abort(): void { @@ -365,42 +389,33 @@ class XMLHttpRequestBase { setReadyState(newState: number): void { this.readyState = newState; - // TODO: workaround flow bug with nullable function checks - var onreadystatechange = this.onreadystatechange; - if (onreadystatechange) { - // We should send an event to handler, but since we don't process that - // event anywhere, let's leave it empty - onreadystatechange.call(this, null); - } + this.dispatchEvent({type: 'readystatechange'}); if (newState === this.DONE && !this._aborted) { if (this._hasError) { if (this._timedOut) { - this._sendEvent(this.ontimeout); + this.dispatchEvent({type: 'timeout'}); } else { - this._sendEvent(this.onerror); + this.dispatchEvent({type: 'error'}); } - } - else { - this._sendEvent(this.onload); + } else { + this.dispatchEvent({type: 'load'}); } } } - _sendEvent(newEvent: ?Function): void { - // TODO: workaround flow bug with nullable function checks - if (newEvent) { - // We should send an event to handler, but since we don't process that - // event anywhere, let's leave it empty - newEvent(null); + /* global EventListener */ + addEventListener(type: string, listener: EventListener): void { + // If we dont' have a 'readystatechange' event handler, we don't + // have to send repeated LOADING events with incremental updates + // to responseText, which will avoid a bunch of native -> JS + // bridge traffic. + if (type === 'readystatechange') { + this._incrementalEvents = true; } + super.addEventListener(type, listener); } } -XMLHttpRequestBase.UNSENT = UNSENT; -XMLHttpRequestBase.OPENED = OPENED; -XMLHttpRequestBase.HEADERS_RECEIVED = HEADERS_RECEIVED; -XMLHttpRequestBase.LOADING = LOADING; -XMLHttpRequestBase.DONE = DONE; function toArrayBuffer(text: string, contentType: string): ArrayBuffer { const {length} = text; diff --git a/Libraries/Network/__tests__/XMLHttpRequestBase-test.js b/Libraries/Network/__tests__/XMLHttpRequestBase-test.js index 9e30a996989e..7218cd9bb912 100644 --- a/Libraries/Network/__tests__/XMLHttpRequestBase-test.js +++ b/Libraries/Network/__tests__/XMLHttpRequestBase-test.js @@ -2,47 +2,118 @@ jest .disableAutomock() - .unmock('XMLHttpRequestBase'); + .dontMock('event-target-shim') + .dontMock('XMLHttpRequestBase'); const XMLHttpRequestBase = require('XMLHttpRequestBase'); +class XMLHttpRequest extends XMLHttpRequestBase {} + describe('XMLHttpRequestBase', function(){ var xhr; + var handleTimeout; + var handleError; + var handleLoad; + var handleReadyStateChange; beforeEach(() => { - xhr = new XMLHttpRequestBase(); + xhr = new XMLHttpRequest(); + xhr.ontimeout = jest.fn(); xhr.onerror = jest.fn(); xhr.onload = jest.fn(); + xhr.onreadystatechange = jest.fn(); + + handleTimeout = jest.fn(); + handleError = jest.fn(); + handleLoad = jest.fn(); + handleReadyStateChange = jest.fn(); + + xhr.addEventListener('timeout', handleTimeout); + xhr.addEventListener('error', handleError); + xhr.addEventListener('load', handleLoad); + xhr.addEventListener('readystatechange', handleReadyStateChange); + xhr.didCreateRequest(1); }); afterEach(() => { xhr = null; + handleTimeout = null; + handleError = null; + handleLoad = null; }); + it('should transition readyState correctly', function() { + expect(xhr.readyState).toBe(xhr.UNSENT); + + xhr.open('GET', 'blabla'); + + expect(xhr.onreadystatechange.mock.calls.length).toBe(1); + expect(handleReadyStateChange.mock.calls.length).toBe(1); + expect(xhr.readyState).toBe(xhr.OPENED); + }); + it('should call ontimeout function when the request times out', function(){ xhr._didCompleteResponse(1, 'Timeout', true); - expect(xhr.ontimeout).toBeCalledWith(null); + expect(xhr.readyState).toBe(xhr.DONE); + + expect(xhr.ontimeout.mock.calls.length).toBe(1); expect(xhr.onerror).not.toBeCalled(); expect(xhr.onload).not.toBeCalled(); + + expect(handleTimeout.mock.calls.length).toBe(1); + expect(handleError).not.toBeCalled(); + expect(handleLoad).not.toBeCalled(); }); it('should call onerror function when the request times out', function(){ xhr._didCompleteResponse(1, 'Generic error'); - expect(xhr.onerror).toBeCalledWith(null); + expect(xhr.readyState).toBe(xhr.DONE); + + expect(xhr.onreadystatechange.mock.calls.length).toBe(1); + expect(xhr.onerror.mock.calls.length).toBe(1); expect(xhr.ontimeout).not.toBeCalled(); expect(xhr.onload).not.toBeCalled(); + + expect(handleReadyStateChange.mock.calls.length).toBe(1); + expect(handleError.mock.calls.length).toBe(1); + expect(handleTimeout).not.toBeCalled(); + expect(handleLoad).not.toBeCalled(); }); it('should call onload function when there is no error', function(){ xhr._didCompleteResponse(1, null); - expect(xhr.onload).toBeCalledWith(null); + expect(xhr.readyState).toBe(xhr.DONE); + + expect(xhr.onreadystatechange.mock.calls.length).toBe(1); + expect(xhr.onload.mock.calls.length).toBe(1); expect(xhr.onerror).not.toBeCalled(); expect(xhr.ontimeout).not.toBeCalled(); + + expect(handleReadyStateChange.mock.calls.length).toBe(1); + expect(handleLoad.mock.calls.length).toBe(1); + expect(handleError).not.toBeCalled(); + expect(handleTimeout).not.toBeCalled(); + }); + + it('should call onload function when there is no error', function() { + xhr.upload.onprogress = jest.fn(); + var handleProgress = jest.fn(); + xhr.upload.addEventListener('progress', handleProgress); + + xhr._didUploadProgress(1, 42, 100); + + expect(xhr.upload.onprogress.mock.calls.length).toBe(1); + expect(handleProgress.mock.calls.length).toBe(1); + + expect(xhr.upload.onprogress.mock.calls[0][0].loaded).toBe(42); + expect(xhr.upload.onprogress.mock.calls[0][0].total).toBe(100); + expect(handleProgress.mock.calls[0][0].loaded).toBe(42); + expect(handleProgress.mock.calls[0][0].total).toBe(100); }); });