Skip to content

Commit

Permalink
Make MessagePack functionality tree-shakable
Browse files Browse the repository at this point in the history
We move the MessagePack functionality into a tree-shakable MsgPack
module.

Resolves #1375.
  • Loading branch information
lawrence-forooghian committed Aug 21, 2023
1 parent afff1c8 commit f320e0a
Show file tree
Hide file tree
Showing 17 changed files with 106 additions and 15 deletions.
2 changes: 1 addition & 1 deletion scripts/moduleReport.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const esbuild = require('esbuild');

// List of all modules accepted in ModulesMap
const moduleNames = ['Rest', 'Crypto'];
const moduleNames = ['Rest', 'Crypto', 'MsgPack'];

// List of all free-standing functions exported by the library along with the
// ModulesMap entries that we expect them to transitively import
Expand Down
5 changes: 3 additions & 2 deletions src/common/lib/client/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ class BaseClient {

private readonly _rest: Rest | null;
readonly _Crypto: IUntypedCryptoStatic | null;
private readonly __MsgPack: MsgPack | null = Platform.Config.msgpack;
private readonly __MsgPack: MsgPack | null;

get _MsgPack(): MsgPack {
if (!this.__MsgPack) {
throw new ErrorInfo('MsgPack not available', 400, 40000);
throwMissingModuleError('MsgPack');
}
return this.__MsgPack;
}
Expand All @@ -55,6 +55,7 @@ class BaseClient {
'initialized with clientOptions ' + Platform.Config.inspect(options)
);

this.__MsgPack = modules.MsgPack ?? null;
const normalOptions = (this.options = Defaults.normaliseOptions(optionsObj, this.__MsgPack));

/* process options */
Expand Down
10 changes: 9 additions & 1 deletion src/common/lib/client/defaultrealtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,19 @@ import ConnectionManager from '../transport/connectionmanager';
import ProtocolMessage from '../types/protocolmessage';
import Platform from 'common/platform';
import { DefaultMessage } from '../types/defaultmessage';
import { MsgPack } from 'common/types/msgpack';

/**
`DefaultRealtime` is the class that the non tree-shakable version of the SDK exports as `Realtime`. It ensures that this version of the SDK includes all of the functionality which is optionally available in the tree-shakable version.
*/
export class DefaultRealtime extends BaseRealtime {
constructor(options: ClientOptions) {
super(options, { ...allCommonModules, Crypto: DefaultRealtime.Crypto ?? undefined });
const MsgPack = DefaultRealtime._MsgPack;
if (!MsgPack) {
throw new Error('Expected DefaultRealtime._MsgPack to have been set');
}

super(options, { ...allCommonModules, Crypto: DefaultRealtime.Crypto ?? undefined, MsgPack });
}

static Utils = Utils;
Expand All @@ -32,4 +38,6 @@ export class DefaultRealtime extends BaseRealtime {
}

static Message = DefaultMessage;

static _MsgPack: MsgPack | null = null;
}
14 changes: 13 additions & 1 deletion src/common/lib/client/defaultrest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,23 @@ import ClientOptions from '../../types/ClientOptions';
import { allCommonModules } from './modulesmap';
import Platform from 'common/platform';
import { DefaultMessage } from '../types/defaultmessage';
import { MsgPack } from 'common/types/msgpack';

/**
`DefaultRest` is the class that the non tree-shakable version of the SDK exports as `Rest`. It ensures that this version of the SDK includes all of the functionality which is optionally available in the tree-shakable version.
*/
export class DefaultRest extends BaseRest {
constructor(options: ClientOptions | string) {
super(options, { ...allCommonModules, Crypto: DefaultRest.Crypto ?? undefined });
const MsgPack = DefaultRest._MsgPack;
if (!MsgPack) {
throw new Error('Expected DefaultRest._MsgPack to have been set');
}

super(options, {
...allCommonModules,
Crypto: DefaultRest.Crypto ?? undefined,
MsgPack: DefaultRest._MsgPack ?? undefined,
});
}

private static _Crypto: typeof Platform.Crypto = null;
Expand All @@ -25,4 +35,6 @@ export class DefaultRest extends BaseRest {
}

static Message = DefaultMessage;

static _MsgPack: MsgPack | null = null;
}
2 changes: 2 additions & 0 deletions src/common/lib/client/modulesmap.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { Rest } from './rest';
import { IUntypedCryptoStatic } from '../../types/ICryptoStatic';
import { MsgPack } from 'common/types/msgpack';

export interface ModulesMap {
Rest?: typeof Rest;
Crypto?: IUntypedCryptoStatic;
MsgPack?: MsgPack;
}

export const allCommonModules: ModulesMap = { Rest };
3 changes: 0 additions & 3 deletions src/common/types/IPlatformConfig.d.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
import { MsgPack } from './msgpack';

export interface IPlatformConfig {
agent: string;
logTimestamps: boolean;
binaryType: BinaryType;
WebSocket: typeof WebSocket | typeof import('ws');
useProtocolHeartbeats: boolean;
msgpack: MsgPack;
supportsBinary: boolean;
preferBinary: boolean;
nextTick: process.nextTick;
Expand Down
2 changes: 0 additions & 2 deletions src/platform/nativescript/config.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
/* eslint-disable no-undef */
import msgpack from '../web/lib/util/msgpack';
require('nativescript-websockets');

var randomBytes;
Expand Down Expand Up @@ -28,7 +27,6 @@ var Config = {
allowComet: true,
streamingSupported: false,
useProtocolHeartbeats: true,
msgpack: msgpack,
supportsBinary: typeof TextDecoder !== 'undefined' && TextDecoder,
preferBinary: false,
ArrayBuffer: ArrayBuffer,
Expand Down
1 change: 1 addition & 0 deletions src/platform/nativescript/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ Platform.WebStorage = WebStorage;

for (const clientClass of [DefaultRest, DefaultRealtime]) {
clientClass.Crypto = Crypto;
clientClass._MsgPack = msgpack;
}

Logger.initLogHandlers();
Expand Down
1 change: 0 additions & 1 deletion src/platform/nodejs/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ const Config: IPlatformConfig = {
binaryType: 'nodebuffer' as BinaryType,
WebSocket,
useProtocolHeartbeats: false,
msgpack: require('@ably/msgpack-js'),
supportsBinary: true,
preferBinary: true,
nextTick: process.nextTick,
Expand Down
2 changes: 2 additions & 0 deletions src/platform/nodejs/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import Transports from './lib/transport';
import Logger from '../../common/lib/util/logger';
import { getDefaults } from '../../common/lib/util/defaults';
import PlatformDefaults from './lib/util/defaults';
import msgpack = require('@ably/msgpack-js');

const Crypto = createCryptoClass(BufferUtils);

Expand All @@ -26,6 +27,7 @@ Platform.WebStorage = null;

for (const clientClass of [DefaultRest, DefaultRealtime]) {
clientClass.Crypto = Crypto;
clientClass._MsgPack = msgpack;
}

Logger.initLogHandlers();
Expand Down
2 changes: 0 additions & 2 deletions src/platform/react-native/config.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import msgpack from '../web/lib/util/msgpack';
import { IPlatformConfig } from '../../common/types/IPlatformConfig';
import BufferUtils from '../web/lib/util/bufferutils';

Expand All @@ -13,7 +12,6 @@ export default function (bufferUtils: typeof BufferUtils): IPlatformConfig {
allowComet: true,
streamingSupported: true,
useProtocolHeartbeats: true,
msgpack: msgpack,
supportsBinary: !!(typeof TextDecoder !== 'undefined' && TextDecoder),
preferBinary: false,
ArrayBuffer: typeof ArrayBuffer !== 'undefined' && ArrayBuffer,
Expand Down
1 change: 1 addition & 0 deletions src/platform/react-native/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ Platform.WebStorage = WebStorage;

for (const clientClass of [DefaultRest, DefaultRealtime]) {
clientClass.Crypto = Crypto;
clientClass._MsgPack = msgpack;
}

Logger.initLogHandlers();
Expand Down
2 changes: 0 additions & 2 deletions src/platform/web/config.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import msgpack from './lib/util/msgpack';
import { IPlatformConfig } from '../../common/types/IPlatformConfig';
import * as Utils from 'common/lib/util/utils';

Expand Down Expand Up @@ -37,7 +36,6 @@ const Config: IPlatformConfig = {
allowComet: allowComet(),
streamingSupported: true,
useProtocolHeartbeats: true,
msgpack: msgpack,
supportsBinary: !!globalObject.TextDecoder,
preferBinary: false,
ArrayBuffer: globalObject.ArrayBuffer,
Expand Down
1 change: 1 addition & 0 deletions src/platform/web/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ Platform.WebStorage = WebStorage;

for (const clientClass of [DefaultRest, DefaultRealtime]) {
clientClass.Crypto = Crypto;
clientClass._MsgPack = msgpack;
}

Logger.initLogHandlers();
Expand Down
1 change: 1 addition & 0 deletions src/platform/web/modules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,6 @@ if (Platform.Config.noUpgrade) {

export * from './modules/crypto';
export * from './modules/message';
export * from './modules/msgpack';
export { Rest } from '../../common/lib/client/rest';
export { BaseRest, BaseRealtime };
1 change: 1 addition & 0 deletions src/platform/web/modules/msgpack.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default as MsgPack } from '../lib/util/msgpack';
71 changes: 71 additions & 0 deletions test/browser/modules.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
decodeMessages,
decodeEncryptedMessages,
Crypto,
MsgPack,
} from '../../build/modules/index.js';

describe('browser/modules', function () {
Expand Down Expand Up @@ -232,4 +233,74 @@ describe('browser/modules', function () {
}
});
});

describe('MsgPack', () => {
async function testRestUsesContentType(rest, expectedContentType) {
const channelName = 'channel';
const channel = rest.channels.get(channelName);
const contentTypeUsedForPublishPromise = new Promise((resolve, reject) => {
rest.http.do = (method, client, path, headers, body, params, callback) => {
if (!(method == 'post' && path == `/channels/${channelName}/messages`)) {
return;
}
resolve(headers['content-type']);
callback(null);
};
});

await channel.publish('message', 'body');

const contentTypeUsedForPublish = await contentTypeUsedForPublishPromise;
expect(contentTypeUsedForPublish).to.equal(expectedContentType);
}

async function testRealtimeUsesFormat(realtime, expectedFormat) {
const formatUsedForConnectionPromise = new Promise((resolve, reject) => {
realtime.connection.connectionManager.connectImpl = (transportParams) => {
resolve(transportParams.format);
};
});
realtime.connect();

const formatUsedForConnection = await formatUsedForConnectionPromise;
expect(formatUsedForConnection).to.equal(expectedFormat);
}

// TODO once https://github.com/ably/ably-js/issues/1424 is fixed, this should also test the case where the useBinaryProtocol option is not specified
describe('with useBinaryProtocol client option', () => {
describe('without MsgPack', () => {
describe('BaseRest', () => {
it('uses JSON', async () => {
const client = new BaseRest(ablyClientOptions({ useBinaryProtocol: true }), {});
await testRestUsesContentType(client, 'application/json');
});
});

describe('BaseRealtime', () => {
it('uses JSON', async () => {
const client = new BaseRealtime(ablyClientOptions({ useBinaryProtocol: true, autoConnect: false }), {});
await testRealtimeUsesFormat(client, 'json');
});
});
});

describe('with MsgPack', () => {
describe('BaseRest', () => {
it('uses MessagePack', async () => {
const client = new BaseRest(ablyClientOptions({ useBinaryProtocol: true }), { MsgPack });
await testRestUsesContentType(client, 'application/x-msgpack');
});
});

describe('BaseRealtime', () => {
it('uses MessagePack', async () => {
const client = new BaseRealtime(ablyClientOptions({ useBinaryProtocol: true, autoConnect: false }), {
MsgPack,
});
await testRealtimeUsesFormat(client, 'msgpack');
});
});
});
});
});
});

0 comments on commit f320e0a

Please sign in to comment.