Skip to content

Commit

Permalink
style: enable strict and type-checked lints on eslint
Browse files Browse the repository at this point in the history
Enables much stricter rules on eslint that also check types for increased
protection.
  • Loading branch information
AndyTWF committed Jun 26, 2024
1 parent c4b4b1a commit ac6ec14
Show file tree
Hide file tree
Showing 29 changed files with 302 additions and 182 deletions.
28 changes: 26 additions & 2 deletions .eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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',
],
Expand All @@ -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
Expand All @@ -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: {
Expand Down
15 changes: 9 additions & 6 deletions __mocks__/ably/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -18,8 +21,8 @@ function createMockPresence() {
fn();
},
},
subscribe: () => {},
unsubscribe: () => {},
subscribe: methodReturningVoidPromise,
unsubscribe: methodReturningVoidPromise,
};
}

Expand All @@ -37,8 +40,8 @@ function createMockChannel() {
attach: methodReturningVoidPromise,
detach: methodReturningVoidPromise,
presence: createMockPresence(),
subscribe: () => {},
unsubscribe: () => {},
subscribe: methodReturningVoidPromise,
unsubscribe: methodReturningVoidPromise,
on: () => {},
off: () => {},
publish: () => {},
Expand Down Expand Up @@ -79,7 +82,7 @@ class MockRealtime {
state: 'connected',
};

this['options'] = {};
this.options = {};
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/ChatApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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;
}
Expand Down
8 changes: 6 additions & 2 deletions src/Message.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
42 changes: 30 additions & 12 deletions src/Messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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) {
Expand Down Expand Up @@ -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),
);
}
}
14 changes: 5 additions & 9 deletions src/Occupancy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand All @@ -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.
Expand Down Expand Up @@ -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;
Expand All @@ -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;
});
Expand Down Expand Up @@ -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);
Expand Down
37 changes: 25 additions & 12 deletions src/Presence.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@ interface PresenceEventsMap {
/**
* Type for PresenceData
*/
export type PresenceData = {
[key: string]: unknown;
};
export type PresenceData = Record<string, unknown>;

/**
* Type for AblyPresenceData
Expand Down Expand Up @@ -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,
}));
Expand Down Expand Up @@ -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) {
Expand All @@ -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();
});
}

/**
Expand All @@ -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);
Expand All @@ -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,
Expand All @@ -358,7 +371,7 @@ export class DefaultPresence extends EventEmitter<PresenceEventsMap> implements
50000,
500,
(error as Error).message,
);
) as unknown as Error;
}
};
}
Loading

0 comments on commit ac6ec14

Please sign in to comment.