Skip to content

[MMB-156] Prepare for adding documentation comments #197

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 18 commits into from
Oct 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
473ca77
Remove unused type `Provider`
lawrence-forooghian Sep 27, 2023
8295f04
Make naming of event map types consistent
lawrence-forooghian Sep 20, 2023
9f9ff98
Mark non-public API where we haven’t already
lawrence-forooghian Sep 20, 2023
9dc816c
Export all types that are referenced by exported types
lawrence-forooghian Sep 20, 2023
3b78093
Extract a couple of events to their own types
lawrence-forooghian Sep 21, 2023
38f153e
Get rid of EventMap and EventKey types
lawrence-forooghian Sep 27, 2023
705ea95
Improve typing of callListener function
lawrence-forooghian Sep 27, 2023
28067d5
Convert *EventMap types to interfaces
lawrence-forooghian Sep 21, 2023
bd7f5d7
Split LockStatus’s allowed values into separate types
lawrence-forooghian Sep 26, 2023
97f5ac2
Create overload signatures for event-related methods
lawrence-forooghian Sep 27, 2023
4979610
Add `this` type parameter to EventListener
lawrence-forooghian Sep 28, 2023
9da57ad
Create type for `EventListener`’s `this`
lawrence-forooghian Oct 2, 2023
cc7d295
Create overload signatures for Space.prototype.updateProfileData
lawrence-forooghian Oct 2, 2023
440941f
Rename type param in Subset type
lawrence-forooghian Oct 2, 2023
19ac73f
Extract return value of Space.prototype.getState to its own type
lawrence-forooghian Oct 2, 2023
57be83d
Extract type of updateProfileData’s function param
lawrence-forooghian Oct 3, 2023
c27c748
Fix ably-js links in class-definitions.md
lawrence-forooghian Oct 3, 2023
f053f80
Remove types from class-definitions.md that don’t exist in codebase
lawrence-forooghian Sep 20, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 14 additions & 40 deletions docs/class-definitions.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,15 @@ Refer to the [Ably docs for the JS SDK](https://ably.com/docs/getting-started/se

#### client

Instance of the [Ably-JS](https://github.com/ably/ably-js#introduction) client that was passed to the [constructor](#constructor).
Instance of the [ably-js](https://github.com/ably/ably-js) client that was passed to the [constructor](#constructor).

```ts
type client = Ably.RealtimePromise;
```

#### connection

Instance of the [Ably-JS](https://github.com/ably/ably-js#introduction) connection, belonging to the client that was passed to the [constructor](#constructor).
Instance of the [ably-js](https://github.com/ably/ably-js) connection, belonging to the client that was passed to the [constructor](#constructor).

```ts
type connection = Ably.ConnectionPromise;
Expand Down Expand Up @@ -296,8 +296,11 @@ type SpaceMember = {
connectionId: string;
isConnected: boolean;
profileData: Record<string, unknown>;
location: Location;
lastEvent: PresenceEvent;
location: unknown;
lastEvent: {
name: Types.PresenceAction;
timestamp: number;
};
};
```

Expand Down Expand Up @@ -325,15 +328,6 @@ The current location of the user within the space.

The most recent event emitted by [presence](https://ably.com/docs/presence-occupancy/presence?lang=javascript) and its timestamp. Events will be either `enter`, `leave`, `update` or `present`.

#### PresenceEvent

```ts
type PresenceEvent = {
name: 'enter' | 'leave' | 'update' | 'present';
timestamp: number;
};
```

## Member Locations

Handles the tracking of member locations within a space. Inherits from [EventEmitter](/docs/usage.md#event-emitters).
Expand All @@ -348,18 +342,18 @@ Available events:

- ##### **update**

Fires when a member updates their location. The argument supplied to the event listener is a [LocationUpdate](#locationupdate-1).
Fires when a member updates their location. The argument supplied to the event listener is an UpdateEvent.

```ts
space.locations.subscribe('update', (locationUpdate: LocationUpdate) => {});
space.locations.subscribe('update', (locationUpdate: LocationsEvents.UpdateEvent) => {});
```

#### set()

Set your current location. [Location](#location-1) can be any JSON-serializable object. Emits a [locationUpdate](#locationupdate) event to all connected clients in this space.
Set your current location. The `location` argument can be any JSON-serializable object. Emits a `locationUpdate` event to all connected clients in this space.

```ts
type set = (update: Location) => Promise<void>;
type set = (location: unknown) => Promise<void>;
```

#### unsubscribe()
Expand All @@ -375,7 +369,7 @@ space.locations.unsubscribe('update');
Get location for self.

```ts
type getSelf = () => Promise<Location>;
type getSelf = () => Promise<unknown>;
```

Example:
Expand All @@ -389,7 +383,7 @@ const myLocation = await space.locations.getSelf();
Get location for all members.

```ts
type getAll = () => Promise<Record<ConnectionId, Location>>;
type getAll = () => Promise<Record<ConnectionId, unknown>>;
```

Example:
Expand All @@ -403,7 +397,7 @@ const allLocations = await space.locations.getAll();
Get location for other members

```ts
type getOthers = () => Promise<Record<ConnectionId, Location>>;
type getOthers = () => Promise<Record<ConnectionId, unknown>>;
```

Example:
Expand All @@ -414,26 +408,6 @@ const otherLocations = await space.locations.getOthers()

### Related types

#### Location

Represents a location in an application.

```ts
type Location = string | Record<string, unknown> | null;
```

#### LocationUpdate

Represents a change between locations for a given [`SpaceMember`](#spacemember).

```ts
type LocationUpdate = {
member: SpaceMember;
currentLocation: Location;
previousLocation: Location;
};
```

## Live Cursors

Handles tracking of member cursors within a space. Inherits from [EventEmitter](/docs/usage.md#event-emitters).
Expand Down
38 changes: 22 additions & 16 deletions src/Cursors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,17 @@ import { Types } from 'ably';
import Space from './Space.js';
import CursorBatching from './CursorBatching.js';
import CursorDispensing from './CursorDispensing.js';
import EventEmitter, {
InvalidArgumentError,
inspect,
type EventKey,
type EventListener,
} from './utilities/EventEmitter.js';
import EventEmitter, { InvalidArgumentError, inspect, type EventListener } from './utilities/EventEmitter.js';
import CursorHistory from './CursorHistory.js';
import { CURSOR_UPDATE } from './CursorConstants.js';

import type { CursorsOptions, CursorUpdate } from './types.js';
import type { RealtimeMessage } from './utilities/types.js';
import { ERR_NOT_ENTERED_SPACE } from './Errors.js';

type CursorsEventMap = {
export interface CursorsEventMap {
update: CursorUpdate;
};
}

const CURSORS_CHANNEL_TAG = '::$cursors';

Expand All @@ -27,10 +22,11 @@ export default class Cursors extends EventEmitter<CursorsEventMap> {
private readonly cursorDispensing: CursorDispensing;
private readonly cursorHistory: CursorHistory;
readonly options: CursorsOptions;
readonly channelName: string;
private readonly channelName: string;

public channel?: Types.RealtimeChannelPromise;

/** @internal */
constructor(private space: Space) {
super();

Expand Down Expand Up @@ -90,7 +86,7 @@ export default class Cursors extends EventEmitter<CursorsEventMap> {
return !this.emitterHasListeners(subscriptions);
}

private emitterHasListeners = (emitter: EventEmitter<{}>) => {
private emitterHasListeners = <T>(emitter: EventEmitter<T>) => {
const flattenEvents = (obj: Record<string, Function[]>) =>
Object.entries(obj)
.map((_, v) => v)
Expand All @@ -104,9 +100,14 @@ export default class Cursors extends EventEmitter<CursorsEventMap> {
);
};

subscribe<K extends EventKey<CursorsEventMap>>(
listenerOrEvents?: K | K[] | EventListener<CursorsEventMap[K]>,
listener?: EventListener<CursorsEventMap[K]>,
subscribe<K extends keyof CursorsEventMap>(
eventOrEvents: K | K[],
listener?: EventListener<CursorsEventMap, K>,
): void;
subscribe(listener?: EventListener<CursorsEventMap, keyof CursorsEventMap>): void;
subscribe<K extends keyof CursorsEventMap>(
listenerOrEvents?: K | K[] | EventListener<CursorsEventMap, K>,
listener?: EventListener<CursorsEventMap, K>,
) {
try {
super.on(listenerOrEvents, listener);
Expand All @@ -129,9 +130,14 @@ export default class Cursors extends EventEmitter<CursorsEventMap> {
}
}

unsubscribe<K extends EventKey<CursorsEventMap>>(
listenerOrEvents?: K | K[] | EventListener<CursorsEventMap[K]>,
listener?: EventListener<CursorsEventMap[K]>,
unsubscribe<K extends keyof CursorsEventMap>(
eventOrEvents: K | K[],
listener?: EventListener<CursorsEventMap, K>,
): void;
unsubscribe(listener?: EventListener<CursorsEventMap, keyof CursorsEventMap>): void;
unsubscribe<K extends keyof CursorsEventMap>(
listenerOrEvents?: K | K[] | EventListener<CursorsEventMap, K>,
listener?: EventListener<CursorsEventMap, K>,
) {
try {
super.off(listenerOrEvents, listener);
Expand Down
45 changes: 30 additions & 15 deletions src/Locations.ts
Original file line number Diff line number Diff line change
@@ -1,27 +1,32 @@
import EventEmitter, {
InvalidArgumentError,
inspect,
type EventKey,
type EventListener,
} from './utilities/EventEmitter.js';
import EventEmitter, { InvalidArgumentError, inspect, type EventListener } from './utilities/EventEmitter.js';

import type { SpaceMember } from './types.js';
import type { PresenceMember } from './utilities/types.js';
import type Space from './Space.js';
import { ERR_NOT_ENTERED_SPACE } from './Errors.js';
import SpaceUpdate from './SpaceUpdate.js';

type LocationsEventMap = {
update: { member: SpaceMember; currentLocation: unknown; previousLocation: unknown };
};
export namespace LocationsEvents {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer that we do not use namespaces if possible. Namespaces, and enums, are two features of TS that break it's main advantage - of adding just types, not features (there is no namespaces or enums in JS). Could we use a module with named exports for this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind explaining your objection to my use of namespaces in this case, given that I'm using them as a way to organise types?

I am also happy to change it, but I don't think I understand what you mean by using a module, would you mind showing me an example?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey @lawrence-forooghian thought about this and I think it's ok to use them just for types.

My objection for using namespaces is that they do not exist in JavaScript and so if you add code into them they will have to compile into something else. They were deprecated at some point in TS but I think that decision was reversed (because of how many people use them maybe). Apart from them not existing in JavaScript, the main issue is they duplicate what modules do (ie. organise code, namespace methods and properties).

export interface UpdateEvent {
member: SpaceMember;
currentLocation: unknown;
previousLocation: unknown;
}
}

export interface LocationsEventMap {
update: LocationsEvents.UpdateEvent;
}

export default class Locations extends EventEmitter<LocationsEventMap> {
private lastLocationUpdate: Record<string, PresenceMember['data']['locationUpdate']['id']> = {};

/** @internal */
constructor(private space: Space, private presenceUpdate: Space['presenceUpdate']) {
super();
}

/** @internal */
async processPresenceMessage(message: PresenceMember) {
// Only an update action is currently a valid location update.
if (message.action !== 'update') return;
Expand Down Expand Up @@ -61,9 +66,14 @@ export default class Locations extends EventEmitter<LocationsEventMap> {
await this.presenceUpdate(update.updateLocation(location));
}

subscribe<K extends EventKey<LocationsEventMap>>(
listenerOrEvents?: K | K[] | EventListener<LocationsEventMap[K]>,
listener?: EventListener<LocationsEventMap[K]>,
subscribe<K extends keyof LocationsEventMap>(
eventOrEvents: K | K[],
listener?: EventListener<LocationsEventMap, K>,
): void;
subscribe(listener?: EventListener<LocationsEventMap, keyof LocationsEventMap>): void;
subscribe<K extends keyof LocationsEventMap>(
listenerOrEvents?: K | K[] | EventListener<LocationsEventMap, K>,
listener?: EventListener<LocationsEventMap, K>,
) {
try {
super.on(listenerOrEvents, listener);
Expand All @@ -78,9 +88,14 @@ export default class Locations extends EventEmitter<LocationsEventMap> {
}
}

unsubscribe<K extends EventKey<LocationsEventMap>>(
listenerOrEvents?: K | K[] | EventListener<LocationsEventMap[K]>,
listener?: EventListener<LocationsEventMap[K]>,
unsubscribe<K extends keyof LocationsEventMap>(
eventOrEvents: K | K[],
listener?: EventListener<LocationsEventMap, K>,
): void;
unsubscribe(listener?: EventListener<LocationsEventMap, keyof LocationsEventMap>): void;
unsubscribe<K extends keyof LocationsEventMap>(
listenerOrEvents?: K | K[] | EventListener<LocationsEventMap, K>,
listener?: EventListener<LocationsEventMap, K>,
) {
try {
super.off(listenerOrEvents, listener);
Expand Down
Loading