Skip to content

Commit

Permalink
[ECO-5184] feat: update channel hooks implementation
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ttypic committed Jan 21, 2025
1 parent 94d676b commit fd6d136
Show file tree
Hide file tree
Showing 12 changed files with 158 additions and 18 deletions.
7 changes: 7 additions & 0 deletions ably.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down
7 changes: 6 additions & 1 deletion src/common/lib/client/realtimechannel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 5 additions & 1 deletion src/common/lib/client/realtimepresence.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion src/platform/react-hooks/src/AblyReactHooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
}
4 changes: 2 additions & 2 deletions src/platform/react-hooks/src/ChannelProvider.tsx
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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 <AblyContext.Provider value={value}>{children}</AblyContext.Provider>;
Expand Down
12 changes: 12 additions & 0 deletions src/platform/react-hooks/src/fakes/ably.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,10 @@ export class ClientSingleChannelConnection extends EventEmitter {
public async setOptions() {
// do nothing
}

public async attach() {
// do nothing
}
}

export class ClientSingleDerivedChannelConnection extends EventEmitter {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -348,6 +356,10 @@ export class Channel {
public async setOptions() {
// do nothing
}

public async attach() {
// do nothing
}
}

export class ChannelPresence {
Expand Down
4 changes: 4 additions & 0 deletions src/platform/react-hooks/src/hooks/constants.ts
Original file line number Diff line number Diff line change
@@ -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'];
3 changes: 3 additions & 0 deletions src/platform/react-hooks/src/hooks/useChannel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<Ably.Message>;

Expand Down Expand Up @@ -82,6 +83,8 @@ export function useChannel(
};
}, [channelEvent, channel, skip]);

useChannelAttach(channel, channelHookOptions.ablyId, skip);

return { channel, publish, ably, connectionError, channelError };
}

Expand Down
64 changes: 64 additions & 0 deletions src/platform/react-hooks/src/hooks/useChannelAttach.test.tsx
Original file line number Diff line number Diff line change
@@ -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<LocalTestContext>(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<LocalTestContext>('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<LocalTestContext>('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<LocalTestContext>('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<LocalTestContext>('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();
});
});
46 changes: 46 additions & 0 deletions src/platform/react-hooks/src/hooks/useChannelAttach.ts
Original file line number Diff line number Diff line change
@@ -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 };
}
17 changes: 4 additions & 13 deletions src/platform/react-hooks/src/hooks/usePresence.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,16 @@ 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<T> {
updateStatus: (messageOrPresenceObject: T) => Promise<void>;
connectionError: Ably.ErrorInfo | null;
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<T = any>(
channelNameOrNameAndOptions: ChannelParameters,
messageOrPresenceObject?: T,
Expand All @@ -41,22 +39,15 @@ export function usePresence<T = any>(
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);
useChannelStateListener(params, (stateChange) => {
setChannelState(stateChange.current);
});

const { connectionState } = useChannelAttach(channel, params.ablyId, skip);

const shouldNotEnterPresence =
INACTIVE_CONNECTION_STATES.includes(connectionState) || INACTIVE_CHANNEL_STATES.includes(channelState) || skip;

Expand Down
3 changes: 3 additions & 0 deletions src/platform/react-hooks/src/hooks/usePresenceListener.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<T = any> extends Ably.PresenceMessage {
data: T;
Expand Down Expand Up @@ -64,5 +65,7 @@ export function usePresenceListener<T = any>(
};
}, [skip, onMount, onUnmount]);

useChannelAttach(channel, params.ablyId, skip);

return { presenceData, connectionError, channelError };
}

0 comments on commit fd6d136

Please sign in to comment.