feat: Apply private property naming standard. Mangle browser private properties. (#620)

1. Update private properties and methods to start with a leading
underscore.
2. Update browser terser config to mangle properties that start with an
underscore that are not `_meta`. (It would be nice to have a more
elegant way to do this, but this is what we have.)
3. Add linting rules for typescript naming conventions.
5. Fix lint issues from rules.

Conventions

Private properties and methods start with _ and CamelCase case names.
Public methods and properties use CamelCase and do not start with an
underscore.
Public static properties use PascalCase and do not start with an
underscore. (This is negotiable, but required fewer changes.)

sdk-764

Author: kinyoklion

.eslintrc.js

@@ -47,6 +47,38 @@ module.exports = {
     'jest/no-focused-tests': 'error',
     'jest/no-identical-title': 'error',
     'jest/valid-expect': 'error',
+    'no-underscore-dangle': ['error', { allowAfterThis: true }],
+    '@typescript-eslint/naming-convention': [
+      'error',
+      {
+        selector: ['method'],
+        format: ['camelCase'],
+        leadingUnderscore: 'forbid',
+      },
+      {
+        selector: ['method'],
+        format: ['camelCase'],
+        modifiers: ['private'],
+        leadingUnderscore: 'require',
+      },
+      {
+        selector: ['classProperty', 'parameterProperty'],
+        format: ['camelCase'],
+        leadingUnderscore: 'forbid',
+      },
+      {
+        selector: ['classProperty', 'parameterProperty'],
+        modifiers: ['static'],
+        format: ['PascalCase'],
+        leadingUnderscore: 'forbid',
+      },
+      {
+        selector: ['classProperty', 'parameterProperty'],
+        modifiers: ['private'],
+        format: ['camelCase'],
+        leadingUnderscore: 'require',
+      },
+    ],
   },
   globals: {
     BigInt: 'readonly',

.github/workflows/release-please.yml

@@ -191,7 +191,6 @@ jobs:
           workspace_path: packages/sdk/browser
           aws_assume_role: ${{ vars.AWS_ROLE_ARN }}
 
-
   release-server-node:
     runs-on: ubuntu-latest
     needs: ['release-please', 'release-sdk-server']

packages/sdk/akamai-edgekv/src/edgekv/edgeKVProvider.ts

@@ -8,15 +8,15 @@ type EdgeKVProviderParams = {
 };
 
 export default class EdgeKVProvider implements EdgeProvider {
-  private edgeKv: EdgeKV;
+  private _edgeKv: EdgeKV;
 
   constructor({ namespace, group }: EdgeKVProviderParams) {
-    this.edgeKv = new EdgeKV({ namespace, group } as any);
+    this._edgeKv = new EdgeKV({ namespace, group } as any);
   }
 
   async get(rootKey: string): Promise<string | null | undefined> {
     try {
-      return await this.edgeKv.getText({ item: rootKey } as any);
+      return await this._edgeKv.getText({ item: rootKey } as any);
     } catch (e) {
       /* empty */
     }

packages/sdk/browser/README.md

@@ -9,6 +9,7 @@
 -->
 
 # ⛔️⛔️⛔️⛔️
+
 > [!CAUTION]
 > This library is a alpha version and should not be considered ready for production use while this message is visible.
 

packages/sdk/browser/contract-tests/entity/src/ClientEntity.ts

@@ -75,17 +75,17 @@ function makeDefaultInitialContext() {
 
 export class ClientEntity {
   constructor(
-    private readonly client: LDClient,
-    private readonly logger: LDLogger,
+    private readonly _client: LDClient,
+    private readonly _logger: LDLogger,
   ) {}
 
   close() {
-    this.client.close();
-    this.logger.info('Test ended');
+    this._client.close();
+    this._logger.info('Test ended');
   }
 
   async doCommand(params: CommandParams) {
-    this.logger.info(`Received command: ${params.command}`);
+    this._logger.info(`Received command: ${params.command}`);
     switch (params.command) {
       case CommandType.EvaluateFlag: {
         const evaluationParams = params.evaluate;
@@ -95,23 +95,23 @@ export class ClientEntity {
         if (evaluationParams.detail) {
           switch (evaluationParams.valueType) {
             case ValueType.Bool:
-              return this.client.boolVariationDetail(
+              return this._client.boolVariationDetail(
                 evaluationParams.flagKey,
                 evaluationParams.defaultValue as boolean,
               );
             case ValueType.Int: // Intentional fallthrough.
             case ValueType.Double:
-              return this.client.numberVariationDetail(
+              return this._client.numberVariationDetail(
                 evaluationParams.flagKey,
                 evaluationParams.defaultValue as number,
               );
             case ValueType.String:
-              return this.client.stringVariationDetail(
+              return this._client.stringVariationDetail(
                 evaluationParams.flagKey,
                 evaluationParams.defaultValue as string,
               );
             default:
-              return this.client.variationDetail(
+              return this._client.variationDetail(
                 evaluationParams.flagKey,
                 evaluationParams.defaultValue,
               );
@@ -120,42 +120,45 @@ export class ClientEntity {
         switch (evaluationParams.valueType) {
           case ValueType.Bool:
             return {
-              value: this.client.boolVariation(
+              value: this._client.boolVariation(
                 evaluationParams.flagKey,
                 evaluationParams.defaultValue as boolean,
               ),
             };
           case ValueType.Int: // Intentional fallthrough.
           case ValueType.Double:
             return {
-              value: this.client.numberVariation(
+              value: this._client.numberVariation(
                 evaluationParams.flagKey,
                 evaluationParams.defaultValue as number,
               ),
             };
           case ValueType.String:
             return {
-              value: this.client.stringVariation(
+              value: this._client.stringVariation(
                 evaluationParams.flagKey,
                 evaluationParams.defaultValue as string,
               ),
             };
           default:
             return {
-              value: this.client.variation(evaluationParams.flagKey, evaluationParams.defaultValue),
+              value: this._client.variation(
+                evaluationParams.flagKey,
+                evaluationParams.defaultValue,
+              ),
             };
         }
       }
 
       case CommandType.EvaluateAllFlags:
-        return { state: this.client.allFlags() };
+        return { state: this._client.allFlags() };
 
       case CommandType.IdentifyEvent: {
         const identifyParams = params.identifyEvent;
         if (!identifyParams) {
           throw malformedCommand;
         }
-        await this.client.identify(identifyParams.user || identifyParams.context);
+        await this._client.identify(identifyParams.user || identifyParams.context);
         return undefined;
       }
 
@@ -164,7 +167,7 @@ export class ClientEntity {
         if (!customEventParams) {
           throw malformedCommand;
         }
-        this.client.track(
+        this._client.track(
           customEventParams.eventKey,
           customEventParams.data,
           customEventParams.metricValue,
@@ -173,7 +176,7 @@ export class ClientEntity {
       }
 
       case CommandType.FlushEvents:
-        this.client.flush();
+        this._client.flush();
         return undefined;
 
       default:

packages/sdk/browser/contract-tests/entity/src/TestHarnessWebSocket.ts

@@ -4,31 +4,31 @@ import { ClientEntity, newSdkClientEntity } from './ClientEntity';
 import { makeLogger } from './makeLogger';
 
 export default class TestHarnessWebSocket {
-  private ws?: WebSocket;
-  private readonly entities: Record<string, ClientEntity> = {};
-  private clientCounter = 0;
-  private logger: LDLogger = makeLogger('TestHarnessWebSocket');
+  private _ws?: WebSocket;
+  private readonly _entities: Record<string, ClientEntity> = {};
+  private _clientCounter = 0;
+  private _logger: LDLogger = makeLogger('TestHarnessWebSocket');
 
-  constructor(private readonly url: string) {}
+  constructor(private readonly _url: string) {}
 
   connect() {
-    this.logger.info(`Connecting to web socket.`);
-    this.ws = new WebSocket(this.url, ['v1']);
-    this.ws.onopen = () => {
-      this.logger.info('Connected to websocket.');
+    this._logger.info(`Connecting to web socket.`);
+    this._ws = new WebSocket(this._url, ['v1']);
+    this._ws.onopen = () => {
+      this._logger.info('Connected to websocket.');
     };
-    this.ws.onclose = () => {
-      this.logger.info('Websocket closed. Attempting to reconnect in 1 second.');
+    this._ws.onclose = () => {
+      this._logger.info('Websocket closed. Attempting to reconnect in 1 second.');
       setTimeout(() => {
         this.connect();
       }, 1000);
     };
-    this.ws.onerror = (err) => {
-      this.logger.info(`error:`, err);
+    this._ws.onerror = (err) => {
+      this._logger.info(`error:`, err);
     };
 
-    this.ws.onmessage = async (msg) => {
-      this.logger.info('Test harness message', msg);
+    this._ws.onmessage = async (msg) => {
+      this._logger.info('Test harness message', msg);
       const data = JSON.parse(msg.data);
       const resData: any = { reqId: data.reqId };
       switch (data.command) {
@@ -46,33 +46,33 @@ export default class TestHarnessWebSocket {
           break;
         case 'createClient':
           {
-            resData.resourceUrl = `/clients/${this.clientCounter}`;
+            resData.resourceUrl = `/clients/${this._clientCounter}`;
             resData.status = 201;
             const entity = await newSdkClientEntity(data.body);
-            this.entities[this.clientCounter] = entity;
-            this.clientCounter += 1;
+            this._entities[this._clientCounter] = entity;
+            this._clientCounter += 1;
           }
           break;
         case 'runCommand':
-          if (Object.prototype.hasOwnProperty.call(this.entities, data.id)) {
-            const entity = this.entities[data.id];
+          if (Object.prototype.hasOwnProperty.call(this._entities, data.id)) {
+            const entity = this._entities[data.id];
             const body = await entity.doCommand(data.body);
             resData.body = body;
             resData.status = body ? 200 : 204;
           } else {
             resData.status = 404;
-            this.logger.warn(`Client did not exist: ${data.id}`);
+            this._logger.warn(`Client did not exist: ${data.id}`);
           }
 
           break;
         case 'deleteClient':
-          if (Object.prototype.hasOwnProperty.call(this.entities, data.id)) {
-            const entity = this.entities[data.id];
+          if (Object.prototype.hasOwnProperty.call(this._entities, data.id)) {
+            const entity = this._entities[data.id];
             entity.close();
-            delete this.entities[data.id];
+            delete this._entities[data.id];
           } else {
             resData.status = 404;
-            this.logger.warn(`Could not delete client because it did not exist: ${data.id}`);
+            this._logger.warn(`Could not delete client because it did not exist: ${data.id}`);
           }
           break;
         default:
@@ -84,10 +84,10 @@ export default class TestHarnessWebSocket {
   }
 
   disconnect() {
-    this.ws?.close();
+    this._ws?.close();
   }
 
   send(data: unknown) {
-    this.ws?.send(JSON.stringify(data));
+    this._ws?.send(JSON.stringify(data));
   }
 }

packages/sdk/browser/rollup.config.js

@@ -33,14 +33,32 @@ export default [
         esmExternals: true,
       }),
       resolve(),
-      terser(),
+      terser({
+        mangle: {
+          properties: {
+            regex: /^_/,
+          },
+        },
+      }),
       json(),
       // The 'sourcemap' option allows using the minified size, not the size before minification.
       visualizer({ sourcemap: true }),
     ],
   },
   {
     ...getSharedConfig('cjs', 'dist/index.cjs.js'),
-    plugins: [typescript(), common(), resolve(), terser(), json()],
+    plugins: [
+      typescript(),
+      common(),
+      resolve(),
+      terser({
+        mangle: {
+          properties: {
+            regex: /^_/,
+          },
+        },
+      }),
+      json(),
+    ],
   },
 ];

packages/sdk/browser/src/BrowserClient.ts

@@ -86,10 +86,10 @@ export type LDClient = Omit<
 };
 
 export class BrowserClient extends LDClientImpl implements LDClient {
-  private readonly goalManager?: GoalManager;
+  private readonly _goalManager?: GoalManager;
 
   constructor(
-    private readonly clientSideId: string,
+    clientSideId: string,
     autoEnvAttributes: AutoEnvAttributes,
     options: BrowserOptions = {},
     overridePlatform?: Platform,
@@ -174,7 +174,7 @@ export class BrowserClient extends LDClientImpl implements LDClient {
     this.setEventSendingEnabled(true, false);
 
     if (validatedBrowserOptions.fetchGoals) {
-      this.goalManager = new GoalManager(
+      this._goalManager = new GoalManager(
         clientSideId,
         platform.requests,
         baseUrl,
@@ -215,7 +215,7 @@ export class BrowserClient extends LDClientImpl implements LDClient {
       // "waitForGoalsReady", then we would make an async immediately invoked function expression
       // which emits the event, and assign its promise to a member. The "waitForGoalsReady" function
       // would return that promise.
-      this.goalManager.initialize();
+      this._goalManager.initialize();
 
       if (validatedBrowserOptions.automaticBackgroundHandling) {
         registerStateDetection(() => this.flush());
@@ -225,7 +225,7 @@ export class BrowserClient extends LDClientImpl implements LDClient {
 
   override async identify(context: LDContext, identifyOptions?: LDIdentifyOptions): Promise<void> {
     await super.identify(context, identifyOptions);
-    this.goalManager?.startTracking();
+    this._goalManager?.startTracking();
   }
 
   setStreaming(streaming?: boolean): void {
@@ -235,7 +235,7 @@ export class BrowserClient extends LDClientImpl implements LDClient {
     browserDataManager.setForcedStreaming(streaming);
   }
 
-  private updateAutomaticStreamingState() {
+  private _updateAutomaticStreamingState() {
     const browserDataManager = this.dataManager as BrowserDataManager;
     // This will need changed if support for listening to individual flag change
     // events it added.
@@ -244,11 +244,11 @@ export class BrowserClient extends LDClientImpl implements LDClient {
 
   override on(eventName: LDEmitterEventName, listener: Function): void {
     super.on(eventName, listener);
-    this.updateAutomaticStreamingState();
+    this._updateAutomaticStreamingState();
   }
 
   override off(eventName: LDEmitterEventName, listener: Function): void {
     super.off(eventName, listener);
-    this.updateAutomaticStreamingState();
+    this._updateAutomaticStreamingState();
   }
 }