Merge pull request #228 from ably-labs/test-typing-evil-eslint

style: enable strict and type-checked lints on eslint

.eslintrc.cjs

@@ -8,11 +8,14 @@ module.exports = {
   parser: '@typescript-eslint/parser',
   parserOptions: {
     sourceType: 'module',
+    project: ['./tsconfig.json', './tsconfig.test.json'],
   },
   plugins: ['@typescript-eslint', 'security', 'jsdoc', 'import', 'simple-import-sort'],
   extends: [
     'eslint:recommended',
-    'plugin:@typescript-eslint/recommended',
+    'plugin:@typescript-eslint/recommended-type-checked',
+    'plugin:@typescript-eslint/strict-type-checked',
+    'plugin:@typescript-eslint/stylistic-type-checked',
     'plugin:security/recommended-legacy',
     'plugin:import/recommended',
   ],
@@ -35,6 +38,7 @@ module.exports = {
         // TypeScript already enforces these rules better than any eslint setup can
         'no-undef': 'off',
         'no-dupe-class-members': 'off',
+        'require-await': 'off',
         // see:
         // https://github.com/ably/spaces/issues/76
         // https://github.com/microsoft/TypeScript/issues/16577#issuecomment-703190339
@@ -47,8 +51,28 @@ module.exports = {
         ],
       },
     },
+    {
+      files: ['test/**/*.{ts,tsx}'],
+      rules: {
+        '@typescript-eslint/unbound-method': 'off',
+        '@typescript-eslint/no-empty-function': 'off',
+      },
+    },
+  ],
+  ignorePatterns: [
+    '.eslintrc.cjs',
+    'dist',
+    'node_modules',
+    'ably-common',
+    'typedoc',
+    'src/utils',
+    'test/utils',
+    'scripts/cdn_deploy.js',
+    'vitest.config.ts',
+    'vite.config.ts',
+    'test/helper/testSetup.ts',
+    '__mocks__',
   ],
-  ignorePatterns: ['dist', 'node_modules', 'ably-common', 'typedoc', 'src/utils', 'scripts/cdn_deploy.js'],
   settings: {
     jsdoc: {
       tagNamePreference: {

__mocks__/ably/index.ts

@@ -2,7 +2,10 @@ import * as Ably from 'ably';
 
 const MOCK_CLIENT_ID = 'MOCK_CLIENT_ID';
 
-const mockPromisify = <T>(expectedReturnValue): Promise<T> => new Promise((resolve) => resolve(expectedReturnValue));
+const mockPromisify = <T>(expectedReturnValue): Promise<T> =>
+  new Promise((resolve) => {
+    resolve(expectedReturnValue);
+  });
 const methodReturningVoidPromise = () => mockPromisify<void>((() => {})());
 
 function createMockPresence() {
@@ -18,8 +21,8 @@ function createMockPresence() {
         fn();
       },
     },
-    subscribe: () => {},
-    unsubscribe: () => {},
+    subscribe: methodReturningVoidPromise,
+    unsubscribe: methodReturningVoidPromise,
   };
 }
 
@@ -37,8 +40,8 @@ function createMockChannel() {
     attach: methodReturningVoidPromise,
     detach: methodReturningVoidPromise,
     presence: createMockPresence(),
-    subscribe: () => {},
-    unsubscribe: () => {},
+    subscribe: methodReturningVoidPromise,
+    unsubscribe: methodReturningVoidPromise,
     on: () => {},
     off: () => {},
     publish: () => {},
@@ -79,7 +82,7 @@ class MockRealtime {
       state: 'connected',
     };
 
-    this['options'] = {};
+    this.options = {};
   }
 }
 

src/ChatApi.ts

@@ -70,19 +70,18 @@ export class ChatApi {
     method: 'POST' | 'GET' | ' PUT' | 'DELETE' | 'PATCH',
     body?: REQ,
   ): Promise<RES> {
-    const response = await this.realtime.request(method, url, 1.1, {}, body);
+    const response = await this.realtime.request<RES>(method, url, 1.1, {}, body);
     if (!response.success) {
       this._logger.error('ChatApi.makeAuthorisedRequest(); failed to make request', {
         url,
         statusCode: response.statusCode,
         errorCode: response.errorCode,
         errorMessage: response.errorMessage,
       });
-      throw new Ably.ErrorInfo(response.errorMessage, response.errorCode, response.statusCode);
+      throw new Ably.ErrorInfo(response.errorMessage, response.errorCode, response.statusCode) as unknown as Error;
     }
 
-    const [result] = response.items;
-    return result as RES;
+    return response.items[0] as RES;
   }
 
   private async makeAuthorisedPaginatedRequest<RES, REQ = undefined>(
@@ -98,7 +97,7 @@ export class ChatApi {
         errorCode: response.errorCode,
         errorMessage: response.errorMessage,
       });
-      throw new Ably.ErrorInfo(response.errorMessage, response.errorCode, response.statusCode);
+      throw new Ably.ErrorInfo(response.errorMessage, response.errorCode, response.statusCode) as unknown as Error;
     }
     return response;
   }

src/Message.ts

@@ -106,9 +106,13 @@ export class DefaultMessage implements Message {
    */
   private static timeserialCompare(first: Message, second: Message): number {
     const firstTimeserial =
-      (first as DefaultMessage)._calculatedTimeserial ?? DefaultMessage.calculateTimeserial(first.timeserial);
+      first instanceof DefaultMessage
+        ? first._calculatedTimeserial
+        : DefaultMessage.calculateTimeserial(first.timeserial);
     const secondTimeserial =
-      (second as DefaultMessage)._calculatedTimeserial ?? DefaultMessage.calculateTimeserial(second.timeserial);
+      second instanceof DefaultMessage
+        ? second._calculatedTimeserial
+        : DefaultMessage.calculateTimeserial(second.timeserial);
 
     // Compare the timestamp
     const timestampDiff = firstTimeserial.timestamp - secondTimeserial.timestamp;

src/Messages.ts

@@ -192,7 +192,7 @@ export class DefaultMessages extends EventEmitter<MessageEventsMap> implements M
     if (!hasListeners) {
       this._logger.debug('Messages.subscribe(); subscribing internal listener');
       this._internalListener = this.processEvent.bind(this);
-      return this._managedChannel.subscribe([MessageEvents.created], this._internalListener!);
+      return this._managedChannel.subscribe([MessageEvents.created], this._internalListener);
     }
 
     return this._managedChannel.channel.attach();
@@ -209,7 +209,11 @@ export class DefaultMessages extends EventEmitter<MessageEventsMap> implements M
     }
 
     this._logger.debug('Messages.unsubscribe(); unsubscribing internal listener');
-    return this._managedChannel.unsubscribe(this._internalListener!);
+    if (this._internalListener) {
+      return this._managedChannel.unsubscribe(this._internalListener);
+    }
+
+    return Promise.resolve();
   }
 
   private processEvent(channelEventMessage: Ably.InboundMessage) {
@@ -238,40 +242,54 @@ export class DefaultMessages extends EventEmitter<MessageEventsMap> implements M
    * Validate the realtime message and convert it to a chat message.
    */
   private parseNewMessage(channelEventMessage: Ably.InboundMessage): Message | undefined {
-    const { data, clientId, timestamp, extras } = channelEventMessage;
+    interface MessagePayload {
+      data?: {
+        content?: string;
+      };
+      clientId?: string;
+      timestamp?: number;
+      extras?: {
+        timeserial?: string;
+      };
+    }
+    const messageCreatedMessage = channelEventMessage as MessagePayload;
 
-    if (!data) {
+    if (!messageCreatedMessage.data) {
       this._logger.error(`received incoming message without data`, channelEventMessage);
       return;
     }
 
-    if (!clientId) {
+    if (!messageCreatedMessage.clientId) {
       this._logger.error(`received incoming message without clientId`, channelEventMessage);
       return;
     }
 
-    if (!timestamp) {
+    if (!messageCreatedMessage.timestamp) {
       this._logger.error(`received incoming message without timestamp`, channelEventMessage);
       return;
     }
 
-    const { content } = data;
-    if (!content) {
+    if (messageCreatedMessage.data.content === undefined) {
       this._logger.error(`received incoming message without content`, channelEventMessage);
       return;
     }
 
-    if (!extras) {
+    if (!messageCreatedMessage.extras) {
       this._logger.error(`received incoming message without extras`, channelEventMessage);
       return;
     }
 
-    const { timeserial } = extras;
-    if (!timeserial) {
+    if (!messageCreatedMessage.extras.timeserial) {
       this._logger.error(`received incoming message without timeserial`, channelEventMessage);
       return;
     }
 
-    return new DefaultMessage(timeserial, clientId, this._roomId, content, new Date(timestamp));
+    return new DefaultMessage(
+      messageCreatedMessage.extras.timeserial,
+      messageCreatedMessage.clientId,
+      this._roomId,
+      messageCreatedMessage.data.content,
+      new Date(messageCreatedMessage.timestamp),
+    );
   }
 }

src/Occupancy.ts

@@ -46,7 +46,7 @@ export interface Occupancy {
 /**
  * Represents the occupancy of a chat room.
  */
-export type OccupancyEvent = {
+export interface OccupancyEvent {
   /**
    * The number of connections to the chat room.
    */
@@ -56,7 +56,7 @@ export type OccupancyEvent = {
    * The number of presence members in the chat room - members who have entered presence.
    */
   presenceMembers: number;
-};
+}
 
 /**
  * A listener that is called when the occupancy of a chat room changes.
@@ -99,7 +99,7 @@ export class DefaultOccupancy extends EventEmitter<OccupancyEventsMap> implement
       this._logger.debug('Occupancy.subscribe(); adding internal listener');
       this._internalListener = this.internalOccupancyListener.bind(this);
       return this._managedChannel
-        .subscribe(['[meta]occupancy'], this._internalListener as Ably.messageCallback<Ably.InboundMessage>)
+        .subscribe(['[meta]occupancy'], this._internalListener)
         .then(async (stateChange: Ably.ChannelStateChange | null) => {
           await this._managedChannel.channel.setOptions({ params: { occupancy: 'metrics' } });
           return stateChange;
@@ -119,9 +119,7 @@ export class DefaultOccupancy extends EventEmitter<OccupancyEventsMap> implement
       this._logger.debug('Occupancy.unsubscribe(); removing internal listener');
       return this._managedChannel.channel
         .setOptions({})
-        .then(() =>
-          this._managedChannel.unsubscribe(this._internalListener as Ably.messageCallback<Ably.InboundMessage>),
-        )
+        .then(() => (this._internalListener ? this._managedChannel.unsubscribe(this._internalListener) : null))
         .then(() => {
           this._internalListener = undefined;
         });
@@ -150,9 +148,7 @@ export class DefaultOccupancy extends EventEmitter<OccupancyEventsMap> implement
    * occupancy events for the public API.
    */
   private internalOccupancyListener(message: Ably.InboundMessage): void {
-    const {
-      data: { metrics },
-    } = message;
+    const { metrics } = message.data as { metrics?: { connections?: number; presenceMembers?: number } };
 
     if (metrics === undefined) {
       this._logger.error('invalid occupancy event received; metrics is missing', message);

src/Presence.ts

@@ -18,9 +18,7 @@ interface PresenceEventsMap {
 /**
  * Type for PresenceData
  */
-export type PresenceData = {
-  [key: string]: unknown;
-};
+export type PresenceData = Record<string, unknown>;
 
 /**
  * Type for AblyPresenceData
@@ -209,13 +207,27 @@ export class DefaultPresence extends EventEmitter<PresenceEventsMap> implements
   async get(params?: Ably.RealtimePresenceParams): Promise<PresenceMember[]> {
     this._logger.trace('Presence.get()', { params });
     const userOnPresence = await this.subscriptionManager.channel.presence.get(params);
+    const userDataToReturn = (data: string) => {
+      try {
+        const parsedData = JSON.parse(data) as AblyPresenceData;
+        if (!parsedData.userCustomData) {
+          return undefined;
+        }
+
+        return parsedData.userCustomData;
+      } catch (error) {
+        this._logger.error('Presence.get(); error parsing user data', { error });
+        return undefined;
+      }
+    };
 
     // ably-js never emits the 'absent' event, so we can safely ignore it here.
     return userOnPresence.map((user) => ({
       clientId: user.clientId,
       action: user.action as PresenceEvents,
-      data: user.data ? (JSON.parse(user.data).userCustomData as PresenceData) : undefined,
+      data: userDataToReturn(user.data as string),
       timestamp: user.timestamp,
+      // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
       extras: user.extras,
       id: user.id,
     }));
@@ -286,7 +298,7 @@ export class DefaultPresence extends EventEmitter<PresenceEventsMap> implements
     this._logger.trace('Presence.subscribe(); listenerOrEvents', { listenerOrEvents });
     if (!listenerOrEvents && !listener) {
       this._logger.error('could not subscribe to presence; invalid arguments');
-      throw new Ably.ErrorInfo('could not subscribe listener: invalid arguments', 40000, 400);
+      throw new Ably.ErrorInfo('could not subscribe listener: invalid arguments', 40000, 400) as unknown as Error;
     }
     const hasListeners = this.hasListeners();
     if (!listener) {
@@ -298,7 +310,9 @@ export class DefaultPresence extends EventEmitter<PresenceEventsMap> implements
       this._logger.debug('Presence.subscribe(); adding internal listener');
       return this.subscriptionManager.presenceSubscribe(this.subscribeToEvents);
     }
-    return this.subscriptionManager.channel.attach().then(() => {});
+    return this.subscriptionManager.channel.attach().then(() => {
+      return Promise.resolve();
+    });
   }
 
   /**
@@ -320,7 +334,7 @@ export class DefaultPresence extends EventEmitter<PresenceEventsMap> implements
     this._logger.trace('Presence.unsubscribe(); listenerOrEvents', { listenerOrEvents });
     if (!listenerOrEvents && !listener) {
       this._logger.error('could not unsubscribe from presence; invalid arguments');
-      throw new Ably.ErrorInfo('could not unsubscribe listener: invalid arguments', 40000, 400);
+      throw new Ably.ErrorInfo('could not unsubscribe listener: invalid arguments', 40000, 400) as unknown as Error;
     }
     if (!listener) {
       this.off(listenerOrEvents);
@@ -342,11 +356,10 @@ export class DefaultPresence extends EventEmitter<PresenceEventsMap> implements
    */
   subscribeToEvents = (member: Ably.PresenceMessage) => {
     try {
-      const parsedData = JSON.parse(member.data);
-
+      const parsedData = JSON.parse(member.data as string) as AblyPresenceData;
       // ably-js never emits the 'absent' event, so we can safely ignore it here.
-      this.emit(PresenceEvents[member.action as PresenceEvents], {
-        action: PresenceEvents[member.action as PresenceEvents],
+      this.emit(member.action as PresenceEvents, {
+        action: member.action as PresenceEvents,
         clientId: member.clientId,
         timestamp: member.timestamp,
         data: parsedData.userCustomData,
@@ -358,7 +371,7 @@ export class DefaultPresence extends EventEmitter<PresenceEventsMap> implements
         50000,
         500,
         (error as Error).message,
-      );
+      ) as unknown as Error;
     }
   };
 }