From ac6ec14f7ae79cc12ee43ebd6dec34baf84c3555 Mon Sep 17 00:00:00 2001 From: Andy Ford Date: Tue, 25 Jun 2024 22:36:19 +0100 Subject: [PATCH] style: enable strict and type-checked lints on eslint Enables much stricter rules on eslint that also check types for increased protection. --- .eslintrc.cjs | 28 ++++++++++++++-- __mocks__/ably/index.ts | 15 +++++---- src/ChatApi.ts | 6 ++-- src/Message.ts | 8 +++-- src/Messages.ts | 42 +++++++++++++++++------- src/Occupancy.ts | 14 +++----- src/Presence.ts | 37 +++++++++++++++------- src/RoomReactions.ts | 15 ++++++--- src/Rooms.ts | 7 +++- src/SubscriptionManager.ts | 6 ++-- src/Typing.ts | 5 +-- src/logger.ts | 11 +++---- src/utils/EventEmitter.ts | 2 +- test/Messages.integration.test.ts | 10 +++--- test/Messages.test.ts | 44 ++++++++++++++------------ test/Occupancy.integration.test.ts | 29 ++++++++++------- test/Occupany.test.ts | 41 +++++++++++++----------- test/Presence.integration.test.ts | 16 ++++++---- test/RoomReactions.integration.test.ts | 10 +++--- test/RoomReactions.test.ts | 34 +++++++++++--------- test/SubscriptionManager.test.ts | 14 ++++---- test/Typing.integration.test.ts | 16 +++++----- test/Typing.test.ts | 19 ++++++----- test/helper/environment.ts | 2 +- test/helper/logger.ts | 10 +++++- test/helper/realtimeClient.ts | 27 ++++++++++++---- test/logger.test.ts | 4 +-- tsconfig.json | 3 ++ tsconfig.test.json | 9 ++++-- 29 files changed, 302 insertions(+), 182 deletions(-) diff --git a/.eslintrc.cjs b/.eslintrc.cjs index 6284fdb4..7ba432f6 100644 --- a/.eslintrc.cjs +++ b/.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: { diff --git a/__mocks__/ably/index.ts b/__mocks__/ably/index.ts index 1ffe917e..f6fa775b 100644 --- a/__mocks__/ably/index.ts +++ b/__mocks__/ably/index.ts @@ -2,7 +2,10 @@ import * as Ably from 'ably'; const MOCK_CLIENT_ID = 'MOCK_CLIENT_ID'; -const mockPromisify = (expectedReturnValue): Promise => new Promise((resolve) => resolve(expectedReturnValue)); +const mockPromisify = (expectedReturnValue): Promise => + new Promise((resolve) => { + resolve(expectedReturnValue); + }); const methodReturningVoidPromise = () => mockPromisify((() => {})()); 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 = {}; } } diff --git a/src/ChatApi.ts b/src/ChatApi.ts index 3d9305ec..59dd5946 100644 --- a/src/ChatApi.ts +++ b/src/ChatApi.ts @@ -78,10 +78,10 @@ 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; } - const [result] = response.items; + const [result] = response.items as RES[]; return result as RES; } @@ -98,7 +98,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; } diff --git a/src/Message.ts b/src/Message.ts index 4bb4d9e9..3b8f5fac 100644 --- a/src/Message.ts +++ b/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; diff --git a/src/Messages.ts b/src/Messages.ts index 675f568f..17bdb192 100644 --- a/src/Messages.ts +++ b/src/Messages.ts @@ -192,7 +192,7 @@ export class DefaultMessages extends EventEmitter 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 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 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), + ); } } diff --git a/src/Occupancy.ts b/src/Occupancy.ts index ebc75386..dc9563e9 100644 --- a/src/Occupancy.ts +++ b/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 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) + .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 implement this._logger.debug('Occupancy.unsubscribe(); removing internal listener'); return this._managedChannel.channel .setOptions({}) - .then(() => - this._managedChannel.unsubscribe(this._internalListener as Ably.messageCallback), - ) + .then(() => (this._internalListener ? this._managedChannel.unsubscribe(this._internalListener) : null)) .then(() => { this._internalListener = undefined; }); @@ -150,9 +148,7 @@ export class DefaultOccupancy extends EventEmitter 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); diff --git a/src/Presence.ts b/src/Presence.ts index 77a158e6..7088b037 100644 --- a/src/Presence.ts +++ b/src/Presence.ts @@ -18,9 +18,7 @@ interface PresenceEventsMap { /** * Type for PresenceData */ -export type PresenceData = { - [key: string]: unknown; -}; +export type PresenceData = Record; /** * Type for AblyPresenceData @@ -209,13 +207,27 @@ export class DefaultPresence extends EventEmitter implements async get(params?: Ably.RealtimePresenceParams): Promise { 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 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 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 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 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 implements 50000, 500, (error as Error).message, - ); + ) as unknown as Error; } }; } diff --git a/src/RoomReactions.ts b/src/RoomReactions.ts index fb666c4d..553927b0 100644 --- a/src/RoomReactions.ts +++ b/src/RoomReactions.ts @@ -33,6 +33,7 @@ export interface RoomReactions { * @param metadata Any JSON-serializable data that will be attached to the reaction. * @returns The returned promise resolves when the reaction was sent. Note that it is possible to receive your own reaction via the reactions listener before this promise resolves. */ + // eslint-disable-next-line @typescript-eslint/unified-signatures send(type: string, metadata?: unknown): Promise; /** @@ -150,7 +151,13 @@ export class DefaultRoomReactions extends EventEmitter im } parseNewReaction(inbound: Ably.InboundMessage, clientId: string): Reaction | undefined { - if (!inbound.data || !inbound.data.type || typeof inbound.data.type !== 'string') { + const data = inbound.data as ReactionPayload | undefined; + if (!data) { + this._logger.error('RoomReactions.realtimeMessageToReaction(); invalid reaction message with no data', inbound); + return; + } + + if (!data.type || typeof data.type !== 'string') { // not a reaction if there's no type or type is not a string this._logger.error('RoomReactions.realtimeMessageToReaction(); invalid reaction message with no type', inbound); return; @@ -166,11 +173,11 @@ export class DefaultRoomReactions extends EventEmitter im } return new DefaultReaction( - inbound.data.type, - inbound.clientId!, + data.type, + inbound.clientId, new Date(inbound.timestamp), inbound.clientId === clientId, - inbound.data.metadata, + data.metadata, ); } } diff --git a/src/Rooms.ts b/src/Rooms.ts index b21f380e..e91e76da 100644 --- a/src/Rooms.ts +++ b/src/Rooms.ts @@ -66,7 +66,8 @@ export class DefaultRooms implements Rooms { */ get(roomId: string): Room { this._logger.trace('Rooms.get();', { roomId }); - if (this.rooms[roomId]) return this.rooms[roomId]; + const existing = this.rooms[roomId]; + if (existing) return existing; const room = new DefaultRoom(roomId, this.realtime, this.chatApi, this._clientOptions, this._logger); this.rooms[roomId] = room; @@ -90,6 +91,10 @@ export class DefaultRooms implements Rooms { if (!room) { return; } + + // eslint-disable-next-line @typescript-eslint/no-dynamic-delete delete this.rooms[roomId]; + + return Promise.resolve(); } } diff --git a/src/SubscriptionManager.ts b/src/SubscriptionManager.ts index 8124f70c..453a9a10 100644 --- a/src/SubscriptionManager.ts +++ b/src/SubscriptionManager.ts @@ -40,7 +40,7 @@ export class DefaultSubscriptionManager implements SubscriptionManager { private readonly _channel: Ably.RealtimeChannel; private readonly _listeners: Set; private readonly _presenceListeners: Set; - private _presenceEntered: boolean = false; + private _presenceEntered = false; private _logger: Logger; constructor(channel: Ably.RealtimeChannel, logger: Logger) { @@ -56,7 +56,8 @@ export class DefaultSubscriptionManager implements SubscriptionManager { channel: this._channel.name, }); this._presenceEntered = false; - this.detachChannelIfNotListening().then(() => {}); + // eslint-disable-next-line @typescript-eslint/no-empty-function + void this.detachChannelIfNotListening().then(() => {}); } }); } @@ -170,6 +171,7 @@ export class DefaultSubscriptionManager implements SubscriptionManager { async presenceLeaveClient(clientId: string, data?: string): Promise { this._logger.trace('DefaultSubscriptionManager.presenceLeaveClient();', { clientId }); this._presenceEntered = false; + // eslint-disable-next-line @typescript-eslint/no-unsafe-return, @typescript-eslint/no-misused-promises return this._channel.presence.leaveClient(clientId, data).finally(() => { return this.detachChannelIfNotListening(); }); diff --git a/src/Typing.ts b/src/Typing.ts index 645db63e..933d5a9a 100644 --- a/src/Typing.ts +++ b/src/Typing.ts @@ -77,7 +77,7 @@ export interface Typing { /** * Represents a typing event. */ -export type TypingEvent = { +export interface TypingEvent { /** * A set of clientIds that are currently typing. */ @@ -97,7 +97,7 @@ export type TypingEvent = { */ isTyping: boolean; }; -}; +} /** * A listener which listens for typing events. @@ -160,6 +160,7 @@ export class DefaultTyping extends EventEmitter implements Typi */ private startTypingTimer(): void { this._logger.trace(`DefaultTyping.startTypingTimer();`); + // eslint-disable-next-line @typescript-eslint/no-misused-promises this._timerId = setTimeout(async () => { this._logger.debug(`DefaultTyping.startTypingTimer(); timeout expired`); await this.stop(); diff --git a/src/logger.ts b/src/logger.ts index 00bc948d..662f7c66 100644 --- a/src/logger.ts +++ b/src/logger.ts @@ -83,10 +83,8 @@ export enum LogLevel { * Represents the context of a log message. * It is an object of key-value pairs that can be used to provide additional context to a log message. */ -export interface LogContext { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - [key: string]: any; -} +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export type LogContext = Record; /** * A function that can be used to handle log messages. @@ -126,7 +124,7 @@ const consoleLogger = (message: string, level: LogLevel, context?: LogContext) = }; export const makeLogger = (options: NormalisedClientOptions): Logger => { - const logHandler = options.logHandler || consoleLogger; + const logHandler = options.logHandler ?? consoleLogger; return new DefaultLogger(logHandler, options.logLevel); }; @@ -168,7 +166,8 @@ class DefaultLogger implements Logger { throw new Error(`Invalid log level: ${level}`); } - this._levelNumber = logLevelNumberMap.get(level) as LogLevelNumbers; + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + this._levelNumber = logLevelNumberMap.get(level)!; } trace(message: string, context?: LogContext): void { diff --git a/src/utils/EventEmitter.ts b/src/utils/EventEmitter.ts index e4047f84..61dd3912 100644 --- a/src/utils/EventEmitter.ts +++ b/src/utils/EventEmitter.ts @@ -28,7 +28,7 @@ export function removeListener( let eventName: keyof T; for (let targetListenersIndex = 0; targetListenersIndex < targetListeners.length; targetListenersIndex++) { - listeners = targetListeners[targetListenersIndex]; + listeners = targetListeners[targetListenersIndex]!; if (isString(eventFilter) && isObject(listeners)) { listeners = listeners[eventFilter]; diff --git a/test/Messages.integration.test.ts b/test/Messages.integration.test.ts index f0bc580b..92bb760e 100644 --- a/test/Messages.integration.test.ts +++ b/test/Messages.integration.test.ts @@ -31,7 +31,7 @@ describe('messages integration', () => { context.chat = newChatClient(); }); - it('sets the agent version on the channel', async (context) => { + it('sets the agent version on the channel', (context) => { const { chat } = context; const roomName = Math.random().toString(36).substring(7); @@ -146,7 +146,7 @@ describe('messages integration', () => { // Do a history request to get the next 2 messages const history2 = await history1.next(); - expect(history2!.items).toEqual([ + expect(history2?.items).toEqual([ expect.objectContaining({ content: "Don't try it!", clientId: chat.clientId, @@ -155,7 +155,7 @@ describe('messages integration', () => { ]); // We shouldn't have a "next" link in the response - expect(history2!.hasNext()).toBe(false); + expect(history2?.hasNext()).toBe(false); }); it('should be able to paginate chat history, but backwards', async (context) => { @@ -196,7 +196,7 @@ describe('messages integration', () => { // Do a history request to get the next 2 messages const history2 = await history1.next(); - expect(history2!.items).toEqual([ + expect(history2?.items).toEqual([ expect.objectContaining({ content: 'Hello there!', clientId: chat.clientId, @@ -205,6 +205,6 @@ describe('messages integration', () => { ]); // We shouldn't have a "next" link in the response - expect(history2!.hasNext()).toBe(false); + expect(history2?.hasNext()).toBe(false); }); }); diff --git a/test/Messages.test.ts b/test/Messages.test.ts index 9e71c171..4da3b6aa 100644 --- a/test/Messages.test.ts +++ b/test/Messages.test.ts @@ -32,7 +32,7 @@ describe('Messages', () => { vi.spyOn(channel, 'subscribe').mockImplementation( // @ts-expect-error overriding mock async ( - eventsOrListeners: Array | Ably.messageCallback, + eventsOrListeners: string[] | Ably.messageCallback, listener: Ably.messageCallback, ) => { if (Array.isArray(eventsOrListeners)) { @@ -42,21 +42,25 @@ describe('Messages', () => { context.channelLevelListeners.set(listener, []); } context.emulateBackendPublish = (msg) => { - context.channelLevelListeners.forEach((_, cb) => cb(msg)); + context.channelLevelListeners.forEach((_, cb) => { + cb(msg); + }); }; + + return Promise.resolve(); }, ); vi.spyOn(channel, 'unsubscribe').mockImplementation( // @ts-expect-error overriding mock - async (listener: Ably.messageCallback) => { + (listener: Ably.messageCallback) => { context.channelLevelListeners.delete(listener); }, ); // Mock the attach vi.spyOn(channel, 'attach').mockImplementation(async () => { - return null; + return Promise.resolve(null); }); // Mock the detach @@ -113,8 +117,8 @@ describe('Messages', () => { roomId: room.roomId, }), ); - } catch (err) { - reject(err); + } catch (err: unknown) { + reject(err as Error); } done(); }) @@ -131,8 +135,8 @@ describe('Messages', () => { timestamp: publishTimestamp, }); }) - .catch((err) => { - reject(err); + .catch((err: unknown) => { + reject(err as Error); }); })); }); @@ -185,8 +189,8 @@ describe('Messages', () => { .then(() => { done(); }) - .catch((error: Ably.ErrorInfo) => { - reject(error); + .catch((error: unknown) => { + reject(error as Error); }); })); @@ -213,8 +217,8 @@ describe('Messages', () => { .then(() => { done(); }) - .catch((error: Ably.ErrorInfo) => { - reject(error); + .catch((error: unknown) => { + reject(error as Error); }); })); @@ -239,8 +243,8 @@ describe('Messages', () => { .then(() => { done(); }) - .catch((error: Ably.ErrorInfo) => { - reject(error); + .catch((error: unknown) => { + reject(error as Error); }); })); @@ -266,8 +270,8 @@ describe('Messages', () => { .then(() => { done(); }) - .catch((error: Ably.ErrorInfo) => { - reject(error); + .catch((error: unknown) => { + reject(error as Error); }); })); @@ -293,8 +297,8 @@ describe('Messages', () => { .then(() => { done(); }) - .catch((error: Ably.ErrorInfo) => { - reject(error); + .catch((error: unknown) => { + reject(error as Error); }); })); @@ -320,8 +324,8 @@ describe('Messages', () => { .then(() => { done(); }) - .catch((error: Ably.ErrorInfo) => { - reject(error); + .catch((error: unknown) => { + reject(error as Error); }); })); }); diff --git a/test/Occupancy.integration.test.ts b/test/Occupancy.integration.test.ts index 6ba2ec8f..5b4e65e7 100644 --- a/test/Occupancy.integration.test.ts +++ b/test/Occupancy.integration.test.ts @@ -16,21 +16,28 @@ const TEST_TIMEOUT = 20000; // Wait for the occupancy of a room to reach the expected occupancy. // Do this with a 10s timeout. const waitForExpectedInstantaneousOccupancy = (room: Room, expectedOccupancy: OccupancyEvent) => { - return new Promise((resolve) => { - const interval = setInterval(async () => { - const occupancy = await room.occupancy.get(); - if ( - occupancy.connections === expectedOccupancy.connections && - occupancy.presenceMembers === expectedOccupancy.presenceMembers - ) { - clearInterval(interval); - resolve(); - } + return new Promise((resolve, reject) => { + const interval = setInterval(() => { + room.occupancy + .get() + .then((occupancy) => { + if ( + occupancy.connections === expectedOccupancy.connections && + occupancy.presenceMembers === expectedOccupancy.presenceMembers + ) { + clearInterval(interval); + resolve(); + } + }) + .catch((err: unknown) => { + clearInterval(interval); + reject(err as Error); + }); }, 1000); setTimeout(() => { clearInterval(interval); - resolve(); + reject(new Error('Timed out waiting for occupancy')); }, TEST_TIMEOUT); }); }; diff --git a/test/Occupany.test.ts b/test/Occupany.test.ts index bc0cc04c..35e6971a 100644 --- a/test/Occupany.test.ts +++ b/test/Occupany.test.ts @@ -49,19 +49,21 @@ describe('Occupancy', () => { } else { context.channelLevelListeners.set(nameOrListener as Ably.messageCallback, []); } + + return Promise.resolve(); }, ); vi.spyOn(channel, 'unsubscribe').mockImplementation( // @ts-expect-error overriding mock - async (listener: Ably.messageCallback) => { + (listener: Ably.messageCallback) => { context.channelLevelListeners.delete(listener); }, ); // Mock the attach vi.spyOn(channel, 'attach').mockImplementation(async () => { - return null; + return Promise.resolve(null); }); // Mock the detach @@ -70,6 +72,7 @@ describe('Occupancy', () => { // Mock the setOptions vi.spyOn(channel, 'setOptions').mockImplementation(async (options: Ably.ChannelOptions) => { context.currentChannelOptions = options; + return Promise.resolve(); }); }); @@ -133,8 +136,8 @@ describe('Occupancy', () => { presenceMembers: 3, }); done(); - } catch (err) { - reject(err); + } catch (err: unknown) { + reject(err as Error); } }) .then(() => { @@ -148,8 +151,8 @@ describe('Occupancy', () => { }, }); }) - .catch((err) => { - reject(err); + .catch((err: unknown) => { + reject(err as Error); }); })); @@ -164,8 +167,8 @@ describe('Occupancy', () => { presenceMembers: 0, }); done(); - } catch (err) { - reject(err); + } catch (err: unknown) { + reject(err as Error); } }) .then(() => { @@ -179,8 +182,8 @@ describe('Occupancy', () => { }, }); }) - .catch((err) => { - reject(err); + .catch((err: unknown) => { + reject(err as Error); }); })); @@ -189,7 +192,7 @@ describe('Occupancy', () => { const room = makeRoom(context); room.occupancy .subscribe(() => { - reject('should not have received occupancy event without connections'); + reject(new Error('should not have received occupancy event without connections')); }) .then(() => { context.emulateOccupancyUpdate({ @@ -204,8 +207,8 @@ describe('Occupancy', () => { .then(() => { done(); }) - .catch((error: Ably.ErrorInfo) => { - reject(error); + .catch((error: unknown) => { + reject(error as Error); }); })); @@ -214,7 +217,7 @@ describe('Occupancy', () => { const room = makeRoom(context); room.occupancy .subscribe(() => { - reject('should not have received occupancy event without presenceMembers'); + reject(new Error('should not have received occupancy event without presenceMembers')); }) .then(() => { context.emulateOccupancyUpdate({ @@ -229,8 +232,8 @@ describe('Occupancy', () => { .then(() => { done(); }) - .catch((error: Ably.ErrorInfo) => { - reject(error); + .catch((error: unknown) => { + reject(error as Error); }); })); @@ -239,7 +242,7 @@ describe('Occupancy', () => { const room = makeRoom(context); room.occupancy .subscribe(() => { - reject('should not have received occupancy event without metrics'); + reject(new Error('should not have received occupancy event without metrics')); }) .then(() => { context.emulateOccupancyUpdate({ @@ -250,8 +253,8 @@ describe('Occupancy', () => { .then(() => { done(); }) - .catch((error: Ably.ErrorInfo) => { - reject(error); + .catch((error: unknown) => { + reject(error as Error); }); })); }); diff --git a/test/Presence.integration.test.ts b/test/Presence.integration.test.ts index 7747b8c7..b8e330b6 100644 --- a/test/Presence.integration.test.ts +++ b/test/Presence.integration.test.ts @@ -46,7 +46,7 @@ const waitForPresenceEvent = async ( describe('UserPresence', { timeout: 10000 }, () => { // Setup before each test, create a new Ably Realtime client and a new Room - beforeEach(async (context) => { + beforeEach((context) => { context.realtime = ablyRealtimeClient(); const roomId = randomRoomId(); context.chat = newChatClient(undefined, context.realtime); @@ -61,12 +61,16 @@ describe('UserPresence', { timeout: 10000 }, () => { realtimeChannelName: string, expectationFn: (member: Ably.PresenceMessage) => void, ) { - return new Promise((resolve) => { + return new Promise((resolve, reject) => { const presence = realtimeClient.channels.get(realtimeChannelName).presence; - presence.subscribe(event, (member) => { - expectationFn(member); - resolve(); - }); + presence + .subscribe(event, (member) => { + expectationFn(member); + resolve(); + }) + .catch((err: unknown) => { + reject(err as Error); + }); }); } diff --git a/test/RoomReactions.integration.test.ts b/test/RoomReactions.integration.test.ts index a109b17e..aa34ba17 100644 --- a/test/RoomReactions.integration.test.ts +++ b/test/RoomReactions.integration.test.ts @@ -16,16 +16,14 @@ const waitForReactions = (foundTypes: string[], expectedTypes: string[]) => { const interval = setInterval(() => { if (foundTypes.length === expectedTypes.length) { clearInterval(interval); - if (timeout) { - clearTimeout(timeout); - } + clearTimeout(timeout); foundTypes.forEach((foundType, idx) => { const expectedType = expectedTypes[idx]; try { expect(foundType).toEqual(expectedType); - } catch (err) { - reject(err); + } catch (err: unknown) { + reject(err as Error); return; } }); @@ -45,7 +43,7 @@ describe('room-level reactions integration test', () => { context.chat = newChatClient(); }); - it('sets the agent version on the channel', async (context) => { + it('sets the agent version on the channel', (context) => { const { chat } = context; const roomName = Math.random().toString(36).substring(7); diff --git a/test/RoomReactions.test.ts b/test/RoomReactions.test.ts index 8a9d7456..0954a311 100644 --- a/test/RoomReactions.test.ts +++ b/test/RoomReactions.test.ts @@ -43,8 +43,12 @@ describe('Reactions', () => { } context.emulateBackendPublish = (msg) => { - listeners.forEach((listener) => listener(msg)); + listeners.forEach((listener) => { + listener(msg); + }); }; + + return Promise.resolve(); }, ); vi.spyOn(channel, 'publish').mockImplementation( @@ -80,8 +84,8 @@ describe('Reactions', () => { type: 'like', }), ); - } catch (err) { - reject(err); + } catch (err: unknown) { + reject(err as Error); } done(); }) @@ -95,8 +99,8 @@ describe('Reactions', () => { timestamp: publishTimestamp, }); }) - .catch((err) => { - reject(err); + .catch((err: unknown) => { + reject(err as Error); }); })); @@ -117,8 +121,8 @@ describe('Reactions', () => { type: 'hate', }), ); - } catch (err) { - reject(err); + } catch (err: unknown) { + reject(err as Error); } done(); }) @@ -132,8 +136,8 @@ describe('Reactions', () => { timestamp: publishTimestamp, }); }) - .catch((err) => { - reject(err); + .catch((err: unknown) => { + reject(err as Error); }); })); }); @@ -162,8 +166,8 @@ describe('Reactions', () => { .then(() => { done(); }) - .catch((err) => { - reject(err); + .catch((err: unknown) => { + reject(err as Error); }); }), ); @@ -186,16 +190,16 @@ describe('Reactions', () => { type: 'love', }), ); - } catch (err) { - reject(err); + } catch (err: unknown) { + reject(err as Error); } done(); }) .then(() => { return room.reactions.send('love'); }) - .catch((err) => { - reject(err); + .catch((err: unknown) => { + reject(err as Error); }); })); }); diff --git a/test/SubscriptionManager.test.ts b/test/SubscriptionManager.test.ts index 1a28bbf2..f3092b75 100644 --- a/test/SubscriptionManager.test.ts +++ b/test/SubscriptionManager.test.ts @@ -109,7 +109,7 @@ describe('subscription manager', { timeout: 15000 }, () => { await waitForMessages(receivedMessages, 1); expect(receivedMessages.length).toBe(1); - expect(receivedMessages[0].data).toBe('test-message'); + expect(receivedMessages[0]?.data).toBe('test-message'); }); it('subscribes to channel with implicit attach on all events', async (context) => { @@ -130,8 +130,8 @@ describe('subscription manager', { timeout: 15000 }, () => { await waitForMessages(receivedMessages, 2); expect(receivedMessages.length).toBe(2); - expect(receivedMessages[0].data).toBe('test-message'); - expect(receivedMessages[1].data).toBe('another-message'); + expect(receivedMessages[0]?.data).toBe('test-message'); + expect(receivedMessages[1]?.data).toBe('another-message'); }); it('subscribes to channel with implicit attach on presence all events', async (context) => { @@ -171,8 +171,8 @@ describe('subscription manager', { timeout: 15000 }, () => { // Wait for the message to be received in the receivedMessages await waitForMessages(receivedMessages, 1); expect(receivedMessages.length).toBe(1); - expect(receivedMessages[0].action).toBe('update'); - expect(receivedMessages[0].data).toBe('test-message-2'); + expect(receivedMessages[0]?.action).toBe('update'); + expect(receivedMessages[0]?.data).toBe('test-message-2'); }); it('unsubscribes from channel with implicit detach if last messages listener', async (context) => { @@ -285,8 +285,8 @@ describe('subscription manager', { timeout: 15000 }, () => { await context.subscriptionManager.presenceEnterClient(context.defaultClientId, 'test-data'); // should receive one enter event await waitForMessages(receivedMessages, 1); - expect(receivedMessages[0].action).toBe('enter'); - expect(receivedMessages[0].data).toBe('test-data'); + expect(receivedMessages[0]?.action).toBe('enter'); + expect(receivedMessages[0]?.data).toBe('test-data'); }); it('should attach to the channel when updating presence', async (context) => { const { channel, subscriptionManager } = context; diff --git a/test/Typing.integration.test.ts b/test/Typing.integration.test.ts index 791b48e5..33277885 100644 --- a/test/Typing.integration.test.ts +++ b/test/Typing.integration.test.ts @@ -64,11 +64,11 @@ describe('Typing', () => { // Once the timout timer expires, the typingStopped event should be emitted await waitForMessages(events, 2); // Should have received a typingStarted and then typingStopped event - expect(events[0].change.clientId, 'client ids should match').toEqual(context.clientId); - expect(events[0].change.isTyping, 'isTyping should be true').toEqual(true); + expect(events[0]?.change.clientId, 'client ids should match').toEqual(context.clientId); + expect(events[0]?.change.isTyping, 'isTyping should be true').toEqual(true); // Wait for the typing timeout to expire and the stop typing event to be received - expect(events[1].change.clientId, 'client ids should match').toEqual(context.clientId); - expect(events[1].change.isTyping, 'isTyping should be false').toEqual(false); + expect(events[1]?.change.clientId, 'client ids should match').toEqual(context.clientId); + expect(events[1]?.change.isTyping, 'isTyping should be false').toEqual(false); }, TEST_TIMEOUT, ); @@ -88,12 +88,12 @@ describe('Typing', () => { expect(events.length, 'typingStopped event should have been received').toEqual(2); // First event should be typingStarted - expect(events[0].currentlyTypingClientIds.has(context.clientId)).toEqual(true); - expect(events[0].change.isTyping, 'first event should be typingStarted').toEqual(true); + expect(events[0]?.currentlyTypingClientIds.has(context.clientId)).toEqual(true); + expect(events[0]?.change.isTyping, 'first event should be typingStarted').toEqual(true); // Last event should be typingStopped - expect(events[1].change.isTyping, 'second event should be typingStopped').toEqual(false); - expect(events[1].currentlyTypingClientIds.has(context.clientId)).toEqual(false); + expect(events[1]?.change.isTyping, 'second event should be typingStopped').toEqual(false); + expect(events[1]?.currentlyTypingClientIds.has(context.clientId)).toEqual(false); }, TEST_TIMEOUT, ); diff --git a/test/Typing.test.ts b/test/Typing.test.ts index 3485ef04..f07ed175 100644 --- a/test/Typing.test.ts +++ b/test/Typing.test.ts @@ -36,25 +36,24 @@ describe('Typing', () => { context.channelLevelListeners.add(listener); context.emulateBackendPublish = (msg) => { - context.channelLevelListeners.forEach((_, cb) => cb(msg)); + context.channelLevelListeners.forEach((_, cb) => { + cb(msg); + }); }; + + return Promise.resolve(); }, ); vi.spyOn(presence, 'unsubscribe').mockImplementation( // @ts-expect-error overriding mock - async (listener: Ably.messageCallback) => { + (listener: Ably.messageCallback) => { context.channelLevelListeners.delete(listener); }, ); // Mock the attach - vi.spyOn(channel, 'attach').mockImplementation(async () => { - return null; - }); - - // Mock the detach - vi.spyOn(channel, 'detach').mockImplementation(async () => {}); + vi.spyOn(channel, 'attach').mockImplementation(() => Promise.resolve(null)); }); it('delays stop timeout while still typing', async (context) => { @@ -124,8 +123,8 @@ describe('Typing', () => { .then(() => { done(); }) - .catch((err) => { - reject(err); + .catch((err: unknown) => { + reject(err as Error); }); }); diff --git a/test/helper/environment.ts b/test/helper/environment.ts index caa8ea24..7021fba3 100644 --- a/test/helper/environment.ts +++ b/test/helper/environment.ts @@ -3,7 +3,7 @@ export const isNonSandboxEnvironment = () => { }; export const testEnvironment = () => { - return process.env.VITE_ABLY_ENV || 'sandbox'; + return process.env.VITE_ABLY_ENV ?? 'sandbox'; }; export const isLocalEnvironment = () => { diff --git a/test/helper/logger.ts b/test/helper/logger.ts index 173b814e..53667a36 100644 --- a/test/helper/logger.ts +++ b/test/helper/logger.ts @@ -11,4 +11,12 @@ export const makeTestLogger = (): Logger => { }; // testLoggingLevel returns the log level specified by the VITE_TEST_LOG_LEVEL environment variable. -export const testLoggingLevel = (): LogLevel => (process.env.VITE_TEST_LOG_LEVEL as LogLevel) ?? LogLevel.silent; +export const testLoggingLevel = (): LogLevel => { + const level = process.env.VITE_TEST_LOG_LEVEL; + + if (level) { + return level as LogLevel; + } + + return LogLevel.silent; +}; diff --git a/test/helper/realtimeClient.ts b/test/helper/realtimeClient.ts index f99b7155..21faa532 100644 --- a/test/helper/realtimeClient.ts +++ b/test/helper/realtimeClient.ts @@ -5,14 +5,18 @@ import { ablyApiKey, isLocalEnvironment, testEnvironment } from './environment.j import { randomClientId } from './identifier.js'; const baseOptions = (options?: Ably.ClientOptions): Ably.ClientOptions => { - options = options || {}; - options.clientId = options.clientId || randomClientId(); - options.environment = options.environment || testEnvironment(); - options.key = options.key || ablyApiKey(); + options = options ?? {}; + options.clientId = options.clientId ?? randomClientId(); + options.environment = options.environment ?? testEnvironment(); + options.key = options.key ?? ablyApiKey(); // TODO: Support non-JSON protocol options.useBinaryProtocol = false; - options.logHandler = options.logHandler || ((msg) => console.error(msg)); - options.logLevel = options.logLevel || 1; // error + options.logHandler = + options.logHandler ?? + ((msg) => { + console.error(msg); + }); + options.logLevel = options.logLevel ?? 1; // error if (isLocalEnvironment()) { options.port = 8081; @@ -31,7 +35,16 @@ const ablyRealtimeClient = (options?: Ably.ClientOptions): Ably.Realtime => { // At the moment, chat doesn't support keys for authentication, so create a client that uses tokens const ablyRealtimeClientWithToken = (options?: Ably.ClientOptions): Ably.Realtime => { options = baseOptions(options); - const [keyId, keySecret] = options!.key!.split(':'); + + if (!options.key) { + throw new Error('key must be provided when using tokens'); + } + + const [keyId, keySecret] = options.key.split(':'); + if (!keyId || !keySecret) { + throw new Error('key must be in the format "keyId:key'); + } + options.useTokenAuth = true; // Generate the token diff --git a/test/logger.test.ts b/test/logger.test.ts index 0bf64e06..702a09ff 100644 --- a/test/logger.test.ts +++ b/test/logger.test.ts @@ -39,7 +39,7 @@ describe('logger', () => { const logger = makeLogger(options); callMethodForLevel(logger, logLevel, logContext); - reject('Expected logHandler to be called'); + reject(new Error('Expected logHandler to be called')); }), ); @@ -66,7 +66,7 @@ describe('logger', () => { const options = normaliseClientOptions({ logLevel: configuredLevel, logHandler: () => { - reject('Expected logHandler to not be called'); + reject(new Error('Expected logHandler to not be called')); }, }); diff --git a/tsconfig.json b/tsconfig.json index 85166cb5..4951f89b 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -3,6 +3,7 @@ "./src/**/*.ts" ], "exclude": [ + "./src/utils/**/*.ts", "./test/**/*.test.tsx", "./test/**/*.test.ts", "./src/fakes/**/*.ts" @@ -21,6 +22,8 @@ "allowJs": true, "allowSyntheticDefaultImports": true, "resolveJsonModule": true, + "strictNullChecks": true, + "noUncheckedIndexedAccess": true, "lib": [ "DOM", "DOM.Iterable", diff --git a/tsconfig.test.json b/tsconfig.test.json index 3187837c..a80ab1b5 100644 --- a/tsconfig.test.json +++ b/tsconfig.test.json @@ -1,10 +1,13 @@ { "include": [ "test/**/*.spec.ts", - "test/**/*.test.ts" + "test/**/*.test.ts", + "test/helper/**/*.ts", ], "exclude": [ - "test/utils/**/*.ts" + "src/**/*.ts", + "test/utils/**/*.ts", + "ably-common/**/*", ], "compilerOptions": { "noEmit": true, @@ -21,6 +24,8 @@ "allowSyntheticDefaultImports": true, "resolveJsonModule": true, "allowImportingTsExtensions": true, + "strictNullChecks": true, + "noUncheckedIndexedAccess": true, "lib": [ "DOM", "DOM.Iterable",