Skip to content

Commit

Permalink
Make HTTP request implementations tree-shakable
Browse files Browse the repository at this point in the history
We expose XHRRequest and FetchRequest modules.

Resolves #1395.
  • Loading branch information
lawrence-forooghian committed Oct 26, 2023
1 parent afecc41 commit 768eb0c
Show file tree
Hide file tree
Showing 13 changed files with 143 additions and 16 deletions.
2 changes: 2 additions & 0 deletions scripts/moduleReport.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ const moduleNames = [
'XHRPolling',
'XHRStreaming',
'WebSocketTransport',
'XHRRequest',
'FetchRequest',
];

// List of all free-standing functions exported by the library along with the
Expand Down
10 changes: 10 additions & 0 deletions src/common/lib/client/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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);
Expand Down
5 changes: 5 additions & 0 deletions src/common/lib/client/baserest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
}
Expand Down
4 changes: 4 additions & 0 deletions src/common/lib/client/modulesmap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -12,6 +14,8 @@ export interface ModulesMap {
WebSocketTransport?: TransportInitialiser;
XHRPolling?: TransportInitialiser;
XHRStreaming?: TransportInitialiser;
XHRRequest?: typeof XHRRequest;
FetchRequest?: typeof fetchRequest;
}

export const allCommonModules: ModulesMap = { Rest };
3 changes: 3 additions & 0 deletions src/platform/nativescript/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -33,6 +34,8 @@ for (const clientClass of [DefaultRest, DefaultRealtime]) {
clientClass._MsgPack = msgpack;
}

Http.bundledRequestImplementations = defaultBundledRequestImplementations;

Logger.initLogHandlers();

Platform.Defaults = getDefaults(PlatformDefaults);
Expand Down
3 changes: 3 additions & 0 deletions src/platform/react-native/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -33,6 +34,8 @@ for (const clientClass of [DefaultRest, DefaultRealtime]) {
clientClass._MsgPack = msgpack;
}

Http.bundledRequestImplementations = defaultBundledRequestImplementations;

Logger.initLogHandlers();

Platform.Defaults = getDefaults(PlatformDefaults);
Expand Down
3 changes: 3 additions & 0 deletions src/platform/web-noencryption/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -27,6 +28,8 @@ for (const clientClass of [DefaultRest, DefaultRealtime]) {
clientClass._MsgPack = msgpack;
}

Http.bundledRequestImplementations = defaultBundledRequestImplementations;

Logger.initLogHandlers();

Platform.Defaults = getDefaults(PlatformDefaults);
Expand Down
3 changes: 3 additions & 0 deletions src/platform/web/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -31,6 +32,8 @@ for (const clientClass of [DefaultRest, DefaultRealtime]) {
clientClass._MsgPack = msgpack;
}

Http.bundledRequestImplementations = defaultBundledRequestImplementations;

Logger.initLogHandlers();

Platform.Defaults = getDefaults(PlatformDefaults);
Expand Down
45 changes: 36 additions & 9 deletions src/platform/web/lib/http/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<ModulesMap, 'XHRRequest' | 'FetchRequest'>;

function shouldFallback(errorInfo: ErrorInfo) {
const statusCode = errorInfo.statusCode as number;
Expand Down Expand Up @@ -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<StandardCallback<boolean>> | null = null;
private client: BaseClient | null;

Expand All @@ -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,
Expand All @@ -61,7 +85,7 @@ const Http: IHttpStatic = class {
body: unknown,
callback: RequestCallback
) {
const req = XHRRequest.createRequest(
const req = xhrRequestImplementation.createRequest(
uri,
headers,
params,
Expand Down Expand Up @@ -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);
Expand All @@ -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);
};
}
}
Expand Down
10 changes: 10 additions & 0 deletions src/platform/web/lib/http/request/index.ts
Original file line number Diff line number Diff line change
@@ -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 = {};
4 changes: 4 additions & 0 deletions src/platform/web/modules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,16 @@ 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;
Platform.Config = Config;
Platform.Transports = ModulesTransports;
Platform.WebStorage = WebStorage;

Http.bundledRequestImplementations = modulesBundledRequestImplementations;

Logger.initLogHandlers();

Platform.Defaults = getDefaults(PlatformDefaults);
Expand All @@ -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 };
2 changes: 2 additions & 0 deletions src/platform/web/modules/http.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export { default as XHRRequest } from '../lib/http/request/xhrrequest';
export { default as FetchRequest } from '../lib/http/request/fetchrequest';
65 changes: 58 additions & 7 deletions test/browser/modules.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import {
XHRPolling,
XHRStreaming,
WebSocketTransport,
FetchRequest,
XHRRequest,
} from '../../build/modules/index.js';

describe('browser/modules', function () {
Expand Down Expand Up @@ -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.'
);
});
});

Expand All @@ -60,15 +64,15 @@ 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');
});
});

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');
});
Expand Down Expand Up @@ -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, () => {
Expand Down Expand Up @@ -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, () => {
Expand Down Expand Up @@ -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');
});
});
Expand All @@ -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');
Expand Down Expand Up @@ -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;
});
});
});
});

0 comments on commit 768eb0c

Please sign in to comment.