diff --git a/CHANGELOG.md b/CHANGELOG.md index 23b76cd819..a8c9248fe3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -186,6 +186,7 @@ Notes: web developers are advised to use [`~` (tilde range)](https://github.com/ - [`webpack-cli@6.0.1`](https://npmjs.com/package/webpack-cli/) - [`webpack@5.98.0`](https://npmjs.com/package/webpack/) - Fixed [#5446](https://github.com/microsoft/BotFramework-WebChat/issues/5446). Embedded `uuid` so `microsoft-cognitiveservices-speech-sdk` do not need to use dynamic loading, as this could fail in Webpack 4 environment, in PR [#5445](https://github.com/microsoft/BotFramework-WebChat/pull/5445), by [@compulim](https://github.com/compulim) +- Improved latency of `ActivityKeyerComposer` and `ActivityTreeComposer` in PR [#5465](https://github.com/microsoft/BotFramework-WebChat/pull/5465) by [@pranavjoshi001](https://github.com/pranavjoshi001) ### Fixed @@ -216,6 +217,7 @@ Notes: web developers are advised to use [`~` (tilde range)](https://github.com/ # Removed - Deprecating `disabled` props and `useDisabled` hook in favor of new `uiState` props and `useUIState` hook, in PR [#5276](https://github.com/microsoft/BotFramework-WebChat/pull/5276), by [@compulim](https://github.com/compulim) +- Deleted `ActivitListenerComposer` and `useUpsertedActivities` hook in PR [#5465](https://github.com/microsoft/BotFramework-WebChat/pull/5465) by [@pranavjoshi001](https://github.com/pranavjoshi001) ## [4.18.0] - 2024-07-10 diff --git a/packages/api/src/hooks/Composer.tsx b/packages/api/src/hooks/Composer.tsx index b4671ec3dc..e8531cdf34 100644 --- a/packages/api/src/hooks/Composer.tsx +++ b/packages/api/src/hooks/Composer.tsx @@ -43,7 +43,6 @@ import normalizeStyleOptions from '../normalizeStyleOptions'; import patchStyleOptionsFromDeprecatedProps from '../patchStyleOptionsFromDeprecatedProps'; import ActivityAcknowledgementComposer from '../providers/ActivityAcknowledgement/ActivityAcknowledgementComposer'; import ActivityKeyerComposer from '../providers/ActivityKeyer/ActivityKeyerComposer'; -import ActivityListenerComposer from '../providers/ActivityListener/ActivityListenerComposer'; import ActivitySendStatusComposer from '../providers/ActivitySendStatus/ActivitySendStatusComposer'; import ActivitySendStatusTelemetryComposer from '../providers/ActivitySendStatusTelemetry/ActivitySendStatusTelemetryComposer'; import ActivityTypingComposer from '../providers/ActivityTyping/ActivityTypingComposer'; @@ -625,18 +624,16 @@ const ComposerCore = ({ return ( - - - - - - {typeof children === 'function' ? children(context) : children} - - - - - - + + + + + {typeof children === 'function' ? children(context) : children} + + + + + {onTelemetry && } ); diff --git a/packages/api/src/hooks/index.ts b/packages/api/src/hooks/index.ts index 2768ef5b4b..e9685467cd 100644 --- a/packages/api/src/hooks/index.ts +++ b/packages/api/src/hooks/index.ts @@ -70,6 +70,7 @@ import useUIState from './useUIState'; import useUserID from './useUserID'; import useUsername from './useUsername'; import useVoiceSelector from './useVoiceSelector'; +import useReduceActivities from './useReduceActivities'; export { useActiveTyping, @@ -143,5 +144,6 @@ export { useUIState, useUserID, useUsername, - useVoiceSelector + useVoiceSelector, + useReduceActivities }; diff --git a/packages/api/src/hooks/useReduceActivities.ts b/packages/api/src/hooks/useReduceActivities.ts new file mode 100644 index 0000000000..00d98131e7 --- /dev/null +++ b/packages/api/src/hooks/useReduceActivities.ts @@ -0,0 +1,3 @@ +import useReduceActivities from '../providers/ActivityTyping/private/useReduceActivities'; + +export default useReduceActivities; diff --git a/packages/api/src/providers/ActivityKeyer/ActivityKeyerComposer.tsx b/packages/api/src/providers/ActivityKeyer/ActivityKeyerComposer.tsx index a6724c246f..8e6716d32c 100644 --- a/packages/api/src/providers/ActivityKeyer/ActivityKeyerComposer.tsx +++ b/packages/api/src/providers/ActivityKeyer/ActivityKeyerComposer.tsx @@ -31,6 +31,7 @@ type KeyToActivitiesMap = Map; * * Local key are only persisted in memory. On refresh, they will be a new random key. */ +// eslint-disable-next-line complexity const ActivityKeyerComposer = ({ children }: Readonly<{ children?: ReactNode | undefined }>) => { const existingContext = useActivityKeyerContext(false); @@ -43,53 +44,36 @@ const ActivityKeyerComposer = ({ children }: Readonly<{ children?: ReactNode | u const activityToKeyMapRef = useRef>(Object.freeze(new Map())); const clientActivityIdToKeyMapRef = useRef>(Object.freeze(new Map())); const keyToActivitiesMapRef = useRef>(Object.freeze(new Map())); + const lastProcessedIndexRef = useRef(0); - // TODO: [P1] `useMemoWithPrevious` to check and cache the resulting array if it hasn't changed. - const activityKeysState = useMemo(() => { - const { current: activityIdToKeyMap } = activityIdToKeyMapRef; - const { current: activityToKeyMap } = activityToKeyMapRef; - const { current: clientActivityIdToKeyMap } = clientActivityIdToKeyMapRef; - const nextActivityIdToKeyMap: ActivityIdToKeyMap = new Map(); - const nextActivityKeys: Set = new Set(); - const nextActivityToKeyMap: ActivityToKeyMap = new Map(); - const nextClientActivityIdToKeyMap: ClientActivityIdToKeyMap = new Map(); - const nextKeyToActivitiesMap: KeyToActivitiesMap = new Map(); - - activities.forEach(activity => { + // Process new activities (if any) + if (activities.length > lastProcessedIndexRef.current) { + for (let i = lastProcessedIndexRef.current; i < activities.length; i++) { + // eslint-disable-next-line security/detect-object-injection + const activity = activities[i]; const activityId = getActivityId(activity); + if (!activityId) { + break; + } const clientActivityId = getClientActivityId(activity); const typingActivityId = getActivityLivestreamingMetadata(activity)?.sessionId; const key = - (clientActivityId && - (clientActivityIdToKeyMap.get(clientActivityId) || nextClientActivityIdToKeyMap.get(clientActivityId))) || - (typingActivityId && - (activityIdToKeyMap.get(typingActivityId) || nextActivityIdToKeyMap.get(typingActivityId))) || - (activityId && (activityIdToKeyMap.get(activityId) || nextActivityIdToKeyMap.get(activityId))) || - activityToKeyMap.get(activity) || - nextActivityToKeyMap.get(activity) || + (clientActivityId && clientActivityIdToKeyMapRef.current.get(clientActivityId)) || + (typingActivityId && activityIdToKeyMapRef.current.get(typingActivityId)) || + (activityId && activityIdToKeyMapRef.current.get(activityId)) || + activityToKeyMapRef.current.get(activity) || uniqueId(); - activityId && nextActivityIdToKeyMap.set(activityId, key); - clientActivityId && nextClientActivityIdToKeyMap.set(clientActivityId, key); - nextActivityToKeyMap.set(activity, key); - nextActivityKeys.add(key); - - const activities = nextKeyToActivitiesMap.has(key) ? [...nextKeyToActivitiesMap.get(key)] : []; - - activities.push(activity); - nextKeyToActivitiesMap.set(key, Object.freeze(activities)); - }); - - activityIdToKeyMapRef.current = Object.freeze(nextActivityIdToKeyMap); - activityToKeyMapRef.current = Object.freeze(nextActivityToKeyMap); - clientActivityIdToKeyMapRef.current = Object.freeze(nextClientActivityIdToKeyMap); - keyToActivitiesMapRef.current = Object.freeze(nextKeyToActivitiesMap); - - // `nextActivityKeys` could potentially same as `prevActivityKeys` despite reference differences, we should memoize it. - return Object.freeze([Object.freeze([...nextActivityKeys.values()])]) as readonly [readonly string[]]; - }, [activities, activityIdToKeyMapRef, activityToKeyMapRef, clientActivityIdToKeyMapRef, keyToActivitiesMapRef]); + activityId && activityIdToKeyMapRef.current.set(activityId, key); + clientActivityId && clientActivityIdToKeyMapRef.current.set(clientActivityId, key); + activityToKeyMapRef.current.set(activity, key); + const existingList = keyToActivitiesMapRef.current.get(key) ?? []; + keyToActivitiesMapRef.current.set(key, Object.freeze([...existingList, activity])); + lastProcessedIndexRef.current = activities.length; + } + } const getActivitiesByKey: (key?: string | undefined) => readonly WebChatActivity[] | undefined = useCallback( (key?: string | undefined): readonly WebChatActivity[] | undefined => key && keyToActivitiesMapRef.current.get(key), [keyToActivitiesMapRef] @@ -110,9 +94,18 @@ const ActivityKeyerComposer = ({ children }: Readonly<{ children?: ReactNode | u [activityIdToKeyMapRef] ); + // eslint-disable-next-line react-hooks/exhaustive-deps + const activityKeys = useMemo(() => Object.freeze([...keyToActivitiesMapRef.current.keys()]), [activities.length]); // we want to update the keys when activities change + const contextValue = useMemo( - () => ({ activityKeysState, getActivityByKey, getActivitiesByKey, getKeyByActivity, getKeyByActivityId }), - [activityKeysState, getActivitiesByKey, getActivityByKey, getKeyByActivity, getKeyByActivityId] + () => ({ + activityKeysState: [activityKeys], + getActivityByKey, + getActivitiesByKey, + getKeyByActivity, + getKeyByActivityId + }), + [activityKeys, getActivitiesByKey, getActivityByKey, getKeyByActivity, getKeyByActivityId] ); const { length: numActivities } = activities; @@ -149,7 +142,7 @@ const ActivityKeyerComposer = ({ children }: Readonly<{ children?: ReactNode | u ); } - if (activityKeysState[0].length !== keyToActivitiesMapRef.current.size) { + if (activityKeys.length !== keyToActivitiesMapRef.current.size) { console.warn( 'botframework-webchat internal assertion: "activityKeys.length" should be same as "keyToActivitiesMap.size".' ); diff --git a/packages/api/src/providers/ActivityListener/ActivityListenerComposer.tsx b/packages/api/src/providers/ActivityListener/ActivityListenerComposer.tsx deleted file mode 100644 index fc36762b68..0000000000 --- a/packages/api/src/providers/ActivityListener/ActivityListenerComposer.tsx +++ /dev/null @@ -1,28 +0,0 @@ -import type { WebChatActivity } from 'botframework-webchat-core'; -import React, { memo, useMemo, type ReactNode } from 'react'; -import usePrevious from '../../hooks/internal/usePrevious'; -import useActivities from '../../hooks/useActivities'; -import ActivityListenerContext, { type ActivityListenerContextType } from './private/Context'; - -type Props = Readonly<{ children?: ReactNode | undefined }>; - -const ActivityListenerComposer = memo(({ children }: Props) => { - const [activities] = useActivities(); - const prevActivities = usePrevious(activities, []); - - const upsertedActivitiesState = useMemo(() => { - const upserts: WebChatActivity[] = []; - - for (const activity of activities) { - prevActivities.includes(activity) || upserts.push(activity); - } - - return Object.freeze([Object.freeze(upserts)]); - }, [activities, prevActivities]); - - const context = useMemo(() => ({ upsertedActivitiesState }), [upsertedActivitiesState]); - - return {children}; -}); - -export default ActivityListenerComposer; diff --git a/packages/api/src/providers/ActivityListener/private/Context.tsx b/packages/api/src/providers/ActivityListener/private/Context.tsx deleted file mode 100644 index b9062610b2..0000000000 --- a/packages/api/src/providers/ActivityListener/private/Context.tsx +++ /dev/null @@ -1,16 +0,0 @@ -import { type WebChatActivity } from 'botframework-webchat-core'; -import { createContext } from 'react'; - -export type ActivityListenerContextType = { - upsertedActivitiesState: readonly [readonly WebChatActivity[]]; -}; - -const ActivityListenerContext = createContext( - new Proxy({} as ActivityListenerContextType, { - get() { - throw new Error('botframework-webchat internal: This hook can only used under .'); - } - }) -); - -export default ActivityListenerContext; diff --git a/packages/api/src/providers/ActivityListener/private/useContext.ts b/packages/api/src/providers/ActivityListener/private/useContext.ts deleted file mode 100644 index 4425511379..0000000000 --- a/packages/api/src/providers/ActivityListener/private/useContext.ts +++ /dev/null @@ -1,6 +0,0 @@ -import { useContext } from 'react'; -import ActivityListenerContext from './Context'; - -export default function useActivityListenerContext() { - return useContext(ActivityListenerContext); -} diff --git a/packages/api/src/providers/ActivityListener/useUpsertedActivities.ts b/packages/api/src/providers/ActivityListener/useUpsertedActivities.ts deleted file mode 100644 index 34489ed90f..0000000000 --- a/packages/api/src/providers/ActivityListener/useUpsertedActivities.ts +++ /dev/null @@ -1,6 +0,0 @@ -import type { WebChatActivity } from 'botframework-webchat-core'; -import useActivityListenerContext from './private/useContext'; - -export default function useUpsertedActivities(): readonly [readonly WebChatActivity[]] { - return useActivityListenerContext().upsertedActivitiesState; -} diff --git a/packages/component/src/providers/ActivityTree/ActivityTreeComposer.tsx b/packages/component/src/providers/ActivityTree/ActivityTreeComposer.tsx index d5569b92ce..1234b61be2 100644 --- a/packages/component/src/providers/ActivityTree/ActivityTreeComposer.tsx +++ b/packages/component/src/providers/ActivityTree/ActivityTreeComposer.tsx @@ -1,6 +1,6 @@ import { hooks, type ActivityComponentFactory } from 'botframework-webchat-api'; import type { WebChatActivity } from 'botframework-webchat-core'; -import React, { useMemo, type ReactNode } from 'react'; +import React, { useMemo, useRef, type ReactNode } from 'react'; import useMemoWithPrevious from '../../hooks/internal/useMemoWithPrevious'; import ActivityTreeContext from './private/Context'; @@ -13,7 +13,14 @@ import type { ActivityTreeContextType } from './private/Context'; type ActivityTreeComposerProps = Readonly<{ children?: ReactNode | undefined }>; -const { useActivities, useActivityKeys, useCreateActivityRenderer, useGetActivitiesByKey, useGetKeyByActivity } = hooks; +const { + useActivities, + useActivityKeys, + useCreateActivityRenderer, + useGetActivitiesByKey, + useGetKeyByActivity, + useReduceActivities +} = hooks; const ActivityTreeComposer = ({ children }: ActivityTreeComposerProps) => { const existingContext = useActivityTreeContext(false); @@ -27,26 +34,35 @@ const ActivityTreeComposer = ({ children }: ActivityTreeComposerProps) => { const getKeyByActivity = useGetKeyByActivity(); const activityKeys = useActivityKeys(); - const activities = useMemo(() => { - const activities: WebChatActivity[] = []; + // Persistent Map to store activities by their keys + const activityMapRef = useRef>>( + Object.freeze(new Map()) + ); - if (!activityKeys) { - return rawActivities; - } + const activities = + useReduceActivities((prevActivities = [], activity) => { + if (!activityKeys) { + return rawActivities; + } - for (const activity of rawActivities) { - // If an activity has multiple revisions, display the latest revision only at the position of the first revision. + const activityKey = getKeyByActivity(activity); // "Activities with same key" means "multiple revisions of same activity." - const activitiesWithSameKey = getActivitiesByKey(getKeyByActivity(activity)); + const activitiesWithSameKey = getActivitiesByKey(activityKey); // TODO: We may want to send all revisions of activity to the middleware so they can render UI to see previous revisions. - activitiesWithSameKey?.[0] === activity && - activities.push(activitiesWithSameKey[activitiesWithSameKey.length - 1]); - } + if (activitiesWithSameKey?.[activitiesWithSameKey.length - 1] === activity) { + const activityMap = activityMapRef.current; + + // Update or add the activity in the persistent Map + activityMap.set(activityKey, activity); + + // Return the updated activities as an array + return Array.from(activityMap.values()); + } - return Object.freeze(activities); - }, [activityKeys, getActivitiesByKey, getKeyByActivity, rawActivities]); + return prevActivities; + }) || []; const createActivityRenderer: ActivityComponentFactory = useCreateActivityRenderer();