diff --git a/.changelog/5958-fix-render-phase-updates.md b/.changelog/5958-fix-render-phase-updates.md
new file mode 100644
index 0000000000..fa106d3dc6
--- /dev/null
+++ b/.changelog/5958-fix-render-phase-updates.md
@@ -0,0 +1,3 @@
+### Fixed
+
+- Fixed state updates while rendering that React development builds logged as "Cannot update a component while rendering a different component", from the toaster, the typing indicator, the "New messages" button, and part grouping, in PR [#5958](https://github.com/microsoft/BotFramework-WebChat/pull/5958), by [@OEvgeny](https://github.com/OEvgeny)
diff --git a/__tests__/html2/activityOrdering/partGroupingShouldUseMaxTimestamp.html b/__tests__/html2/activityOrdering/partGroupingShouldUseMaxTimestamp.html
index 04b302d31e..adefcf09cb 100644
--- a/__tests__/html2/activityOrdering/partGroupingShouldUseMaxTimestamp.html
+++ b/__tests__/html2/activityOrdering/partGroupingShouldUseMaxTimestamp.html
@@ -24,10 +24,8 @@
import { version } from 'react';
const IGNORING_KEYWORDS = [
- 'Cannot update a component',
'Support for defaultProps',
'ReactDOM.render is no longer supported',
- 'To locate the bad setState()',
'has been deprecated, use `import {'
];
diff --git a/__tests__/html2/scrollToEndButton/scrollToEndButton.noUpdateWhileRendering.html b/__tests__/html2/scrollToEndButton/scrollToEndButton.noUpdateWhileRendering.html
new file mode 100644
index 0000000000..efdb736b8a
--- /dev/null
+++ b/__tests__/html2/scrollToEndButton/scrollToEndButton.noUpdateWhileRendering.html
@@ -0,0 +1,95 @@
+
+
+
+
+
+
+
+
+
+
+
diff --git a/__tests__/html2/scrollToEndButton/scrollToEndButton.noUpdateWhileRendering.html.snap-1.png b/__tests__/html2/scrollToEndButton/scrollToEndButton.noUpdateWhileRendering.html.snap-1.png
new file mode 100644
index 0000000000..e531ed9346
Binary files /dev/null and b/__tests__/html2/scrollToEndButton/scrollToEndButton.noUpdateWhileRendering.html.snap-1.png differ
diff --git a/__tests__/html2/themeProvider/override.html b/__tests__/html2/themeProvider/override.html
index ca18774bb9..1e69e12434 100644
--- a/__tests__/html2/themeProvider/override.html
+++ b/__tests__/html2/themeProvider/override.html
@@ -54,10 +54,7 @@
!(
message.includes(
'Support for defaultProps will be removed from function components in a future major release. Use JavaScript default parameters instead.'
- ) ||
- message.includes('ReactDOM.render is no longer supported in React 18. Use createRoot instead.') ||
- // TODO: [P0] We should fix the "Cannot update a component while rendering a different component" error.
- (message.includes('Cannot update a component') && message.includes('while rendering a different component'))
+ ) || message.includes('ReactDOM.render is no longer supported in React 18. Use createRoot instead.')
)
) {
consoleError(...args);
@@ -108,14 +105,11 @@
stylesRoot,
variant: 'fluent'
},
- createElement(
- ReactWebChat,
- {
- className: 'webchat-with-overrides',
- directLine,
- store
- }
- )
+ createElement(ReactWebChat, {
+ className: 'webchat-with-overrides',
+ directLine,
+ store
+ })
)
)
)
diff --git a/__tests__/html2/toast/noUpdateWhileRendering.html b/__tests__/html2/toast/noUpdateWhileRendering.html
new file mode 100644
index 0000000000..b1f99f61d4
--- /dev/null
+++ b/__tests__/html2/toast/noUpdateWhileRendering.html
@@ -0,0 +1,89 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/__tests__/html2/toast/noUpdateWhileRendering.html.snap-1.png b/__tests__/html2/toast/noUpdateWhileRendering.html.snap-1.png
new file mode 100644
index 0000000000..e142cff088
Binary files /dev/null and b/__tests__/html2/toast/noUpdateWhileRendering.html.snap-1.png differ
diff --git a/__tests__/html2/typing/typingIndicator.liveRegion.noUpdateWhileRendering.html b/__tests__/html2/typing/typingIndicator.liveRegion.noUpdateWhileRendering.html
new file mode 100644
index 0000000000..f88329898d
--- /dev/null
+++ b/__tests__/html2/typing/typingIndicator.liveRegion.noUpdateWhileRendering.html
@@ -0,0 +1,79 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/__tests__/html2/typing/typingIndicator.liveRegion.noUpdateWhileRendering.html.snap-1.png b/__tests__/html2/typing/typingIndicator.liveRegion.noUpdateWhileRendering.html.snap-1.png
new file mode 100644
index 0000000000..fb78f33e69
Binary files /dev/null and b/__tests__/html2/typing/typingIndicator.liveRegion.noUpdateWhileRendering.html.snap-1.png differ
diff --git a/packages/api-graph/src/private/GraphProvider.tsx b/packages/api-graph/src/private/GraphProvider.tsx
index 7872eaf540..116567b015 100644
--- a/packages/api-graph/src/private/GraphProvider.tsx
+++ b/packages/api-graph/src/private/GraphProvider.tsx
@@ -7,7 +7,7 @@ import {
type GraphSubscriber,
type Identifier
} from 'botframework-webchat-core/graph.js';
-import React, { memo, useEffect, useMemo, useState } from 'react';
+import React, { memo, useLayoutEffect, useMemo, useState } from 'react';
import { custom, function_, is, object, optional, parse, pipe, readonly, type InferInput } from 'valibot';
import GraphContext, { graphContextSchema, GraphContextType } from './GraphContext';
@@ -33,7 +33,9 @@ function GraphProvider(props: GraphProviderProps) {
const [orderedActivityNodes, setOrderedActivityNodes] = useState(EMPTY_ARRAY);
- useEffect(() => {
+ // Layout effect: activities already in the store (e.g. rehydrated) must be in this state before the re-render of any other layout effect.
+ // Otherwise, that re-render would see activities in Redux which are not in the graph yet.
+ useLayoutEffect(() => {
// Sync between graph and `orderedActivities`.
const handleChange: GraphSubscriber = record => {
const state = graph.getState();
diff --git a/packages/component/src/BasicTranscript.tsx b/packages/component/src/BasicTranscript.tsx
index a80949ff6e..763cc6ab73 100644
--- a/packages/component/src/BasicTranscript.tsx
+++ b/packages/component/src/BasicTranscript.tsx
@@ -8,6 +8,7 @@ import React, {
Fragment,
memo,
useCallback,
+ useLayoutEffect,
useMemo,
useRef,
type KeyboardEventHandler,
@@ -493,15 +494,10 @@ const InternalTranscriptScrollable = ({ children, onFocusFiller }: InternalTrans
const stickyChangedToTrue = prevSticky !== sticky && sticky;
- useMemo(
- () =>
- stickyChangedToTrue &&
- // TODO: [P2] Both `markActivityKeyAsRead` and `markAllAsAcknowledged` hook are setters of useState.
- // This means, in a render loop, we will be calling setter and will cause another re-render.
- // This is not trivial but we should think if there is a way to avoid this.
- markAllAsAcknowledged(),
- [markAllAsAcknowledged, stickyChangedToTrue]
- );
+ // `markAllAsAcknowledged` is a setter of , it cannot be called while rendering this component.
+ useLayoutEffect(() => {
+ stickyChangedToTrue && markAllAsAcknowledged();
+ }, [markAllAsAcknowledged, stickyChangedToTrue]);
// We need to check if `children` is `false` or not.
// If `children` is `false`, React.Children.count(children) will still return 1 (truthy).
diff --git a/packages/component/src/ChatHistory/private/ScrollToEndButton.tsx b/packages/component/src/ChatHistory/private/ScrollToEndButton.tsx
index 0bf0f06dac..fdaebb7e10 100644
--- a/packages/component/src/ChatHistory/private/ScrollToEndButton.tsx
+++ b/packages/component/src/ChatHistory/private/ScrollToEndButton.tsx
@@ -1,5 +1,5 @@
import { hooks } from 'botframework-webchat-api';
-import React, { Fragment, memo, MutableRefObject, useCallback, useMemo } from 'react';
+import React, { Fragment, memo, MutableRefObject, useCallback, useLayoutEffect, useMemo } from 'react';
import { useAnimatingToEnd, useAtEnd, useScrollToEnd, useSticky } from 'react-scroll-to-bottom';
import { useRefFrom } from 'use-ref-from';
@@ -46,17 +46,17 @@ const useScrollToEndRenderResult = (terminatorRef: MutableRefObject {
- // This code need to be careful reviewed as it will cause another render. The code should be converging.
- // After we call `markActivityKeyAsRead`, everything will be read and nothing will be unread.
- // That means, in next render, `unreadActivityKeys` will be emptied and the `markActivityKeyAsRead` will not get called again.
- if (sticky && unreadActivityKeys.length) {
- markActivityKeyAsRead(unreadActivityKeys[unreadActivityKeys.length - 1]);
-
- return [];
- }
+ const nextUnreadActivityKeys = useMemo(
+ () => (sticky && unreadActivityKeys.length ? [] : unreadActivityKeys),
+ [sticky, unreadActivityKeys]
+ );
- return unreadActivityKeys;
+ // This code need to be careful reviewed as it will cause another render. The code should be converging.
+ // After we call `markActivityKeyAsRead`, everything will be read and nothing will be unread.
+ // That means, in next render, `unreadActivityKeys` will be emptied and the `markActivityKeyAsRead` will not get called again.
+ // Layout effect: updating while rendering this component is not allowed.
+ useLayoutEffect(() => {
+ sticky && unreadActivityKeys.length && markActivityKeyAsRead(unreadActivityKeys[unreadActivityKeys.length - 1]);
}, [markActivityKeyAsRead, sticky, unreadActivityKeys]);
const nextUnreadActivityKeysRef = useRefFrom(nextUnreadActivityKeys);
diff --git a/packages/component/src/Transcript/LiveRegionTranscript.tsx b/packages/component/src/Transcript/LiveRegionTranscript.tsx
index a56adb2524..8baf10d7e2 100644
--- a/packages/component/src/Transcript/LiveRegionTranscript.tsx
+++ b/packages/component/src/Transcript/LiveRegionTranscript.tsx
@@ -7,7 +7,7 @@ import LiveRegionActivity from '../LiveRegion/LiveRegionActivity';
import tabbableElements from '../Utils/tabbableElements';
import useLocalizeAccessKey from '../hooks/internal/useLocalizeAccessKey';
import useSuggestedActionsAccessKey from '../hooks/internal/useSuggestedActionsAccessKey';
-import { useQueueStaticElement } from '../providers/LiveRegionTwin';
+import { useLiveRegion, useQueueStaticElement } from '../providers/LiveRegionTwin';
import LiveRegionSendFailed from './LiveRegion/SendFailed';
import isPresentational from './LiveRegion/isPresentational';
import useTypistNames from './useTypistNames';
@@ -128,7 +128,7 @@ const LiveRegionTranscript = ({ activityElementMapRef }: LiveRegionTranscriptPro
keyedActivities
]);
- useMemo(() => typingIndicator && queueStaticElement(typingIndicator), [queueStaticElement, typingIndicator]);
+ useLiveRegion(() => typingIndicator, [typingIndicator]);
return ;
};
diff --git a/packages/component/src/providers/ActivityLogicalGrouping/ActivityLogicalGroupingComposer.ts b/packages/component/src/providers/ActivityLogicalGrouping/ActivityLogicalGroupingComposer.ts
index d2e402724b..2fdb1b78c8 100644
--- a/packages/component/src/providers/ActivityLogicalGrouping/ActivityLogicalGroupingComposer.ts
+++ b/packages/component/src/providers/ActivityLogicalGrouping/ActivityLogicalGroupingComposer.ts
@@ -7,7 +7,6 @@ import ActivityLogicalGroupingContext, {
type GroupState,
type LogicalGrouping
} from './private/ActivityLogicalGroupingContext';
-import useStateWithOptimisticRef from './private/useStateWithOptimisticRef';
const activityLogicalGroupingComposerPropsSchema = pipe(
object({
@@ -22,9 +21,8 @@ const ActivityLogicalGroupingComposer = (props: ActivityLogicalGroupingComposerP
const { children } = validateProps(activityLogicalGroupingComposerPropsSchema, props);
const logicalGroupingsRef = useRef>(new Map());
- const [_activityToGroupMap, setActivityToGroupMap, activityToGroupMapRef] = useStateWithOptimisticRef<
- ReadonlyMap
- >(new Map());
+ // A ref, not a state: part grouping adds itself while rendering, and its activities read their group while rendering.
+ const activityToGroupMapRef = useRef>(new Map());
const addLogicalGrouping = useCallback(
(grouping: LogicalGrouping) => {
@@ -35,24 +33,22 @@ const ActivityLogicalGroupingComposer = (props: ActivityLogicalGroupingComposerP
const toRemoveSet = new Set(prevGroupingKeys).difference(new Set(grouping.activityKeys));
const toAddSet = new Set(grouping.activityKeys).difference(new Set(prevGroupingKeys));
- setActivityToGroupMap(
- new Map(
- [].concat(
- Array.from(activityToGroupMapRef.current).filter(([, value]) => !!value && !toRemoveSet.has(value)),
- Array.from(toAddSet).map(activityKey => [activityKey, grouping.key] as const)
- )
+ activityToGroupMapRef.current = new Map(
+ [].concat(
+ Array.from(activityToGroupMapRef.current).filter(([, value]) => !!value && !toRemoveSet.has(value)),
+ Array.from(toAddSet).map(activityKey => [activityKey, grouping.key] as const)
)
);
},
- [activityToGroupMapRef, setActivityToGroupMap]
+ [activityToGroupMapRef]
);
const removeLogicalGrouping = useCallback(
(key: string) => {
logicalGroupingsRef.current.delete(key);
- setActivityToGroupMap(new Map([...activityToGroupMapRef.current].filter(([, value]) => value !== key)));
+ activityToGroupMapRef.current = new Map([...activityToGroupMapRef.current].filter(([, value]) => value !== key));
},
- [activityToGroupMapRef, setActivityToGroupMap]
+ [activityToGroupMapRef]
);
const getLogicalGroupKey = useCallback(
diff --git a/packages/component/src/providers/ActivityLogicalGrouping/private/useStateWithOptimisticRef.ts b/packages/component/src/providers/ActivityLogicalGrouping/private/useStateWithOptimisticRef.ts
deleted file mode 100644
index 84dc83bb98..0000000000
--- a/packages/component/src/providers/ActivityLogicalGrouping/private/useStateWithOptimisticRef.ts
+++ /dev/null
@@ -1,13 +0,0 @@
-import { useCallback, useRef, useState } from 'react';
-
-export default function useStateWithOptimisticRef(initialState: T | (() => T)) {
- const [state, setState] = useState(initialState);
- const stateRef = useRef(state);
-
- const setStateWithRefUpdate = useCallback((newState: T) => {
- setState(newState);
- stateRef.current = newState;
- }, []);
-
- return [state, setStateWithRefUpdate, stateRef] as const;
-}
diff --git a/packages/component/src/providers/LiveRegionTwin/useLiveRegion.ts b/packages/component/src/providers/LiveRegionTwin/useLiveRegion.ts
index 7e9c70a300..739ac05077 100644
--- a/packages/component/src/providers/LiveRegionTwin/useLiveRegion.ts
+++ b/packages/component/src/providers/LiveRegionTwin/useLiveRegion.ts
@@ -1,6 +1,7 @@
-import { useMemo, type DependencyList, type ReactElement } from 'react';
+import { useLayoutEffect, useRef, type DependencyList, type ReactElement } from 'react';
import { useRefFrom } from 'use-ref-from';
+import shallowEquals from '../../Utils/shallowEquals';
import useQueueStaticElement from './useQueueStaticElement';
/**
@@ -14,14 +15,24 @@ import useQueueStaticElement from './useQueueStaticElement';
export default function useLiveRegion(
createNode: () => false | null | ReactElement | string | undefined,
deps: DependencyList
-) {
+): void {
const createNodeRef = useRefFrom(createNode);
+ const queuedDepsRef = useRef();
const queueStaticElement = useQueueStaticElement();
- useMemo(() => {
+ // Queue after render, not during render: updating while rendering another component is not allowed.
+ // Layout effect, so the node is in the live region before the next paint, as when it was queued during render.
+ useLayoutEffect(() => {
+ // Strict Mode runs the effect twice on mount with the same deps, the node should only be queued once.
+ if (queuedDepsRef.current && shallowEquals(queuedDepsRef.current, deps)) {
+ return;
+ }
+
+ queuedDepsRef.current = deps;
+
const node = createNodeRef?.current();
node && queueStaticElement(node);
- // eslint-disable-next-line react-hooks/exhaustive-deps, react-hooks/use-memo
+ // eslint-disable-next-line react-hooks/exhaustive-deps
}, [...deps, createNodeRef, queueStaticElement]);
}
diff --git a/packages/core/src/graph/createGraphFromStore.ts b/packages/core/src/graph/createGraphFromStore.ts
index 8436473028..f1278b4753 100644
--- a/packages/core/src/graph/createGraphFromStore.ts
+++ b/packages/core/src/graph/createGraphFromStore.ts
@@ -33,7 +33,7 @@ function createGraphFromStore(store: ReturnType): SlantGraph
graph.act(graph => graph.upsert(othersAudience, channelAudience, selfAudience));
- store.subscribe(() => {
+ const handleStoreChange = () => {
const { activities }: { activities: readonly Activity[] } = store.getState();
if (Object.is(activities, prevActivities)) {
@@ -113,7 +113,12 @@ function createGraphFromStore(store: ReturnType): SlantGraph
});
prevActivities = activities;
- });
+ };
+
+ store.subscribe(handleStoreChange);
+
+ // Activities already in the store (e.g. rehydrated) are added now, not on the next dispatch.
+ handleStoreChange();
return graph;
}
diff --git a/packages/test/harness/src/host/common/host/done.js b/packages/test/harness/src/host/common/host/done.js
index 0d2e81e460..776d1451cb 100644
--- a/packages/test/harness/src/host/common/host/done.js
+++ b/packages/test/harness/src/host/common/host/done.js
@@ -25,10 +25,7 @@ module.exports = (webDriver, resolve) =>
!(
message.includes(
'Support for defaultProps will be removed from function components in a future major release. Use JavaScript default parameters instead.'
- ) ||
- message.includes('ReactDOM.render is no longer supported in React 18. Use createRoot instead.') ||
- // TODO: [P0] We should fix the "Cannot update a component while rendering a different component" error.
- (message.includes('Cannot update a component') && message.includes('while rendering a different component'))
+ ) || message.includes('ReactDOM.render is no longer supported in React 18. Use createRoot instead.')
)
);
diff --git a/packages/test/page-object/src/globals/testHelpers/hideKnownError.js b/packages/test/page-object/src/globals/testHelpers/hideKnownError.js
index 608f3c5e62..9d9f89eb2b 100644
--- a/packages/test/page-object/src/globals/testHelpers/hideKnownError.js
+++ b/packages/test/page-object/src/globals/testHelpers/hideKnownError.js
@@ -9,10 +9,7 @@ export default function hideKnownError() {
!(
message.includes(
'Support for defaultProps will be removed from function components in a future major release. Use JavaScript default parameters instead.'
- ) ||
- message.includes('ReactDOM.render is no longer supported in React 18. Use createRoot instead.') ||
- // TODO: [P0] We should fix the "Cannot update a component while rendering a different component" error.
- (message.includes('Cannot update a component') && message.includes('while rendering a different component'))
+ ) || message.includes('ReactDOM.render is no longer supported in React 18. Use createRoot instead.')
)
) {
consoleError(...args);