[MMB-118] Previous location not set to null for first update

package.json

@@ -12,6 +12,7 @@
     "format": "prettier --write --ignore-path .gitignore src demo",
     "format:check": "prettier --check --ignore-path .gitignore src demo",
     "test": "vitest run",
+    "watch": "vitest watch",
     "coverage": "vitest run --coverage",
     "build": "npm run build:mjs && npm run build:cjs && npm run build:iife",
     "build:mjs": "tsc --project tsconfig.mjs.json && cp res/package.mjs.json dist/mjs/package.json",

src/LocationTracker.mockClient.test.ts

@@ -2,8 +2,8 @@ import { it, describe, expect, vi, beforeEach, Mock } from 'vitest';
 import { Realtime } from 'ably/promises';
 
 import Space, { SpaceMember } from './Space.js';
-import Locations, { LocationChange } from './Locations.js';
-import LocationTracker, { LocationTrackerPredicate } from './LocationTracker.js';
+import Locations from './Locations.js';
+import LocationTracker, { LocationTrackerPredicate, LocationChange } from './LocationTracker.js';
 import { createPresenceMessage } from './utilities/test/fakes.js';
 import { LOCATION_UPDATE } from './utilities/Constants.js';
 
@@ -14,6 +14,7 @@ interface LocationsTrackerTestContext {
   spaceMember: SpaceMember;
   locationTracker: LocationTracker<{ form: string }>;
   validEvent: LocationChange<{ form: string }>;
+  space: Space;
   spy: Mock;
 }
 
@@ -75,6 +76,8 @@ describe('LocationTracker', () => {
         form: 'settings',
       },
     };
+
+    context.space = space;
   });
 
   it<LocationsTrackerTestContext>('fires when a valid location event is fired', ({
@@ -139,29 +142,34 @@ describe('LocationTracker', () => {
     expect(secondSpy).toHaveBeenCalledOnce();
   });
 
-  it<LocationsTrackerTestContext>('returns a list of only the members that are in the correct location', async ({
+  it<LocationsTrackerTestContext>('returns a list of members that are in the predicated location', async ({
     locationTracker,
     locations,
+    space,
   }) => {
     expect(locationTracker.members()).toEqual([]);
     await locations.space.enter({});
+
     locations.set({
       form: 'settings',
     });
-    expect(locationTracker.members()).toEqual([
-      {
-        clientId: 'MOCK_CLIENT_ID',
-        connectionId: '1',
-        isConnected: true,
-        lastEvent: {
-          name: 'enter',
-          timestamp: 1,
-        },
-        location: {
-          form: 'settings',
-        },
-        profileData: {},
+
+    const member = {
+      clientId: 'MOCK_CLIENT_ID',
+      connectionId: '1',
+      isConnected: true,
+      lastEvent: {
+        name: 'enter' as 'enter',
+        timestamp: 1,
       },
-    ]);
+      location: {
+        form: 'settings',
+      },
+      profileData: {},
+    };
+
+    vi.spyOn(space, 'getMembers').mockImplementationOnce(() => [member]);
+
+    expect(locationTracker.members()).toEqual([member]);
   });
 });

src/LocationTracker.ts

@@ -1,8 +1,14 @@
-import Locations, { LocationChange } from './Locations.js';
+import Locations from './Locations.js';
 import { SpaceMember } from './Space.js';
 import { LOCATION_UPDATE } from './utilities/Constants.js';
 import { EventListener } from './utilities/EventEmitter.js';
 
+export type LocationChange<T> = {
+  member: SpaceMember;
+  previousLocation: unknown;
+  currentLocation: T;
+};
+
 /**
  * Responds to a locationChange with:
  * - `true` if the change should trigger an event to fire

src/Locations.mockClient.test.ts

@@ -55,10 +55,18 @@ describe('Locations (mockClient)', () => {
       await space.enter();
       space.locations.on(LOCATION_UPDATE, spy);
       space.locations['onPresenceUpdate'](
-        createPresenceMessage('update', { clientId: '2', connectionId: '2', data: { location: 'location1' } }),
+        createPresenceMessage('update', {
+          clientId: '2',
+          connectionId: '2',
+          data: { currentLocation: 'location1', previousLocation: null },
+        }),
       );
       space.locations['onPresenceUpdate'](
-        createPresenceMessage('update', { clientId: '2', connectionId: '2', data: { location: 'location2' } }),
+        createPresenceMessage('update', {
+          clientId: '2',
+          connectionId: '2',
+          data: { currentLocation: 'location2', previousLocation: 'location1' },
+        }),
       );
       expect(spy).toHaveBeenLastCalledWith<{ member: SpaceMember; currentLocation: any; previousLocation: any }[]>({
         member: {

src/Locations.ts

@@ -1,6 +1,6 @@
 import { Types } from 'ably';
 
-import Space, { SpaceMember } from './Space.js';
+import Space from './Space.js';
 import EventEmitter from './utilities/EventEmitter.js';
 import LocationTracker, { LocationTrackerPredicate } from './LocationTracker.js';
 import { LOCATION_UPDATE } from './utilities/Constants.js';
@@ -9,12 +9,6 @@ type LocationUpdate = typeof LOCATION_UPDATE;
 
 type LocationEventMap = Record<LocationUpdate, any>;
 
-export type LocationChange<T> = {
-  member: SpaceMember;
-  previousLocation: any;
-  currentLocation: T;
-};
-
 export default class Locations extends EventEmitter<LocationEventMap> {
   constructor(public space: Space, private channel: Types.RealtimeChannelPromise) {
     super();
@@ -24,24 +18,25 @@ export default class Locations extends EventEmitter<LocationEventMap> {
   private onPresenceUpdate(message: Types.PresenceMessage) {
     if (!['update', 'leave'].includes(message.action)) return;
 
-    const { location } = message.data;
     const member = this.space.getMemberFromConnection(message.connectionId);
 
     if (member) {
-      const previousLocation = member.location;
-      member.location = location;
-      this.emit(LOCATION_UPDATE, { member, currentLocation: location, previousLocation });
+      const { previousLocation, currentLocation } = message.data;
+      member.location = currentLocation;
+      this.emit(LOCATION_UPDATE, { member: { ...member }, currentLocation, previousLocation });
     }
   }
 
-  set(location) {
+  set(location: unknown) {
     const self = this.space.getSelf();
     if (!self) {
       throw new Error('Must enter a space before setting a location');
     }
+
     return this.channel.presence.update({
       profileData: self.profileData,
-      location,
+      previousLocation: self.location,
+      currentLocation: location,
     });
   }
 

src/Space.mockClient.test.ts

@@ -170,15 +170,15 @@ describe('Space (mockClient)', () => {
         },
       ]);
 
-      createPresenceEvent(space, 'update', { data: { profileData: { a: 1 } } });
+      createPresenceEvent(space, 'enter', { data: { profileData: { a: 1 } } });
       expect(callbackSpy).toHaveBeenNthCalledWith<SpaceMember[][]>(1, [
         {
           clientId: '1',
           connectionId: '1',
           profileData: { a: 1 },
           isConnected: true,
           location: null,
-          lastEvent: { name: 'update', timestamp: 1 },
+          lastEvent: { name: 'enter', timestamp: 1 },
         },
       ]);
     });
@@ -408,12 +408,16 @@ describe('Space (mockClient)', () => {
         createPresenceEvent(space, 'enter');
 
         // Simulate a "set" location for a user
-        space.locations['onPresenceUpdate'](createPresenceMessage('update', { data: { location: '1' } }));
+        space.locations['onPresenceUpdate'](
+          createPresenceMessage('update', { data: { previousLocation: null, currentLocation: { location: '1' } } }),
+        );
 
         expect(spy).toHaveBeenCalledTimes(1);
 
         // We need to mock the message for both space & locations
-        const msg = createPresenceMessage('leave', { data: { location: '1' } });
+        const msg = createPresenceMessage('leave', {
+          data: { previousLocation: { location: '1' }, currentLocation: { location: '2' } },
+        });
         space['onPresenceUpdate'](msg);
         space.locations['onPresenceUpdate'](msg);
 

src/Space.ts

@@ -22,6 +22,7 @@ export type SpaceMember = {
 
 type SpaceLeaver = {
   clientId: string;
+  connectionId: string;
   timeoutId: ReturnType<typeof setTimeout>;
 };
 
@@ -68,10 +69,14 @@ class Space extends EventEmitter<SpaceEventsMap> {
     return this.channelName;
   }
 
-  getMemberFromConnection(connectionId?: string) {
+  getMemberFromConnection(connectionId: string): SpaceMember | undefined {
     return this.members.find((m) => m.connectionId === connectionId);
   }
 
+  getMemberIndexFromConnection(connectionId: string): number {
+    return this.members.findIndex((m) => m.connectionId === connectionId);
+  }
+
   private updateOrCreateMember(message: Types.PresenceMessage): SpaceMember {
     const member = this.getMemberFromConnection(message.connectionId);
     const lastEvent = {
@@ -81,19 +86,18 @@ class Space extends EventEmitter<SpaceEventsMap> {
 
     if (!member) {
       return {
-        clientId: message.clientId as string,
+        clientId: message.clientId,
         connectionId: message.connectionId,
         isConnected: message.action !== 'leave',
         profileData: message.data.profileData,
-        location: null,
+        location: message?.data?.currentLocation || null,
         lastEvent,
       };
     }
 
     member.isConnected = message.action !== 'leave';
-    member.profileData = message.data?.profileData ?? member.profileData;
-    member.location = member.location ? member.location : message.data?.location ?? null;
     member.lastEvent = lastEvent;
+    member.profileData = message.data?.profileData ?? member.profileData;
 
     return member;
   }
@@ -107,7 +111,7 @@ class Space extends EventEmitter<SpaceEventsMap> {
       const member = this.getMemberFromConnection(message.connectionId);
 
       this.emit('leave', member);
-      this.removeMember(message.clientId);
+      this.removeMember(message.connectionId);
       this.emit(MEMBERS_UPDATE, this.members);
 
       if (member?.location) {
@@ -121,16 +125,18 @@ class Space extends EventEmitter<SpaceEventsMap> {
 
     this.leavers.push({
       clientId: message.clientId,
+      connectionId: message.connectionId,
       timeoutId: setTimeout(timeoutCallback, this.options.offlineTimeout),
     });
   }
-  private removeLeaver(leaverIndex) {
+
+  private removeLeaver(leaverIndex: number): void {
     clearTimeout(this.leavers[leaverIndex].timeoutId);
     this.leavers.splice(leaverIndex, 1);
   }
 
-  private updateLeavers(message: Types.PresenceMessage) {
-    const index = this.leavers.findIndex(({ clientId }) => clientId === message.clientId);
+  private updateLeavers(message: Types.PresenceMessage): void {
+    const index = this.leavers.findIndex(({ connectionId }) => message.connectionId === connectionId);
 
     if (message.action === 'leave' && index < 0) {
       this.addLeaver(message);
@@ -142,8 +148,8 @@ class Space extends EventEmitter<SpaceEventsMap> {
     }
   }
 
-  private updateMembers(message: Types.PresenceMessage) {
-    const index = this.members.findIndex(({ clientId }) => clientId === message.clientId);
+  private updateMembers(message: Types.PresenceMessage): void {
+    const index = this.getMemberIndexFromConnection(message.connectionId);
     const spaceMember = this.updateOrCreateMember(message);
 
     if (index >= 0) {
@@ -154,8 +160,8 @@ class Space extends EventEmitter<SpaceEventsMap> {
     }
   }
 
-  private removeMember(clientId) {
-    const index = this.members.findIndex((member) => member.clientId === clientId);
+  private removeMember(connectionId: string): void {
+    const index = this.getMemberIndexFromConnection(connectionId);
 
     if (index >= 0) {
       this.members.splice(index, 1);
@@ -194,7 +200,11 @@ class Space extends EventEmitter<SpaceEventsMap> {
   }
 
   getSelf(): SpaceMember | undefined {
-    return this.getMemberFromConnection(this.connectionId);
+    if (this.connectionId) {
+      return this.getMemberFromConnection(this.connectionId);
+    }
+
+    return;
   }
 }
 

src/index.ts

@@ -1,6 +1,5 @@
 import Spaces from './Spaces.js';
 import Space, { type SpaceMember } from './Space.js';
-import type { LocationChange } from './Locations.js';
 
-export type { Space, LocationChange, SpaceMember };
+export type { Space, SpaceMember };
 export default Spaces;