From fd6d1361b75381f2d8e7443749735c818447df55 Mon Sep 17 00:00:00 2001 From: evgeny Date: Tue, 14 Jan 2025 12:20:45 +0000 Subject: [PATCH] [ECO-5184] feat: update channel hooks implementation Because of implicit `attach()` hooks can produce additional errors e.g. when updating ably client to avoid this situation, channels created inside `ChannelProvider` now use `attachOnSubscribe = false` flag and attach is happening explicitly. --- ably.d.ts | 7 ++ src/common/lib/client/realtimechannel.ts | 7 +- src/common/lib/client/realtimepresence.ts | 6 +- .../react-hooks/src/AblyReactHooks.ts | 3 +- .../react-hooks/src/ChannelProvider.tsx | 4 +- src/platform/react-hooks/src/fakes/ably.ts | 12 ++++ .../react-hooks/src/hooks/constants.ts | 4 ++ .../react-hooks/src/hooks/useChannel.ts | 3 + .../src/hooks/useChannelAttach.test.tsx | 64 +++++++++++++++++++ .../react-hooks/src/hooks/useChannelAttach.ts | 46 +++++++++++++ .../react-hooks/src/hooks/usePresence.ts | 17 ++--- .../src/hooks/usePresenceListener.ts | 3 + 12 files changed, 158 insertions(+), 18 deletions(-) create mode 100644 src/platform/react-hooks/src/hooks/constants.ts create mode 100644 src/platform/react-hooks/src/hooks/useChannelAttach.test.tsx create mode 100644 src/platform/react-hooks/src/hooks/useChannelAttach.ts diff --git a/ably.d.ts b/ably.d.ts index 5028351ea1..dc41197293 100644 --- a/ably.d.ts +++ b/ably.d.ts @@ -893,6 +893,13 @@ export interface ChannelOptions { * An array of {@link ChannelMode} objects. */ modes?: ChannelMode[]; + /** + * A boolean which determines whether calling subscribe + * on a channel or presence object should trigger an implicit attach. Defaults to `true` + * + * Note: this option is for realtime client libraries only + */ + attachOnSubscribe?: boolean; } /** diff --git a/src/common/lib/client/realtimechannel.ts b/src/common/lib/client/realtimechannel.ts index f915d3d768..bea04fa848 100644 --- a/src/common/lib/client/realtimechannel.ts +++ b/src/common/lib/client/realtimechannel.ts @@ -433,7 +433,12 @@ class RealtimeChannel extends EventEmitter { this.subscriptions.on(event, listener); } - return this.attach(); + // (RTL7g) + if (this.channelOptions.attachOnSubscribe !== false) { + return this.attach(); + } else { + return null; + } } unsubscribe(...args: unknown[] /* [event], listener */): void { diff --git a/src/common/lib/client/realtimepresence.ts b/src/common/lib/client/realtimepresence.ts index 7f1ce2d9da..63b1d4b822 100644 --- a/src/common/lib/client/realtimepresence.ts +++ b/src/common/lib/client/realtimepresence.ts @@ -454,7 +454,11 @@ class RealtimePresence extends EventEmitter { } this.subscriptions.on(event, listener); - await channel.attach(); + + // (RTP6d) + if (channel.channelOptions.attachOnSubscribe !== false) { + await channel.attach(); + } } unsubscribe(..._args: unknown[] /* [event], listener */): void { diff --git a/src/platform/react-hooks/src/AblyReactHooks.ts b/src/platform/react-hooks/src/AblyReactHooks.ts index 6536cb1622..3d5f151781 100644 --- a/src/platform/react-hooks/src/AblyReactHooks.ts +++ b/src/platform/react-hooks/src/AblyReactHooks.ts @@ -14,12 +14,13 @@ export type ChannelParameters = string | ChannelNameAndOptions; export const version = '2.6.1'; -export function channelOptionsWithAgent(options?: Ably.ChannelOptions) { +export function channelOptionsForReactHooks(options?: Ably.ChannelOptions): Ably.ChannelOptions { return { ...options, params: { ...options?.params, agent: `react-hooks/${version}`, }, + attachOnSubscribe: false, }; } diff --git a/src/platform/react-hooks/src/ChannelProvider.tsx b/src/platform/react-hooks/src/ChannelProvider.tsx index 9e6a0c0f30..2d6dd30c04 100644 --- a/src/platform/react-hooks/src/ChannelProvider.tsx +++ b/src/platform/react-hooks/src/ChannelProvider.tsx @@ -1,7 +1,7 @@ import React, { useLayoutEffect, useMemo } from 'react'; import * as Ably from 'ably'; import { type AblyContextValue, AblyContext } from './AblyContext.js'; -import { channelOptionsWithAgent } from './AblyReactHooks.js'; +import { channelOptionsForReactHooks } from './AblyReactHooks.js'; interface ChannelProviderProps { ablyId?: string; @@ -45,7 +45,7 @@ export const ChannelProvider = ({ }, [derived, client, channel, channelName, _channelNameToChannelContext, ablyId, context]); useLayoutEffect(() => { - channel.setOptions(channelOptionsWithAgent(options)); + channel.setOptions(channelOptionsForReactHooks(options)); }, [channel, options]); return {children}; diff --git a/src/platform/react-hooks/src/fakes/ably.ts b/src/platform/react-hooks/src/fakes/ably.ts index 0a43ed3f3a..a26b87b3df 100644 --- a/src/platform/react-hooks/src/fakes/ably.ts +++ b/src/platform/react-hooks/src/fakes/ably.ts @@ -163,6 +163,10 @@ export class ClientSingleChannelConnection extends EventEmitter { public async setOptions() { // do nothing } + + public async attach() { + // do nothing + } } export class ClientSingleDerivedChannelConnection extends EventEmitter { @@ -199,6 +203,10 @@ export class ClientSingleDerivedChannelConnection extends EventEmitter { public async publish() { throw Error('no publish for derived channel'); } + + public async attach() { + // do nothing + } } export class ClientPresenceConnection { @@ -348,6 +356,10 @@ export class Channel { public async setOptions() { // do nothing } + + public async attach() { + // do nothing + } } export class ChannelPresence { diff --git a/src/platform/react-hooks/src/hooks/constants.ts b/src/platform/react-hooks/src/hooks/constants.ts new file mode 100644 index 0000000000..5879994235 --- /dev/null +++ b/src/platform/react-hooks/src/hooks/constants.ts @@ -0,0 +1,4 @@ +import type * as Ably from 'ably'; + +export const INACTIVE_CONNECTION_STATES: Ably.ConnectionState[] = ['suspended', 'closing', 'closed', 'failed']; +export const INACTIVE_CHANNEL_STATES: Ably.ChannelState[] = ['failed', 'suspended', 'detaching']; diff --git a/src/platform/react-hooks/src/hooks/useChannel.ts b/src/platform/react-hooks/src/hooks/useChannel.ts index 55c09755fb..7599cceb53 100644 --- a/src/platform/react-hooks/src/hooks/useChannel.ts +++ b/src/platform/react-hooks/src/hooks/useChannel.ts @@ -4,6 +4,7 @@ import { ChannelParameters } from '../AblyReactHooks.js'; import { useAbly } from './useAbly.js'; import { useStateErrors } from './useStateErrors.js'; import { useChannelInstance } from './useChannelInstance.js'; +import { useChannelAttach } from './useChannelAttach.js'; export type AblyMessageCallback = Ably.messageCallback; @@ -82,6 +83,8 @@ export function useChannel( }; }, [channelEvent, channel, skip]); + useChannelAttach(channel, channelHookOptions.ablyId, skip); + return { channel, publish, ably, connectionError, channelError }; } diff --git a/src/platform/react-hooks/src/hooks/useChannelAttach.test.tsx b/src/platform/react-hooks/src/hooks/useChannelAttach.test.tsx new file mode 100644 index 0000000000..5bf2e4f45a --- /dev/null +++ b/src/platform/react-hooks/src/hooks/useChannelAttach.test.tsx @@ -0,0 +1,64 @@ +import { renderHook } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { useChannelAttach } from './useChannelAttach.js'; + +interface LocalTestContext { + useChannelAttach: typeof useChannelAttach; +} + +describe('useChannelAttach', () => { + const fakeAblyClientRef: any = {}; + + beforeEach(async (context) => { + vi.doMock('./useConnectionStateListener.js', () => ({ + useConnectionStateListener: vi.fn(), + })); + + vi.doMock('./useAbly.js', () => ({ + useAbly: () => fakeAblyClientRef.current, + })); + + context.useChannelAttach = (await import('./useChannelAttach.js')).useChannelAttach; + fakeAblyClientRef.current = { connection: { state: 'initialized' } }; + }); + + it('should call attach on render', ({ useChannelAttach }) => { + const channel = { attach: vi.fn(() => Promise.resolve()) }; + const { result } = renderHook(() => useChannelAttach(channel, undefined, false)); + + expect(result.current.connectionState).toBe('initialized'); + expect(channel.attach).toHaveBeenCalled(); + }); + + it('should not call attach when skipped', ({ useChannelAttach }) => { + const channel = { attach: vi.fn(() => Promise.resolve()) }; + const { result } = renderHook(() => useChannelAttach(channel, undefined, true)); + + expect(result.current.connectionState).toBe('initialized'); + expect(channel.attach).not.toHaveBeenCalled(); + }); + + it('should not call attach when in failed state', ({ useChannelAttach }) => { + fakeAblyClientRef.current = { connection: { state: 'failed' } }; + const channel = { attach: vi.fn(() => Promise.resolve()) }; + const { result } = renderHook(() => useChannelAttach(channel, undefined, false)); + + expect(result.current.connectionState).toBe('failed'); + expect(channel.attach).not.toHaveBeenCalled(); + }); + + it('should call attach when go back to the connected state', async ({ useChannelAttach }) => { + fakeAblyClientRef.current = { connection: { state: 'suspended' } }; + const channel = { attach: vi.fn(() => Promise.resolve()) }; + const { result, rerender } = renderHook(() => useChannelAttach(channel, undefined, false)); + + expect(result.current.connectionState).toBe('suspended'); + expect(channel.attach).not.toHaveBeenCalled(); + + fakeAblyClientRef.current = { connection: { state: 'connected' } }; + rerender(); + + expect(result.current.connectionState).toBe('connected'); + expect(channel.attach).toHaveBeenCalled(); + }); +}); diff --git a/src/platform/react-hooks/src/hooks/useChannelAttach.ts b/src/platform/react-hooks/src/hooks/useChannelAttach.ts new file mode 100644 index 0000000000..94f0cc051f --- /dev/null +++ b/src/platform/react-hooks/src/hooks/useChannelAttach.ts @@ -0,0 +1,46 @@ +import type * as Ably from 'ably'; +import { useEffect, useState } from 'react'; +import { useConnectionStateListener } from './useConnectionStateListener.js'; +import { useAbly } from './useAbly.js'; +import { INACTIVE_CONNECTION_STATES } from './constants.js'; + +interface ChannelAttachResult { + connectionState: Ably.ConnectionState; +} + +export function useChannelAttach( + channel: Ably.RealtimeChannel, + ablyId: string | undefined, + skip: boolean, +): ChannelAttachResult { + const ably = useAbly(ablyId); + + // we need to listen for the current connection state in order to react to it. + // for example, we should attach when first connected, re-enter when reconnected, + // and be able to prevent attaching when the connection is in an inactive state. + // all of that can be achieved by using the useConnectionStateListener hook. + const [connectionState, setConnectionState] = useState(ably.connection.state); + useConnectionStateListener((stateChange) => { + setConnectionState(stateChange.current); + }, ablyId); + + if (ably.connection.state !== connectionState) { + setConnectionState(ably.connection.state); + } + + const shouldAttachToTheChannel = !skip && !INACTIVE_CONNECTION_STATES.includes(connectionState); + + useEffect(() => { + if (shouldAttachToTheChannel) { + channel.attach().catch((reason) => { + // we use a fire-and-forget approach for attaching, but calling detach during the attaching process or while + // suspending can cause errors that will be automatically resolved + console.log(reason); + }); + } + }, [shouldAttachToTheChannel, channel]); + + // we expose `connectionState` here for reuse in the usePresence hook, where we need to prevent + // entering and leaving presence in a similar manner + return { connectionState }; +} diff --git a/src/platform/react-hooks/src/hooks/usePresence.ts b/src/platform/react-hooks/src/hooks/usePresence.ts index a322c47851..27a3687bdd 100644 --- a/src/platform/react-hooks/src/hooks/usePresence.ts +++ b/src/platform/react-hooks/src/hooks/usePresence.ts @@ -4,8 +4,9 @@ import { ChannelParameters } from '../AblyReactHooks.js'; import { useAbly } from './useAbly.js'; import { useChannelInstance } from './useChannelInstance.js'; import { useStateErrors } from './useStateErrors.js'; -import { useConnectionStateListener } from './useConnectionStateListener.js'; import { useChannelStateListener } from './useChannelStateListener.js'; +import { INACTIVE_CHANNEL_STATES, INACTIVE_CONNECTION_STATES } from './constants.js'; +import { useChannelAttach } from './useChannelAttach.js'; export interface PresenceResult { updateStatus: (messageOrPresenceObject: T) => Promise; @@ -13,9 +14,6 @@ export interface PresenceResult { channelError: Ably.ErrorInfo | null; } -const INACTIVE_CONNECTION_STATES: Ably.ConnectionState[] = ['suspended', 'closing', 'closed', 'failed']; -const INACTIVE_CHANNEL_STATES: Ably.ChannelState[] = ['failed', 'suspended', 'detaching']; - export function usePresence( channelNameOrNameAndOptions: ChannelParameters, messageOrPresenceObject?: T, @@ -41,15 +39,6 @@ export function usePresence( messageOrPresenceObjectRef.current = messageOrPresenceObject; }, [messageOrPresenceObject]); - // we need to listen for the current connection state in order to react to it. - // for example, we should enter presence when first connected, re-enter when reconnected, - // and be able to prevent entering presence when the connection is in an inactive state. - // all of that can be achieved by using the useConnectionStateListener hook. - const [connectionState, setConnectionState] = useState(ably.connection.state); - useConnectionStateListener((stateChange) => { - setConnectionState(stateChange.current); - }, params.ablyId); - // similar to connection states, we should only attempt to enter presence when in certain // channel states. const [channelState, setChannelState] = useState(channel.state); @@ -57,6 +46,8 @@ export function usePresence( setChannelState(stateChange.current); }); + const { connectionState } = useChannelAttach(channel, params.ablyId, skip); + const shouldNotEnterPresence = INACTIVE_CONNECTION_STATES.includes(connectionState) || INACTIVE_CHANNEL_STATES.includes(channelState) || skip; diff --git a/src/platform/react-hooks/src/hooks/usePresenceListener.ts b/src/platform/react-hooks/src/hooks/usePresenceListener.ts index 656bcf3339..790bc2c16a 100644 --- a/src/platform/react-hooks/src/hooks/usePresenceListener.ts +++ b/src/platform/react-hooks/src/hooks/usePresenceListener.ts @@ -3,6 +3,7 @@ import { useCallback, useEffect, useRef, useState } from 'react'; import { ChannelParameters } from '../AblyReactHooks.js'; import { useChannelInstance } from './useChannelInstance.js'; import { useStateErrors } from './useStateErrors.js'; +import { useChannelAttach } from './useChannelAttach.js'; interface PresenceMessage extends Ably.PresenceMessage { data: T; @@ -64,5 +65,7 @@ export function usePresenceListener( }; }, [skip, onMount, onUnmount]); + useChannelAttach(channel, params.ablyId, skip); + return { presenceData, connectionError, channelError }; }