From 674e88afc2a232aac2b934948f665943fd897534 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Tue, 5 Sep 2023 10:26:07 +0100 Subject: [PATCH] Make HTTP request implementations tree-shakable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We expose XHRRequest and FetchRequest modules. The user is required to provide an HTTP module, even for Realtime, since it’s used for the internet connectivity check and for making a token request to the authUrl. Resolves #1395. --- scripts/moduleReport.js | 2 + src/common/lib/client/baseclient.ts | 5 ++ src/common/lib/client/modulesmap.ts | 4 ++ src/platform/nativescript/index.ts | 3 + src/platform/react-native/index.ts | 3 + src/platform/web-noencryption/index.ts | 3 + src/platform/web/index.ts | 3 + src/platform/web/lib/http/http.ts | 45 +++++++++--- src/platform/web/lib/http/request/index.ts | 10 +++ src/platform/web/modules.ts | 4 ++ src/platform/web/modules/http.ts | 2 + test/browser/modules.test.js | 84 +++++++++++++++------- 12 files changed, 135 insertions(+), 33 deletions(-) create mode 100644 src/platform/web/lib/http/request/index.ts create mode 100644 src/platform/web/modules/http.ts diff --git a/scripts/moduleReport.js b/scripts/moduleReport.js index ee5cadf0a8..f82a58ea8a 100644 --- a/scripts/moduleReport.js +++ b/scripts/moduleReport.js @@ -9,6 +9,8 @@ const moduleNames = [ 'XHRPolling', 'XHRStreaming', 'WebSocketTransport', + 'XHRRequest', + 'FetchRequest', ]; // List of all free-standing functions exported by the library along with the diff --git a/src/common/lib/client/baseclient.ts b/src/common/lib/client/baseclient.ts index fbb2be3475..ddc726b4d5 100644 --- a/src/common/lib/client/baseclient.ts +++ b/src/common/lib/client/baseclient.ts @@ -15,6 +15,7 @@ import { Rest } from './rest'; import { IUntypedCryptoStatic } from 'common/types/ICryptoStatic'; import { throwMissingModuleError } from '../util/utils'; import { MsgPack } from 'common/types/msgpack'; +import { HTTPRequestImplementations } from 'platform/web/lib/http/http'; type BatchResult = API.Types.BatchResult; type BatchPublishSpec = API.Types.BatchPublishSpec; @@ -41,8 +42,12 @@ class BaseClient { private readonly _rest: Rest | null; readonly _Crypto: IUntypedCryptoStatic | null; readonly _MsgPack: MsgPack | null; + // Extra HTTP request implementations available to this client, in addition to those in web’s Http.bundledRequestImplementations + readonly _additionalHTTPRequestImplementations: HTTPRequestImplementations; constructor(options: ClientOptions | string, modules: ModulesMap) { + this._additionalHTTPRequestImplementations = modules; + if (!options) { const msg = 'no options provided'; Logger.logAction(Logger.LOG_ERROR, 'BaseClient()', msg); diff --git a/src/common/lib/client/modulesmap.ts b/src/common/lib/client/modulesmap.ts index ada7de44ee..dff32a69e4 100644 --- a/src/common/lib/client/modulesmap.ts +++ b/src/common/lib/client/modulesmap.ts @@ -3,6 +3,8 @@ import { IUntypedCryptoStatic } from '../../types/ICryptoStatic'; import { MsgPack } from 'common/types/msgpack'; import RealtimePresence from './realtimepresence'; import { TransportInitialiser } from '../transport/connectionmanager'; +import XHRRequest from 'platform/web/lib/http/request/xhrrequest'; +import fetchRequest from 'platform/web/lib/http/request/fetchrequest'; export interface ModulesMap { Rest?: typeof Rest; @@ -12,6 +14,8 @@ export interface ModulesMap { WebSocketTransport?: TransportInitialiser; XHRPolling?: TransportInitialiser; XHRStreaming?: TransportInitialiser; + XHRRequest?: typeof XHRRequest; + FetchRequest?: typeof fetchRequest; } export const allCommonModules: ModulesMap = { Rest }; diff --git a/src/platform/nativescript/index.ts b/src/platform/nativescript/index.ts index c8354aa69d..119fdcb048 100644 --- a/src/platform/nativescript/index.ts +++ b/src/platform/nativescript/index.ts @@ -19,6 +19,7 @@ import { getDefaults } from '../../common/lib/util/defaults'; import WebStorage from './lib/util/webstorage'; import PlatformDefaults from '../web/lib/util/defaults'; import msgpack from '../web/lib/util/msgpack'; +import { defaultBundledRequestImplementations } from '../web/lib/http/request'; const Crypto = createCryptoClass(Config, BufferUtils); @@ -34,6 +35,8 @@ for (const clientClass of [DefaultRest, DefaultRealtime]) { clientClass._MsgPack = msgpack; } +Http.bundledRequestImplementations = defaultBundledRequestImplementations; + Logger.initLogHandlers(); Platform.Defaults = getDefaults(PlatformDefaults); diff --git a/src/platform/react-native/index.ts b/src/platform/react-native/index.ts index dc27bb62c8..e0539aa92a 100644 --- a/src/platform/react-native/index.ts +++ b/src/platform/react-native/index.ts @@ -17,6 +17,7 @@ import { getDefaults } from '../../common/lib/util/defaults'; import WebStorage from '../web/lib/util/webstorage'; import PlatformDefaults from '../web/lib/util/defaults'; import msgpack from '../web/lib/util/msgpack'; +import { defaultBundledRequestImplementations } from '../web/lib/http/request'; const Config = configFactory(BufferUtils); @@ -34,6 +35,8 @@ for (const clientClass of [DefaultRest, DefaultRealtime]) { clientClass._MsgPack = msgpack; } +Http.bundledRequestImplementations = defaultBundledRequestImplementations; + Logger.initLogHandlers(); Platform.Defaults = getDefaults(PlatformDefaults); diff --git a/src/platform/web-noencryption/index.ts b/src/platform/web-noencryption/index.ts index dcb4120123..64dcf02b5e 100644 --- a/src/platform/web-noencryption/index.ts +++ b/src/platform/web-noencryption/index.ts @@ -16,6 +16,7 @@ import { getDefaults } from '../../common/lib/util/defaults'; import WebStorage from '../web/lib/util/webstorage'; import PlatformDefaults from '../web/lib/util/defaults'; import msgpack from '../web/lib/util/msgpack'; +import { defaultBundledRequestImplementations } from '../web/lib/http/request'; Platform.Crypto = null; Platform.BufferUtils = BufferUtils; @@ -28,6 +29,8 @@ for (const clientClass of [DefaultRest, DefaultRealtime]) { clientClass._MsgPack = msgpack; } +Http.bundledRequestImplementations = defaultBundledRequestImplementations; + Logger.initLogHandlers(); Platform.Defaults = getDefaults(PlatformDefaults); diff --git a/src/platform/web/index.ts b/src/platform/web/index.ts index 3e40e123b7..27d6c9556b 100644 --- a/src/platform/web/index.ts +++ b/src/platform/web/index.ts @@ -17,6 +17,7 @@ import { getDefaults } from '../../common/lib/util/defaults'; import WebStorage from './lib/util/webstorage'; import PlatformDefaults from './lib/util/defaults'; import msgpack from './lib/util/msgpack'; +import { defaultBundledRequestImplementations } from './lib/http/request'; const Crypto = createCryptoClass(Config, BufferUtils); @@ -32,6 +33,8 @@ for (const clientClass of [DefaultRest, DefaultRealtime]) { clientClass._MsgPack = msgpack; } +Http.bundledRequestImplementations = defaultBundledRequestImplementations; + Logger.initLogHandlers(); Platform.Defaults = getDefaults(PlatformDefaults); diff --git a/src/platform/web/lib/http/http.ts b/src/platform/web/lib/http/http.ts index b8566c5633..626a946758 100644 --- a/src/platform/web/lib/http/http.ts +++ b/src/platform/web/lib/http/http.ts @@ -2,16 +2,17 @@ import Platform from 'common/platform'; import * as Utils from 'common/lib/util/utils'; import Defaults from 'common/lib/util/defaults'; import ErrorInfo, { PartialErrorInfo } from 'common/lib/types/errorinfo'; -import { IHttpStatic, RequestCallback, RequestParams } from 'common/types/http'; +import { RequestCallback, RequestParams } from 'common/types/http'; import HttpMethods from 'common/constants/HttpMethods'; import BaseClient from 'common/lib/client/baseclient'; import BaseRealtime from 'common/lib/client/baserealtime'; -import XHRRequest from './request/xhrrequest'; import XHRStates from 'common/constants/XHRStates'; import Logger from 'common/lib/util/logger'; import { StandardCallback } from 'common/types/utils'; -import fetchRequest from './request/fetchrequest'; import { isSuccessCode } from 'common/constants/HttpStatusCodes'; +import { ModulesMap } from 'common/lib/client/modulesmap'; + +export type HTTPRequestImplementations = Pick; function shouldFallback(errorInfo: ErrorInfo) { const statusCode = errorInfo.statusCode as number; @@ -39,10 +40,20 @@ function getHosts(client: BaseClient): string[] { return Defaults.getHosts(client.options); } -const Http: IHttpStatic = class { +function createMissingImplementationError() { + return new ErrorInfo( + 'No HTTP request module provided. Provide at least one of the FetchRequest or XHRRequest modules.', + 400, + 40000 + ); +} + +const Http = class { static methods = [HttpMethods.Get, HttpMethods.Delete, HttpMethods.Post, HttpMethods.Put, HttpMethods.Patch]; static methodsWithoutBody = [HttpMethods.Get, HttpMethods.Delete]; static methodsWithBody = [HttpMethods.Post, HttpMethods.Put, HttpMethods.Patch]; + // HTTP request implementations that are available even without a BaseClient object (needed by some tests which directly instantiate `Http` without a client) + static bundledRequestImplementations: HTTPRequestImplementations; checksInProgress: Array> | null = null; private client: BaseClient | null; @@ -51,7 +62,20 @@ const Http: IHttpStatic = class { const connectivityCheckUrl = client?.options.connectivityCheckUrl || Defaults.connectivityCheckUrl; const connectivityCheckParams = client?.options.connectivityCheckParams ?? null; const connectivityUrlIsDefault = !client?.options.connectivityCheckUrl; - if (Platform.Config.xhrSupported) { + + const requestImplementations = { + ...Http.bundledRequestImplementations, + ...client?._additionalHTTPRequestImplementations, + }; + const xhrRequestImplementation = requestImplementations.XHRRequest; + const fetchRequestImplementation = requestImplementations.FetchRequest; + const hasImplementation = !!(xhrRequestImplementation || fetchRequestImplementation); + + if (!hasImplementation) { + throw createMissingImplementationError(); + } + + if (Platform.Config.xhrSupported && xhrRequestImplementation) { this.supportsAuthHeaders = true; this.Request = function ( method: HttpMethods, @@ -61,7 +85,7 @@ const Http: IHttpStatic = class { body: unknown, callback: RequestCallback ) { - const req = XHRRequest.createRequest( + const req = xhrRequestImplementation.createRequest( uri, headers, params, @@ -104,10 +128,10 @@ const Http: IHttpStatic = class { ); }; } - } else if (Platform.Config.fetchSupported) { + } else if (Platform.Config.fetchSupported && fetchRequestImplementation) { this.supportsAuthHeaders = true; this.Request = (method, uri, headers, params, body, callback) => { - fetchRequest(method, client ?? null, uri, headers, params, body, callback); + fetchRequestImplementation(method, client ?? null, uri, headers, params, body, callback); }; this.checkConnectivity = function (callback: (err: ErrorInfo | null, connectivity: boolean) => void) { Logger.logAction(Logger.LOG_MICRO, '(Fetch)Http.checkConnectivity()', 'Sending; ' + connectivityCheckUrl); @@ -119,7 +143,10 @@ const Http: IHttpStatic = class { }; } else { this.Request = (method, uri, headers, params, body, callback) => { - callback(new PartialErrorInfo('no supported HTTP transports available', null, 400), null); + const error = hasImplementation + ? new PartialErrorInfo('no supported HTTP transports available', null, 400) + : createMissingImplementationError(); + callback(error, null); }; } } diff --git a/src/platform/web/lib/http/request/index.ts b/src/platform/web/lib/http/request/index.ts new file mode 100644 index 0000000000..4fccec5b3b --- /dev/null +++ b/src/platform/web/lib/http/request/index.ts @@ -0,0 +1,10 @@ +import { HTTPRequestImplementations } from '../http'; +import XHRRequest from './xhrrequest'; +import fetchRequest from './fetchrequest'; + +export const defaultBundledRequestImplementations: HTTPRequestImplementations = { + XHRRequest: XHRRequest, + FetchRequest: fetchRequest, +}; + +export const modulesBundledRequestImplementations: HTTPRequestImplementations = {}; diff --git a/src/platform/web/modules.ts b/src/platform/web/modules.ts index f9ee3a521d..e12ede52e7 100644 --- a/src/platform/web/modules.ts +++ b/src/platform/web/modules.ts @@ -15,6 +15,7 @@ import Logger from '../../common/lib/util/logger'; import { getDefaults } from '../../common/lib/util/defaults'; import WebStorage from './lib/util/webstorage'; import PlatformDefaults from './lib/util/defaults'; +import { modulesBundledRequestImplementations } from './lib/http/request'; Platform.BufferUtils = BufferUtils; Platform.Http = Http; @@ -22,6 +23,8 @@ Platform.Config = Config; Platform.Transports = ModulesTransports; Platform.WebStorage = WebStorage; +Http.bundledRequestImplementations = modulesBundledRequestImplementations; + Logger.initLogHandlers(); Platform.Defaults = getDefaults(PlatformDefaults); @@ -45,5 +48,6 @@ export * from './modules/presencemessage'; export * from './modules/msgpack'; export * from './modules/realtimepresence'; export * from './modules/transports'; +export * from './modules/http'; export { Rest } from '../../common/lib/client/rest'; export { BaseRest, BaseRealtime, ErrorInfo }; diff --git a/src/platform/web/modules/http.ts b/src/platform/web/modules/http.ts new file mode 100644 index 0000000000..24b664f30d --- /dev/null +++ b/src/platform/web/modules/http.ts @@ -0,0 +1,2 @@ +export { default as XHRRequest } from '../lib/http/request/xhrrequest'; +export { default as FetchRequest } from '../lib/http/request/fetchrequest'; diff --git a/test/browser/modules.test.js b/test/browser/modules.test.js index efaf654b3b..d73b76624e 100644 --- a/test/browser/modules.test.js +++ b/test/browser/modules.test.js @@ -17,6 +17,8 @@ import { XHRPolling, XHRStreaming, WebSocketTransport, + FetchRequest, + XHRRequest, } from '../../build/modules/index.js'; describe('browser/modules', function () { @@ -45,23 +47,21 @@ describe('browser/modules', function () { }); describe('without any modules', () => { - describe('BaseRest', () => { - it('can be constructed', () => { - expect(() => new BaseRest(ablyClientOptions(), {})).not.to.throw(); - }); - }); - - describe('BaseRealtime', () => { - it('throws an error due to absence of a transport module', () => { - expect(() => new BaseRealtime(ablyClientOptions(), {})).to.throw('no requested transports available'); + for (const clientClass of [BaseRest, BaseRealtime]) { + describe(clientClass.name, () => { + it('throws an error due to the absence of an HTTP module', () => { + expect(() => new clientClass(ablyClientOptions(), {})).to.throw( + 'No HTTP request module provided. Provide at least one of the FetchRequest or XHRRequest modules.' + ); + }); }); - }); + } }); describe('Rest', () => { describe('BaseRest without explicit Rest', () => { it('offers REST functionality', async () => { - const client = new BaseRest(ablyClientOptions(), {}); + const client = new BaseRest(ablyClientOptions(), { FetchRequest }); const time = await client.time(); expect(time).to.be.a('number'); }); @@ -69,7 +69,7 @@ describe('browser/modules', function () { describe('BaseRealtime with Rest', () => { it('offers REST functionality', async () => { - const client = new BaseRealtime(ablyClientOptions(), { WebSocketTransport, Rest }); + const client = new BaseRealtime(ablyClientOptions(), { WebSocketTransport, FetchRequest, Rest }); const time = await client.time(); expect(time).to.be.a('number'); }); @@ -77,7 +77,7 @@ describe('browser/modules', function () { describe('BaseRealtime without Rest', () => { it('throws an error when attempting to use REST functionality', async () => { - const client = new BaseRealtime(ablyClientOptions(), { WebSocketTransport }); + const client = new BaseRealtime(ablyClientOptions(), { WebSocketTransport, FetchRequest }); expect(() => client.time()).to.throw('Rest module not provided'); }); }); @@ -214,10 +214,10 @@ describe('browser/modules', function () { describe('Crypto', () => { describe('without Crypto', () => { async function testThrowsAnErrorWhenGivenChannelOptionsWithACipher(clientClassConfig) { - const client = new clientClassConfig.clientClass( - ablyClientOptions(), - clientClassConfig.additionalModules ?? {} - ); + const client = new clientClassConfig.clientClass(ablyClientOptions(), { + ...clientClassConfig.additionalModules, + FetchRequest, + }); const key = await generateRandomKey(); expect(() => client.channels.get('channel', { cipher: { key } })).to.throw('Crypto module not provided'); } @@ -242,7 +242,7 @@ describe('browser/modules', function () { // Publish the message on a channel configured to use encryption, and receive it on one not configured to use encryption - const rxClient = new BaseRealtime(clientOptions, { WebSocketTransport }); + const rxClient = new BaseRealtime(clientOptions, { WebSocketTransport, FetchRequest }); const rxChannel = rxClient.channels.get('channel'); await rxChannel.attach(); @@ -252,7 +252,8 @@ describe('browser/modules', function () { const txMessage = { name: 'message', data: 'data' }; const txClient = new clientClassConfig.clientClass(clientOptions, { - ...(clientClassConfig.additionalModules ?? {}), + ...clientClassConfig.additionalModules, + FetchRequest, Crypto, }); const txChannel = txClient.channels.get('channel', encryptionChannelOptions); @@ -318,7 +319,7 @@ describe('browser/modules', function () { describe('without MsgPack', () => { describe('BaseRest', () => { it('uses JSON', async () => { - const client = new BaseRest(ablyClientOptions({ useBinaryProtocol: true }), {}); + const client = new BaseRest(ablyClientOptions({ useBinaryProtocol: true }), { FetchRequest }); await testRestUsesContentType(client, 'application/json'); }); }); @@ -327,6 +328,7 @@ describe('browser/modules', function () { it('uses JSON', async () => { const client = new BaseRealtime(ablyClientOptions({ useBinaryProtocol: true, autoConnect: false }), { WebSocketTransport, + FetchRequest, }); await testRealtimeUsesFormat(client, 'json'); }); @@ -337,6 +339,7 @@ describe('browser/modules', function () { describe('BaseRest', () => { it('uses MessagePack', async () => { const client = new BaseRest(ablyClientOptions({ useBinaryProtocol: true }), { + FetchRequest, MsgPack, }); await testRestUsesContentType(client, 'application/x-msgpack'); @@ -347,6 +350,7 @@ describe('browser/modules', function () { it('uses MessagePack', async () => { const client = new BaseRealtime(ablyClientOptions({ useBinaryProtocol: true, autoConnect: false }), { WebSocketTransport, + FetchRequest, MsgPack, }); await testRealtimeUsesFormat(client, 'msgpack'); @@ -359,7 +363,7 @@ describe('browser/modules', function () { describe('RealtimePresence', () => { describe('BaseRealtime without RealtimePresence', () => { it('throws an error when attempting to access the `presence` property', () => { - const client = new BaseRealtime(ablyClientOptions(), { WebSocketTransport }); + const client = new BaseRealtime(ablyClientOptions(), { WebSocketTransport, FetchRequest }); const channel = client.channels.get('channel'); expect(() => channel.presence).to.throw('RealtimePresence module not provided'); @@ -368,12 +372,15 @@ describe('browser/modules', function () { describe('BaseRealtime with RealtimePresence', () => { it('offers realtime presence functionality', async () => { - const rxChannel = new BaseRealtime(ablyClientOptions(), { WebSocketTransport, RealtimePresence }).channels.get( - 'channel' - ); + const rxChannel = new BaseRealtime(ablyClientOptions(), { + WebSocketTransport, + FetchRequest, + RealtimePresence, + }).channels.get('channel'); const txClientId = randomString(); const txChannel = new BaseRealtime(ablyClientOptions({ clientId: txClientId }), { WebSocketTransport, + FetchRequest, RealtimePresence, }).channels.get('channel'); @@ -435,6 +442,14 @@ describe('browser/modules', function () { describe('Transports', () => { describe('BaseRealtime', () => { + describe('without a transport module', () => { + it('throws an error due to absence of a transport module', () => { + expect(() => new BaseRealtime(ablyClientOptions(), { FetchRequest })).to.throw( + 'no requested transports available' + ); + }); + }); + for (const scenario of [ { moduleMapKey: 'WebSocketTransport', transportModule: WebSocketTransport, transportName: 'web_socket' }, { moduleMapKey: 'XHRPolling', transportModule: XHRPolling, transportName: 'xhr_polling' }, @@ -445,6 +460,7 @@ describe('browser/modules', function () { const realtime = new BaseRealtime( ablyClientOptions({ autoConnect: false, transports: [scenario.transportName] }), { + FetchRequest, [scenario.moduleMapKey]: scenario.transportModule, } ); @@ -468,4 +484,24 @@ describe('browser/modules', function () { } }); }); + + describe('HTTP request implementations', () => { + describe('with multiple HTTP request implementations', () => { + it('prefers XHR', async () => { + let usedXHR = false; + + const XHRRequestSpy = class XHRRequestSpy extends XHRRequest { + static createRequest(...args) { + usedXHR = true; + return super.createRequest(...args); + } + }; + + const rest = new BaseRest(ablyClientOptions(), { FetchRequest, XHRRequest: XHRRequestSpy }); + await rest.time(); + + expect(usedXHR).to.be.true; + }); + }); + }); });