From 768eb0c7da443d5c559b0954668c162859ad26e3 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 We expose XHRRequest and FetchRequest modules. Resolves #1395. --- scripts/moduleReport.js | 2 + src/common/lib/client/baseclient.ts | 10 ++++ src/common/lib/client/baserest.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 | 65 +++++++++++++++++++--- 13 files changed, 143 insertions(+), 16 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 a8937d594b..609a3d1e93 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 72a61f7ff2..2e72d57232 100644 --- a/src/common/lib/client/baseclient.ts +++ b/src/common/lib/client/baseclient.ts @@ -14,6 +14,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'; /** `BaseClient` acts as the base class for all of the client classes exported by the SDK. It is an implementation detail and this class is not advertised publicly. @@ -28,11 +29,20 @@ class BaseClient { http: IHttp; auth: Auth; + // Indicates whether this client requires the ability to make HTTP requests. Overridden by BaseRest. + get _requiresHTTP() { + return false; + } + 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/baserest.ts b/src/common/lib/client/baserest.ts index 247145f989..91b8fd8157 100644 --- a/src/common/lib/client/baserest.ts +++ b/src/common/lib/client/baserest.ts @@ -9,6 +9,11 @@ import { Rest } from './rest'; It always includes the `Rest` module. */ export class BaseRest extends BaseClient { + // Overrides the corresponding property in BaseClient. (A BaseRest client that can’t make HTTP requests would be useless.) + get _requiresHTTP() { + return true; + } + constructor(options: ClientOptions | string, modules: ModulesMap) { super(options, { Rest, ...modules }); } 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 f6a3efed6c..23358f840b 100644 --- a/src/platform/nativescript/index.ts +++ b/src/platform/nativescript/index.ts @@ -18,6 +18,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); @@ -33,6 +34,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 35bf6be8b7..bb7defd213 100644 --- a/src/platform/react-native/index.ts +++ b/src/platform/react-native/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'; const Config = configFactory(BufferUtils); @@ -33,6 +34,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 7864e8e47c..09c1ac50fd 100644 --- a/src/platform/web-noencryption/index.ts +++ b/src/platform/web-noencryption/index.ts @@ -15,6 +15,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; @@ -27,6 +28,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 87c1b40c21..532222f444 100644 --- a/src/platform/web/index.ts +++ b/src/platform/web/index.ts @@ -16,6 +16,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); @@ -31,6 +32,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 0b594b1dc9..c80dc30c80 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 (client?._requiresHTTP && !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 c1a2e63e23..e0907b322b 100644 --- a/src/platform/web/modules.ts +++ b/src/platform/web/modules.ts @@ -14,6 +14,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; @@ -21,6 +22,8 @@ Platform.Config = Config; Platform.Transports = ModulesTransports; Platform.WebStorage = WebStorage; +Http.bundledRequestImplementations = modulesBundledRequestImplementations; + Logger.initLogHandlers(); Platform.Defaults = getDefaults(PlatformDefaults); @@ -44,5 +47,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 }; 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 6c70913b11..ab04ac42b4 100644 --- a/test/browser/modules.test.js +++ b/test/browser/modules.test.js @@ -16,6 +16,8 @@ import { XHRPolling, XHRStreaming, WebSocketTransport, + FetchRequest, + XHRRequest, } from '../../build/modules/index.js'; describe('browser/modules', function () { @@ -45,8 +47,10 @@ describe('browser/modules', function () { describe('without any modules', () => { describe('BaseRest', () => { - it('can be constructed', () => { - expect(() => new BaseRest(ablyClientOptions(), {})).not.to.throw(); + it('throws an error due to the absence of an HTTP module', () => { + expect(() => new BaseRest(ablyClientOptions(), {})).to.throw( + 'No HTTP request module provided. Provide at least one of the FetchRequest or XHRRequest modules.' + ); }); }); @@ -60,7 +64,7 @@ describe('browser/modules', function () { 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'); }); @@ -68,7 +72,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, Rest, FetchRequest }); const time = await client.time(); expect(time).to.be.a('number'); }); @@ -222,7 +226,7 @@ describe('browser/modules', function () { } for (const clientClassConfig of [ - { clientClass: BaseRest }, + { clientClass: BaseRest, additionalModules: { FetchRequest } }, { clientClass: BaseRealtime, additionalModules: { WebSocketTransport } }, ]) { describe(clientClassConfig.clientClass.name, () => { @@ -268,7 +272,7 @@ describe('browser/modules', function () { } for (const clientClassConfig of [ - { clientClass: BaseRest }, + { clientClass: BaseRest, additionalModules: { FetchRequest } }, { clientClass: BaseRealtime, additionalModules: { WebSocketTransport } }, ]) { describe(clientClassConfig.clientClass.name, () => { @@ -317,7 +321,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'); }); }); @@ -336,6 +340,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'); @@ -457,4 +462,50 @@ describe('browser/modules', function () { } }); }); + + describe('HTTP request implementations', () => { + describe('without an HTTP request implementation', () => { + describe('BaseRealtime', () => { + it('is still able to publish messages', async () => { + const realtime = new BaseRealtime(ablyClientOptions(), { WebSocketTransport }); + const channel = realtime.channels.get('channel'); + await channel.publish(); + }); + + it('throws an error when attempting to use REST functionality', async () => { + const realtime = new BaseRealtime(ablyClientOptions(), { Rest, WebSocketTransport }); + + let thrownError = null; + try { + await realtime.time(); + } catch (error) { + thrownError = error; + } + + expect(thrownError).not.to.be.null; + expect(thrownError.message).to.equal( + 'No HTTP request module provided. Provide at least one of the FetchRequest or XHRRequest modules.' + ); + }); + }); + }); + + 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; + }); + }); + }); });