Skip to content
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

[ECO-5184] feat: update channel hooks implementation #1947

Merged
merged 2 commits into from
Jan 22, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
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)
VeskeR marked this conversation as resolved.
Show resolved Hide resolved
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)
VeskeR marked this conversation as resolved.
Show resolved Hide resolved
if (channel.channelOptions.attachOnSubscribe !== false) {
await channel.attach();
}
}

unsubscribe(..._args: unknown[] /* [event], listener */): void {
Expand Down
8 changes: 7 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,18 @@ export type ChannelParameters = string | ChannelNameAndOptions;

export const version = '2.6.1';

export function channelOptionsWithAgent(options?: Ably.ChannelOptions) {
/**
* channel options for react-hooks
*/
export function channelOptionsForReactHooks(options?: Ably.ChannelOptions): Ably.ChannelOptions {
return {
...options,
params: {
...options?.params,
agent: `react-hooks/${version}`,
},
// we explicitly attach channels in React hooks (useChannel, usePresence, usePresenceListener)
// to avoid situations where implicit attachment could cause errors (when connection state is failed or disconnected)
attachOnSubscribe: false,
VeskeR marked this conversation as resolved.
Show resolved Hide resolved
};
}
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);
VeskeR marked this conversation as resolved.
Show resolved Hide resolved

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);
}
ttypic marked this conversation as resolved.
Show resolved Hide resolved

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
Copy link
Contributor

Choose a reason for hiding this comment

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

nice, this keeps the behavior with channel attachment same as before, but we now have a better contol over when we should attach to channel, instead of doing it any subscribe/presence.enter call

console.log(reason);
VeskeR marked this conversation as resolved.
Show resolved Hide resolved
});
}
}, [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);
VeskeR marked this conversation as resolved.
Show resolved Hide resolved

return { presenceData, connectionError, channelError };
}
Loading