From 4879d50ee3b1d2faedfd7c9001119222315b5f6d Mon Sep 17 00:00:00 2001 From: Owen Pearson Date: Wed, 4 Oct 2023 17:05:30 +0100 Subject: [PATCH 1/5] fix: wrap usePresence channel options in ref --- src/platform/react-hooks/src/hooks/usePresence.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/platform/react-hooks/src/hooks/usePresence.ts b/src/platform/react-hooks/src/hooks/usePresence.ts index e56bcdd248..88f0c981e8 100644 --- a/src/platform/react-hooks/src/hooks/usePresence.ts +++ b/src/platform/react-hooks/src/hooks/usePresence.ts @@ -1,5 +1,5 @@ import { Types } from '../../../../../ably.js'; -import { useCallback, useEffect, useState } from 'react'; +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { ChannelParameters } from '../AblyReactHooks.js'; import { useAbly } from './useAbly.js'; import { useStateErrors } from './useStateErrors.js'; @@ -28,11 +28,21 @@ export function usePresence( const subscribeOnly = typeof channelNameOrNameAndOptions === 'string' ? false : params.subscribeOnly; - const channel = ably.channels.get(params.channelName, params.options); + const channelOptions = params.options; + const channelOptionsRef = useRef(channelOptions); + + const channel = useMemo(() => ably.channels.get(params.channelName, channelOptionsRef.current), [ably, params.channelName]); const skip = params.skip; const { connectionError, channelError } = useStateErrors(params); + useEffect(() => { + if (channelOptionsRef.current !== channelOptions && channelOptions) { + channel.setOptions(channelOptions); + } + channelOptionsRef.current = channelOptions; + }, [channel, channelOptions]); + const [presenceData, updatePresenceData] = useState>>([]); const updatePresence = async (message?: Types.PresenceMessage) => { From 695e5bbbea26f9097d0fc50b2f3d7ec144749fea Mon Sep 17 00:00:00 2001 From: Owen Pearson Date: Wed, 4 Oct 2023 16:57:05 +0100 Subject: [PATCH 2/5] refactor: send react-hooks agent as channel param --- CONTRIBUTING.md | 2 +- src/platform/react-hooks/src/AblyProvider.tsx | 13 ------------- src/platform/react-hooks/src/AblyReactHooks.ts | 12 ++++++++++++ src/platform/react-hooks/src/hooks/useChannel.ts | 9 ++++++--- src/platform/react-hooks/src/hooks/usePresence.ts | 9 ++++++--- 5 files changed, 25 insertions(+), 20 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index de0cdea3ec..4dce45af00 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -18,7 +18,7 @@ 2. Create a new branch for the release, for example `release/1.2.3` 3. Update the CHANGELOG.md with any customer-affecting changes since the last release and add this to the git index 4. Run `npm version --no-git-tag-version` with the new version and add the changes to the git index -5. Update the version number to the new version in `src/platform/react-hooks/src/AblyProvider.tsx` +5. Update the version number to the new version in `src/platform/react-hooks/src/AblyReactHooks.ts` 6. Create a PR for the release branch 7. Once the release PR is landed to the `main` branch, checkout the `main` branch locally (remember to pull the remote changes) and run `npm run build` 8. Run `git tag ` with the new version and push the tag to git diff --git a/src/platform/react-hooks/src/AblyProvider.tsx b/src/platform/react-hooks/src/AblyProvider.tsx index b6995c5f0e..c26f8e12a4 100644 --- a/src/platform/react-hooks/src/AblyProvider.tsx +++ b/src/platform/react-hooks/src/AblyProvider.tsx @@ -4,8 +4,6 @@ import * as Ably from 'ably'; import { Types } from '../../../../ably.js'; import React, { useMemo } from 'react'; -const version = '1.2.45'; - const canUseSymbol = typeof Symbol === 'function' && typeof Symbol.for === 'function'; interface AblyProviderProps { @@ -28,8 +26,6 @@ export function getContext(ctxId = 'default'): AblyContextType { return ctxMap[ctxId]; } -let hasSentAgent = false; - export const AblyProvider = ({ client, children, id = 'default' }: AblyProviderProps) => { if (!client) { throw new Error('AblyProvider: the `client` prop is required'); @@ -46,14 +42,5 @@ export const AblyProvider = ({ client, children, id = 'default' }: AblyProviderP context = ctxMap[id] = React.createContext(realtime ?? 1); } - React.useEffect(() => { - if (!hasSentAgent) { - hasSentAgent = true; - realtime.request('GET', '/time', null, null, { - 'Ably-Agent': `react-hooks-time-ping/${version}`, - }); - } - }); - return {children}; }; diff --git a/src/platform/react-hooks/src/AblyReactHooks.ts b/src/platform/react-hooks/src/AblyReactHooks.ts index 0d190a818d..fac2c4c7b4 100644 --- a/src/platform/react-hooks/src/AblyReactHooks.ts +++ b/src/platform/react-hooks/src/AblyReactHooks.ts @@ -16,3 +16,15 @@ export type ChannelNameAndId = { id?: string; }; export type ChannelParameters = string | ChannelNameAndOptions; + +export const version = '1.2.45'; + +export function channelOptionsWithAgent(options?: Types.ChannelOptions) { + return { + ...options, + params: { + ...options?.params, + agent: `react-hooks/${version}`, + }, + }; +} diff --git a/src/platform/react-hooks/src/hooks/useChannel.ts b/src/platform/react-hooks/src/hooks/useChannel.ts index 49aae48e6a..d747281a7d 100644 --- a/src/platform/react-hooks/src/hooks/useChannel.ts +++ b/src/platform/react-hooks/src/hooks/useChannel.ts @@ -1,6 +1,6 @@ import { Types } from '../../../../../ably.js'; import { useEffect, useMemo, useRef } from 'react'; -import { ChannelParameters } from '../AblyReactHooks.js'; +import { channelOptionsWithAgent, ChannelParameters } from '../AblyReactHooks.js'; import { useAbly } from './useAbly.js'; import { useStateErrors } from './useStateErrors.js'; @@ -45,13 +45,16 @@ export function useChannel( const channelOptionsRef = useRef(channelOptions); const ablyMessageCallbackRef = useRef(ablyMessageCallback); - const channel = useMemo(() => ably.channels.get(channelName, channelOptionsRef.current), [ably, channelName]); + const channel = useMemo( + () => ably.channels.get(channelName, channelOptionsWithAgent(channelOptionsRef.current)), + [ably, channelName] + ); const { connectionError, channelError } = useStateErrors(channelHookOptions); useEffect(() => { if (channelOptionsRef.current !== channelOptions && channelOptions) { - channel.setOptions(channelOptions); + channel.setOptions(channelOptionsWithAgent(channelOptions)); } channelOptionsRef.current = channelOptions; }, [channel, channelOptions]); diff --git a/src/platform/react-hooks/src/hooks/usePresence.ts b/src/platform/react-hooks/src/hooks/usePresence.ts index 88f0c981e8..c328a6055b 100644 --- a/src/platform/react-hooks/src/hooks/usePresence.ts +++ b/src/platform/react-hooks/src/hooks/usePresence.ts @@ -1,6 +1,6 @@ import { Types } from '../../../../../ably.js'; import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; -import { ChannelParameters } from '../AblyReactHooks.js'; +import { channelOptionsWithAgent, ChannelParameters } from '../AblyReactHooks.js'; import { useAbly } from './useAbly.js'; import { useStateErrors } from './useStateErrors.js'; @@ -31,14 +31,17 @@ export function usePresence( const channelOptions = params.options; const channelOptionsRef = useRef(channelOptions); - const channel = useMemo(() => ably.channels.get(params.channelName, channelOptionsRef.current), [ably, params.channelName]); + const channel = useMemo( + () => ably.channels.get(params.channelName, channelOptionsWithAgent(channelOptionsRef.current)), + [ably, params.channelName] + ); const skip = params.skip; const { connectionError, channelError } = useStateErrors(params); useEffect(() => { if (channelOptionsRef.current !== channelOptions && channelOptions) { - channel.setOptions(channelOptions); + channel.setOptions(channelOptionsWithAgent(channelOptions)); } channelOptionsRef.current = channelOptions; }, [channel, channelOptions]); From a2982405aaf07d29a931c35c4ba5ad6748d76f7f Mon Sep 17 00:00:00 2001 From: Owen Pearson Date: Thu, 5 Oct 2023 12:32:14 +0100 Subject: [PATCH 3/5] refactor: make arrEvery callback params non-optional --- src/common/lib/util/utils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/common/lib/util/utils.ts b/src/common/lib/util/utils.ts index 533daef412..c3e271839d 100644 --- a/src/common/lib/util/utils.ts +++ b/src/common/lib/util/utils.ts @@ -318,10 +318,10 @@ export const arrFilter = (Array.prototype.filter as unknown) }; export const arrEvery = (Array.prototype.every as unknown) - ? function (arr: Array, fn: (value: T, index?: number, arr?: Array) => boolean) { + ? function (arr: Array, fn: (value: T, index: number, arr: Array) => boolean) { return arr.every(fn); } - : function (arr: Array, fn: (value: T, index?: number, arr?: Array) => boolean) { + : function (arr: Array, fn: (value: T, index: number, arr: Array) => boolean) { const len = arr.length; for (let i = 0; i < len; i++) { if (!fn(arr[i], i, arr)) { From 65ff5d7acf6eb670465e4d663837b4a3eb941c8a Mon Sep 17 00:00:00 2001 From: Owen Pearson Date: Thu, 5 Oct 2023 12:32:33 +0100 Subject: [PATCH 4/5] refactor: add `arrEquals` util --- src/common/lib/util/utils.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/common/lib/util/utils.ts b/src/common/lib/util/utils.ts index c3e271839d..cbd1214fd6 100644 --- a/src/common/lib/util/utils.ts +++ b/src/common/lib/util/utils.ts @@ -604,3 +604,12 @@ export function toBase64(str: string) { } return stringifyBase64(parseUtf8(str)); } + +export function arrEquals(a: any[], b: any[]) { + return ( + a.length === b.length && + arrEvery(a, function (val, i) { + return val === b[i]; + }) + ); +} From f38c5a7718593d46931c6b0bd0584bcfbfb9aa9d Mon Sep 17 00:00:00 2001 From: Owen Pearson Date: Thu, 5 Oct 2023 12:33:38 +0100 Subject: [PATCH 5/5] fix: improve channel options reattachment logic This internal change makes the client more selective about which options provided to `channels.get` will be interpreted as requiring reattachment. More precisely, the channel params and modes will now be shallow compared to existing params and modes, and only if they differ will the call the `chnanels.get` throw an error (, provided of course that the channel is already attached or attaching). --- src/common/lib/client/realtimechannel.ts | 15 ++++++++++++++- test/realtime/channel.test.js | 10 +++++++--- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/common/lib/client/realtimechannel.ts b/src/common/lib/client/realtimechannel.ts index ce6f50d687..615ab6a686 100644 --- a/src/common/lib/client/realtimechannel.ts +++ b/src/common/lib/client/realtimechannel.ts @@ -185,7 +185,20 @@ class RealtimeChannel extends Channel { } _shouldReattachToSetOptions(options?: API.Types.ChannelOptions) { - return (this.state === 'attached' || this.state === 'attaching') && (options?.params || options?.modes); + if (!(this.state === 'attached' || this.state === 'attaching')) { + return false; + } + if (options?.params) { + if (!this.params || !Utils.shallowEquals(this.params, options.params)) { + return true; + } + } + if (options?.modes) { + if (!this.modes || !Utils.arrEquals(options.modes, this.modes)) { + return true; + } + } + return false; } publish(...args: any[]): void | Promise { diff --git a/test/realtime/channel.test.js b/test/realtime/channel.test.js index 252a9b1ef6..a079b64062 100644 --- a/test/realtime/channel.test.js +++ b/test/realtime/channel.test.js @@ -647,7 +647,9 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async try { realtime.channels.get(testName, { - params: params, + params: { + modes: 'subscribe', + }, }); } catch (err) { try { @@ -714,7 +716,9 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async var setOptionsReturned = false; channel.setOptions( { - params: params, + params: { + modes: 'publish', + }, }, function () { /* Wait a tick so we don' depend on whether the update event runs the @@ -743,7 +747,7 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async var setOptionsReturned = false; channel.setOptions( { - modes: modes, + modes: ['subscribe'], }, function () { Ably.Realtime.Platform.Config.nextTick(function () {