diff --git a/src/core/rooms.ts b/src/core/rooms.ts index 419527ba..2936b8f0 100644 --- a/src/core/rooms.ts +++ b/src/core/rooms.ts @@ -231,7 +231,10 @@ export class DefaultRooms implements Rooms { this._rooms.delete(roomId); const releasePromise = existing.promise.then((room) => { this._logger.debug('Rooms.release(); releasing room', { roomId, nonce: existing.nonce }); - return room.release(); + return room.release().then(() => { + this._logger.debug('Rooms.release(); room released', { roomId, nonce: existing.nonce }); + this._releasing.delete(roomId); + }); }); this._logger.debug('Rooms.release(); creating new release promise', { roomId, nonce: existing.nonce }); diff --git a/src/react/providers/chat-room-provider.tsx b/src/react/providers/chat-room-provider.tsx index 6d56fa6c..8edd2dfc 100644 --- a/src/react/providers/chat-room-provider.tsx +++ b/src/react/providers/chat-room-provider.tsx @@ -141,6 +141,7 @@ export const ChatRoomProvider: React.FC = ({ // Create an effect that changes the room when the id or options change const prevId = useRef(roomId); const prevOptions = useRef(options); + const prevClient = useRef(client); const roomReleaseQueue = useRef(new RoomReleaseQueue(logger)); // update the release queue if the logger changes - as it means we have a new client @@ -156,28 +157,42 @@ export const ChatRoomProvider: React.FC = ({ }, [logger]); useEffect(() => { - if (prevId.current === roomId && prevOptions.current === options) { + logger.debug(`ChatRoomProvider(); running room options change effect`, { roomId, options }); + if (client == prevClient.current && prevId.current === roomId && prevOptions.current === options) { return; } prevId.current = roomId; prevOptions.current = options; + prevClient.current = client; // The room options or id is changing in some way, so we need to release the old room // before we create a new one setValue((prev: ChatRoomContextType) => { - void client.rooms.release(prev.roomId).catch(); + logger.debug(`ChatRoomProvider(); running options change setValue`, { roomId, options }); + // If we're releasing, release the room - it's guaranteed to be changing + if (release) { + logger.debug(`ChatRoomProvider(); releasing value`, { roomId }); + void client.rooms.release(prev.roomId).catch(); + } else if (attach) { + // If we're not releasing, but we're attaching, detach the room + void prev.room.then((room) => { + logger.debug(`ChatRoomProvider(); detaching value`, { roomId }); + void room.detach().catch(); + }); + } + logger.debug(`ChatRoomProvider(); updating value`, { roomId, options }); return { room: client.rooms.get(roomId, options).catch(), roomId, options }; }); - }, [client, roomId, options, logger]); + }, [client, roomId, options, logger, attach, release]); // Create an effect that attaches and detaches the room, or releases it when the component unmounts useEffect(() => { let unmounted = false; let resolvedRoom: Room | undefined; const currentReleaseQueue = roomReleaseQueue.current; - logger.debug(`ChatRoomProvider(); running useEffect`, { roomId: value.roomId }); + logger.debug(`ChatRoomProvider(); running attachment and release effect`, { roomId: value.roomId }); // If there was a previous release queued for this room, abort it currentReleaseQueue.abort(value.roomId, value.options); @@ -215,10 +230,7 @@ export const ChatRoomProvider: React.FC = ({ logger.debug(`ChatRoomProvider(); releasing room`, { roomId: value.roomId }); currentReleaseQueue.enqueue(client, value.roomId, value.options); return; - } - - // If set, detach the room - it's ok to just do this, because the room doesn't get irreversibly disposed - if (resolvedRoom && attach) { + } else if (resolvedRoom && attach) { logger.debug(`ChatRoomProvider(); detaching room`, { roomId: value.roomId }); void resolvedRoom.detach().catch(() => { // Ignore, the error will be available via various room status properties diff --git a/test/core/rooms.integration.test.ts b/test/core/rooms.integration.test.ts index d85a3ec0..54c3b32b 100644 --- a/test/core/rooms.integration.test.ts +++ b/test/core/rooms.integration.test.ts @@ -1,3 +1,4 @@ +import { RoomOptionsDefaults } from '@ably/chat'; import * as Ably from 'ably'; import { describe, expect, it } from 'vitest'; @@ -30,6 +31,23 @@ describe('Rooms', () => { expect(room).not.toBe(room1); }); + it('releases and recreates a room in cycle', async () => { + // Create a room, then release, then create another room with different options + // We include presence options here because that invokes a change to channel modes - which would flag up + // an error if we were doing releases in the wrong order etc + const chat = newChatClient(); + const room1 = await chat.rooms.get('test', { typing: { timeoutMs: 1000 }, presence: RoomOptionsDefaults.presence }); + await room1.attach(); + await chat.rooms.release('test'); + + const room2 = await chat.rooms.get('test', { typing: { timeoutMs: 2000 }, presence: RoomOptionsDefaults.presence }); + await room2.attach(); + await chat.rooms.release('test'); + + await chat.rooms.get('test', { typing: { timeoutMs: 3000 }, presence: RoomOptionsDefaults.presence }); + await chat.rooms.release('test'); + }); + it('releases a failed room', async () => { // Create a room, fail it, then release. const chat = newChatClient();