Make BaseClient._MsgPack nullable

This is preparation for #1375 (making MessagePack functionality tree-shakable), in which there may not be a MessagePack implementation available. We update normaliseClientOptions so that it only sets useBinaryProtocol to true if there is a MessagePack implementation available.
ably · Aug 22, 2023 · 151ae5c · 151ae5c
1 parent d057ea8
commit 151ae5c
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 7 deletions.
diff --git a/src/common/lib/client/baseclient.ts b/src/common/lib/client/baseclient.ts
@@ -14,6 +14,7 @@ import { ModulesMap } from './modulesmap';
 import { Rest } from './rest';
 import { IUntypedCryptoStatic } from 'common/types/ICryptoStatic';
 import { throwMissingModuleError } from '../util/utils';
+import { MsgPack } from 'common/types/msgpack';
 
 /**
  `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.
@@ -30,7 +31,14 @@ class BaseClient {
 
   private readonly _rest: Rest | null;
   readonly _Crypto: IUntypedCryptoStatic | null;
-  readonly _MsgPack = Platform.Config.msgpack;
+  private readonly __MsgPack: MsgPack | null = Platform.Config.msgpack;
+
+  get _MsgPack(): MsgPack {
+    if (!this.__MsgPack) {
+      throw new ErrorInfo('MsgPack not available', 400, 40000);
+    }
+    return this.__MsgPack;
+  }
 
   constructor(options: ClientOptions | string, modules: ModulesMap) {
     if (!options) {
@@ -47,7 +55,7 @@ class BaseClient {
       'initialized with clientOptions ' + Platform.Config.inspect(options)
     );
 
-    const normalOptions = (this.options = Defaults.normaliseOptions(optionsObj));
+    const normalOptions = (this.options = Defaults.normaliseOptions(optionsObj, this.__MsgPack));
 
     /* process options */
     if (normalOptions.key) {

diff --git a/src/common/lib/util/defaults.ts b/src/common/lib/util/defaults.ts
@@ -5,6 +5,7 @@ import ErrorInfo from 'common/lib/types/errorinfo';
 import { version } from '../../../../package.json';
 import ClientOptions, { InternalClientOptions, NormalisedClientOptions } from 'common/types/ClientOptions';
 import IDefaults from '../../types/IDefaults';
+import { MsgPack } from 'common/types/msgpack';
 
 let agent = 'ably-js/' + version;
 
@@ -41,7 +42,7 @@ type CompleteDefaults = IDefaults & {
   checkHost(host: string): void;
   getRealtimeHost(options: ClientOptions, production: boolean, environment: string): string;
   objectifyOptions(options: ClientOptions | string): ClientOptions;
-  normaliseOptions(options: InternalClientOptions): NormalisedClientOptions;
+  normaliseOptions(options: InternalClientOptions, MsgPack: MsgPack | null): NormalisedClientOptions;
   defaultGetHeaders(options: NormalisedClientOptions, headersOptions?: HeadersOptions): Record<string, string>;
   defaultPostHeaders(options: NormalisedClientOptions, headersOptions?: HeadersOptions): Record<string, string>;
 };
@@ -185,7 +186,7 @@ export function objectifyOptions(options: ClientOptions | string): ClientOptions
   return options;
 }
 
-export function normaliseOptions(options: InternalClientOptions): NormalisedClientOptions {
+export function normaliseOptions(options: InternalClientOptions, MsgPack: MsgPack | null): NormalisedClientOptions {
   if (typeof options.recover === 'function' && options.closeOnUnload === true) {
     Logger.logAction(
       Logger.LOG_ERROR,
@@ -222,10 +223,14 @@ export function normaliseOptions(options: InternalClientOptions): NormalisedClie
 
   const timeouts = getTimeouts(options);
 
-  if ('useBinaryProtocol' in options) {
-    options.useBinaryProtocol = Platform.Config.supportsBinary && options.useBinaryProtocol;
+  if (MsgPack) {
+    if ('useBinaryProtocol' in options) {
+      options.useBinaryProtocol = Platform.Config.supportsBinary && options.useBinaryProtocol;
+    } else {
+      options.useBinaryProtocol = Platform.Config.preferBinary;
+    }
   } else {
-    options.useBinaryProtocol = Platform.Config.preferBinary;
+    options.useBinaryProtocol = false;
   }
 
   const headers: Record<string, string> = {};

diff --git a/test/rest/defaults.test.js b/test/rest/defaults.test.js
@@ -135,6 +135,28 @@ define(['ably', 'chai'], function (Ably, chai) {
       Defaults.ENVIRONMENT = '';
     });
 
+    // 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('normaliseOptions with useBinaryProtocol == true', () => {
+      if (Ably.Realtime.Platform.Config.supportsBinary) {
+        describe('given MsgPack implementation', () => {
+          it('maintains useBinaryProtocol as true', () => {
+            const StubMsgPack = {};
+            var normalisedOptions = Defaults.normaliseOptions({ useBinaryProtocol: true }, StubMsgPack);
+
+            expect(normalisedOptions.useBinaryProtocol).to.be.true;
+          });
+        });
+      }
+
+      describe('given no MsgPack implementation', () => {
+        it('changes useBinaryProtocol to false', () => {
+          var normalisedOptions = Defaults.normaliseOptions({ useBinaryProtocol: true }, null);
+
+          expect(normalisedOptions.useBinaryProtocol).to.be.false;
+        });
+      });
+    });
+
     it('closeOnUnload', function () {
       var options;