[SDK-3975] Strip logging from modular variant of the SDK

.eslintrc.js

@@ -58,6 +58,7 @@ module.exports = {
     "typedoc/generated",
     "react",
     "Gruntfile.js",
+    "grunt",
   ],
   settings: {
     jsdoc: {

Gruntfile.js

@@ -7,6 +7,7 @@ var esbuild = require('esbuild');
 var umdWrapper = require('esbuild-plugin-umd-wrapper');
 var banner = require('./src/fragments/license');
 var process = require('process');
+var stripLogsPlugin = require('./grunt/esbuild/strip-logs').default;
 
 module.exports = function (grunt) {
   grunt.loadNpmTasks('grunt-contrib-concat');
@@ -110,35 +111,41 @@ module.exports = function (grunt) {
   grunt.registerTask('build:browser', function () {
     var done = this.async();
 
-    var baseConfig = {
-      entryPoints: ['src/platform/web/index.ts'],
-      outfile: 'build/ably.js',
-      bundle: true,
-      sourcemap: true,
-      format: 'umd',
-      banner: { js: '/*' + banner + '*/' },
-      plugins: [umdWrapper.default()],
-      target: 'es6',
-    };
-
-    var modulesConfig = {
-      ...baseConfig,
-      entryPoints: ['src/platform/web/modules.ts'],
-      outfile: 'build/modules/index.js',
-      format: 'esm',
-      plugins: [],
-    };
+    function createBaseConfig() {
+      return {
+        entryPoints: ['src/platform/web/index.ts'],
+        outfile: 'build/ably.js',
+        bundle: true,
+        sourcemap: true,
+        format: 'umd',
+        banner: { js: '/*' + banner + '*/' },
+        plugins: [umdWrapper.default()],
+        target: 'es6',
+      };
+    }
 
-    // For reasons I don't understand this build fails when run asynchronously
-    esbuild.buildSync(modulesConfig);
+    function createModulesConfig() {
+      return {
+        // We need to create a new copy of the base config, because calling
+        // esbuild.build() with the base config causes it to mutate the passed
+        // config’s `banner.js` property to add some weird modules shim code,
+        // which we don’t want here.
+        ...createBaseConfig(),
+        entryPoints: ['src/platform/web/modules.ts'],
+        outfile: 'build/modules/index.js',
+        format: 'esm',
+        plugins: [stripLogsPlugin],
+      };
+    }
 
     Promise.all([
-      esbuild.build(baseConfig),
+      esbuild.build(createBaseConfig()),
       esbuild.build({
-        ...baseConfig,
+        ...createBaseConfig(),
         outfile: 'build/ably.min.js',
         minify: true,
       }),
+      esbuild.build(createModulesConfig()),
     ]).then(() => {
       console.log('esbuild succeeded');
       done(true);

README.md

@@ -106,7 +106,14 @@ You must provide:
 
 `BaseRealtime` offers the same API as the `Realtime` class described in the rest of this `README`. This means that you can develop an application using the default variant of the SDK and switch to the modular version when you wish to optimize your bundle size.
 
-For more information, see the [generated documentation](https://sdk.ably.com/builds/ably/ably-js/main/typedoc/modules/modules.html) (this link points to the documentation for the `main` branch).
+In order to further reduce bundle size, the modular variant of the SDK performs less logging than the default variant. It only logs:
+
+- messages that have a `logLevel` of 1 (that is, errors)
+- a small number of other network events
+
+If you need more verbose logging, use the default variant of the SDK.
+
+For more information about the modular variant of the SDK, see the [generated documentation](https://sdk.ably.com/builds/ably/ably-js/main/typedoc/modules/modules.html) (this link points to the documentation for the `main` branch).
 
 ### TypeScript
 

grunt/esbuild/strip-logs.js

@@ -0,0 +1,111 @@
+var path = require('path');
+var fs = require('fs');
+var babel = {
+  types: require('@babel/types'),
+  parser: require('@babel/parser'),
+  traverse: require('@babel/traverse'),
+  generator: require('@babel/generator'),
+};
+
+// This function is copied from
+// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions#escaping
+function escapeRegExp(string) {
+  return string.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); // $& means the whole matched string
+}
+
+// This esbuild plugin strips all log messages from the modular variant of
+// the library, except for error-level logs and other logging statements
+// explicitly marked as not to be stripped.
+const stripLogsPlugin = {
+  name: 'stripLogs',
+  setup(build) {
+    let foundLogToStrip = false;
+    let foundErrorLog = false;
+    let foundNoStripLog = false;
+
+    const filter = new RegExp(`^${escapeRegExp(path.join(__dirname, '..', '..', 'src') + path.sep)}.*\\.[tj]s$`);
+    build.onLoad({ filter }, async (args) => {
+      const contents = (await fs.promises.readFile(args.path)).toString();
+      const lines = contents.split('\n');
+      const ast = babel.parser.parse(contents, { sourceType: 'module', plugins: ['typescript'] });
+      const errors = [];
+
+      babel.traverse.default(ast, {
+        enter(path) {
+          if (
+            path.isCallExpression() &&
+            babel.types.isMemberExpression(path.node.callee) &&
+            babel.types.isIdentifier(path.node.callee.object, { name: 'Logger' })
+          ) {
+            if (babel.types.isIdentifier(path.node.callee.property, { name: 'logAction' })) {
+              const firstArgument = path.node.arguments[0];
+
+              if (
+                babel.types.isMemberExpression(firstArgument) &&
+                babel.types.isIdentifier(firstArgument.object, { name: 'Logger' }) &&
+                firstArgument.property.name.startsWith('LOG_')
+              ) {
+                if (firstArgument.property.name === 'LOG_ERROR') {
+                  // `path` is a call to `Logger.logAction(Logger.LOG_ERROR, ...)`; preserve it.
+                  foundErrorLog = true;
+                } else {
+                  // `path` is a call to `Logger.logAction(Logger.LOG_*, ...) for some other log level; strip it.
+                  foundLogToStrip = true;
+                  path.remove();
+                }
+              } else {
+                // `path` is a call to `Logger.logAction(...)` with some argument other than a `Logger.LOG_*` expression; raise an error because we can’t determine whether to strip it.
+                errors.push({
+                  location: {
+                    file: args.path,
+                    column: firstArgument.loc.start.column,
+                    line: firstArgument.loc.start.line,
+                    lineText: lines[firstArgument.loc.start.line - 1],
+                  },
+                  text: `First argument passed to Logger.logAction() must be Logger.LOG_*, got \`${
+                    babel.generator.default(firstArgument).code
+                  }\``,
+                });
+              }
+            } else if (babel.types.isIdentifier(path.node.callee.property, { name: 'logActionNoStrip' })) {
+              // `path` is a call to `Logger.logActionNoStrip(...)`; preserve it.
+              foundNoStripLog = true;
+            }
+          }
+        },
+      });
+
+      return { contents: babel.generator.default(ast).code, loader: 'ts', errors };
+    });
+
+    build.onEnd(() => {
+      const errorMessages = [];
+
+      // Perform a sense check to make sure that we found some logging
+      // calls to strip (to protect us against accidentally changing the
+      // internal logging API in such a way that would cause us to no
+      // longer strip any calls).
+
+      if (!foundLogToStrip) {
+        errorMessages.push('Did not find any Logger.logAction(...) calls to strip');
+      }
+
+      // Perform a sense check to make sure that we found some logging
+      // calls to preserve (to protect us against accidentally changing the
+      // internal logging API in such a way that would cause us to
+      // accidentally strip all logging calls).
+
+      if (!foundErrorLog) {
+        errorMessages.push('Did not find any Logger.logAction(Logger.LOG_ERROR, ...) calls to preserve');
+      }
+
+      if (!foundNoStripLog) {
+        errorMessages.push('Did not find any Logger.logActionNoStrip(...) calls to preserve');
+      }
+
+      return { errors: errorMessages.map((text) => ({ text })) };
+    });
+  },
+};
+
+exports.default = stripLogsPlugin;

modules.d.ts

@@ -273,6 +273,15 @@ export interface ModulesMap {
  * A client that offers a simple stateless API to interact directly with Ably's REST API.
  *
  * `BaseRest` is the equivalent, in the modular variant of the Ably Client Library SDK, of the [`Rest`](../../default/classes/Rest.html) class in the default variant of the SDK. The difference is that its constructor allows you to decide exactly which functionality the client should include. This allows unused functionality to be tree-shaken, reducing bundle size.
+ *
+ * > **Note**
+ * >
+ * > In order to further reduce bundle size, `BaseRest` performs less logging than the `Rest` class exported by the default variant of the SDK. It only logs:
+ * >
+ * > - messages that have a {@link ClientOptions.logLevel | `logLevel`} of 1 (that is, errors)
+ * > - a small number of other network events
+ * >
+ * > If you need more verbose logging, use the default variant of the SDK.
  */
 export declare class BaseRest extends AbstractRest {
   /**
@@ -292,6 +301,15 @@ export declare class BaseRest extends AbstractRest {
  * A client that extends the functionality of {@link BaseRest} and provides additional realtime-specific features.
  *
  * `BaseRealtime` is the equivalent, in the modular variant of the Ably Client Library SDK, of the [`Realtime`](../../default/classes/Realtime.html) class in the default variant of the SDK. The difference is that its constructor allows you to decide exactly which functionality the client should include. This allows unused functionality to be tree-shaken, reducing bundle size.
+ *
+ * > **Note**
+ * >
+ * > In order to further reduce bundle size, `BaseRealtime` performs less logging than the `Realtime` class exported by the default variant of the SDK. It only logs:
+ * >
+ * > - messages that have a {@link ClientOptions.logLevel | `logLevel`} of 1 (that is, errors)
+ * > - a small number of other network events
+ * >
+ * > If you need more verbose logging, use the default variant of the SDK.
  */
 export declare class BaseRealtime extends AbstractRealtime {
   /**

package.json

@@ -48,6 +48,9 @@
   "devDependencies": {
     "@ably/vcdiff-decoder": "1.0.6",
     "@arethetypeswrong/cli": "^0.13.1",
+    "@babel/generator": "^7.23.6",
+    "@babel/parser": "^7.23.6",
+    "@babel/traverse": "^7.23.7",
     "@testing-library/react": "^13.3.0",
     "@types/jmespath": "^0.15.2",
     "@types/node": "^18.0.0",
@@ -132,8 +135,8 @@
     "lint": "eslint .",
     "lint:fix": "eslint --fix .",
     "prepare": "npm run build",
-    "format": "prettier --write --ignore-path .gitignore --ignore-path .prettierignore src test ably.d.ts modules.d.ts webpack.config.js Gruntfile.js scripts/*.[jt]s docs/chrome-mv3.md",
-    "format:check": "prettier --check --ignore-path .gitignore --ignore-path .prettierignore src test ably.d.ts modules.d.ts webpack.config.js Gruntfile.js scripts/*.[jt]s",
+    "format": "prettier --write --ignore-path .gitignore --ignore-path .prettierignore src test ably.d.ts modules.d.ts webpack.config.js Gruntfile.js scripts/*.[jt]s docs/chrome-mv3.md grunt",
+    "format:check": "prettier --check --ignore-path .gitignore --ignore-path .prettierignore src test ably.d.ts modules.d.ts webpack.config.js Gruntfile.js scripts/*.[jt]s grunt",
     "sourcemap": "source-map-explorer build/ably.min.js",
     "modulereport": "tsc --noEmit scripts/moduleReport.ts && esr scripts/moduleReport.ts",
     "docs": "typedoc"

scripts/moduleReport.ts

@@ -3,7 +3,7 @@ import * as path from 'path';
 import { explore } from 'source-map-explorer';
 
 // The maximum size we allow for a minimal useful Realtime bundle (i.e. one that can subscribe to a channel)
-const minimalUsefulRealtimeBundleSizeThresholdKiB = 109;
+const minimalUsefulRealtimeBundleSizeThresholdKiB = 94;
 
 // List of all modules accepted in ModulesMap
 const moduleNames = [

src/common/lib/client/realtimechannel.ts

@@ -750,12 +750,13 @@ class RealtimeChannel extends EventEmitter {
       this.errorReason = reason;
     }
     const change = new ChannelStateChange(this.state, state, resumed, hasBacklog, reason);
-    const logLevel = state === 'failed' ? Logger.LOG_ERROR : Logger.LOG_MAJOR;
-    Logger.logAction(
-      logLevel,
-      'Channel state for channel "' + this.name + '"',
-      state + (reason ? '; reason: ' + reason : '')
-    );
+    const action = 'Channel state for channel "' + this.name + '"';
+    const message = state + (reason ? '; reason: ' + reason : '');
+    if (state === 'failed') {
+      Logger.logAction(Logger.LOG_ERROR, action, message);
+    } else {
+      Logger.logAction(Logger.LOG_MAJOR, action, message);
+    }
 
     if (state !== 'attaching' && state !== 'suspended') {
       this.retryCount = 0;

src/common/lib/transport/connectionmanager.ts

@@ -1187,12 +1187,13 @@ class ConnectionManager extends EventEmitter {
   }
 
   enactStateChange(stateChange: ConnectionStateChange): void {
-    const logLevel = stateChange.current === 'failed' ? Logger.LOG_ERROR : Logger.LOG_MAJOR;
-    Logger.logAction(
-      logLevel,
-      'Connection state',
-      stateChange.current + (stateChange.reason ? '; reason: ' + stateChange.reason : '')
-    );
+    const action = 'Connection state';
+    const message = stateChange.current + (stateChange.reason ? '; reason: ' + stateChange.reason : '');
+    if (stateChange.current === 'failed') {
+      Logger.logAction(Logger.LOG_ERROR, action, message);
+    } else {
+      Logger.logAction(Logger.LOG_MAJOR, action, message);
+    }
     Logger.logAction(
       Logger.LOG_MINOR,
       'ConnectionManager.enactStateChange',

src/common/lib/transport/protocol.ts

@@ -71,7 +71,7 @@ class Protocol extends EventEmitter {
       this.messageQueue.push(pendingMessage);
     }
     if (Logger.shouldLog(Logger.LOG_MICRO)) {
-      Logger.logAction(
+      Logger.logActionNoStrip(
         Logger.LOG_MICRO,
         'Protocol.send()',
         'sending msg; ' +

src/common/lib/transport/transport.ts

@@ -117,7 +117,7 @@ abstract class Transport extends EventEmitter {
 
   onProtocolMessage(message: ProtocolMessage): void {
     if (Logger.shouldLog(Logger.LOG_MICRO)) {
-      Logger.logAction(
+      Logger.logActionNoStrip(
         Logger.LOG_MICRO,
         'Transport.onProtocolMessage()',
         'received on ' +
@@ -132,7 +132,7 @@ abstract class Transport extends EventEmitter {
 
     switch (message.action) {
       case actions.HEARTBEAT:
-        Logger.logAction(
+        Logger.logActionNoStrip(
           Logger.LOG_MICRO,
           'Transport.onProtocolMessage()',
           this.shortName + ' heartbeat; connectionId = ' + this.connectionManager.connectionId

src/common/lib/util/logger.ts

@@ -98,11 +98,23 @@ class Logger {
   }
 
   /* public static functions */
+  /**
+   * In the modular variant of the SDK, the `stripLogs` esbuild plugin strips out all calls to this method (when invoked as `Logger.logAction(...)`) except when called with level `Logger.LOG_ERROR`. If you wish for a log statement to never be stripped, use the {@link logActionNoStrip} method instead.
+   *
+   * The aforementioned plugin expects `level` to be an expression of the form `Logger.LOG_*`; that is, you can’t dynamically specify the log level.
+   */
   static logAction = (level: LogLevels, action: string, message?: string) => {
+    this.logActionNoStrip(level, action, message);
+  };
+
+  /**
+   * Calls to this method are never stripped by the `stripLogs` esbuild plugin. Use it for log statements that you wish to always be included in the modular variant of the SDK.
+   */
+  static logActionNoStrip(level: LogLevels, action: string, message?: string) {
     if (Logger.shouldLog(level)) {
       (level === LogLevels.Error ? Logger.logErrorHandler : Logger.logHandler)('Ably: ' + action + ': ' + message);
     }
-  };
+  }
 
   /* Where a logging operation is expensive, such as serialisation of data, use shouldLog will prevent
 	   the object being serialised if the log level will not output the message */