Fix undefined reference error in SendFailed.tsx#5811
Fix undefined reference error in SendFailed.tsx#5811peterinnesmsft wants to merge 2 commits intomicrosoft:mainfrom
Conversation
… error with prevActivityKeysOfSendFailed.
There was a problem hiding this comment.
Pull request overview
Fixes a first-render crash in the transcript “Send failed” live-region announcer by ensuring usePrevious can return a defined initial value and updating SendFailed.tsx to use that initial value.
Changes:
- Updated
packages/component’s internalusePrevioushook to support aninitialValueoverload (matching the API package implementation). - Updated
SendFailed.tsxto pass an initial emptySettousePreviousto avoid calling.has()onundefined. - Added a new
prevActivityKeysOfSendFailedfalsy-check (currently redundant given the initial value).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/component/src/hooks/internal/usePrevious.ts | Adds overloads + useRef(initialValue) to allow a defined previous value on first render. |
| packages/component/src/Transcript/LiveRegion/SendFailed.tsx | Uses an initial empty Set for usePrevious to prevent undefined dereference in .has(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (!prevActivityKeysOfSendFailed) { | ||
| return true; | ||
| } | ||
|
|
There was a problem hiding this comment.
prevActivityKeysOfSendFailed is always a Set<string> here because usePrevious is called with an initialValue. The if (!prevActivityKeysOfSendFailed) { return true; } branch is therefore dead code and (if it ever became reachable) would incorrectly return true even when activityKeysOfSendFailed is empty. Suggest removing this check entirely, or (if you intend to support an undefined previous value) change it to return activityKeysOfSendFailed.size > 0 instead of unconditionally true.
| if (!prevActivityKeysOfSendFailed) { | |
| return true; | |
| } |
|
Please write a test to show how we previously failed the case. This will make sure the new code is fortifying things in proper places. Also, instead of modifying |
Changelog Entry
Description
In Dynamics 365 Contact Center, an error has been discovered in our own telemetry which points to an error occurring within SendFailed.tsx. The stack trace is as follows:
GitHub Copilot analysis points to the following minified code as being the problem:
Design
The issue appears to be caused on initial render of the web chat control; we still don't quite fully understand the exact condition, but it seems like this is due to the ordering of the underlying
useRefinusePreviousstarting as undefined, and only being updated after initial render inuseEffect, meaning that on initial render we can hit an undefined reference onprevActivityKeysOfSendFailed.Specific Changes
/component/src/hooks/internal/usePrevious.tsto match the duplicated implementation that allows for setting an initial value.SendFailed.tsxto specify an empty Set as the default value inusePreviousforprevActivityKeysOfSendFailed.hasNewSendFailedonprevActivityKeysOfSendFailedso that if it doesn't matchactivityKeysOfSendFailedand evaluates to false, we return true to indicate that a new send has indeed failed.TODO: Need to address the below.
CHANGELOG.mdReview Checklist
z-index)package.jsonandpackage-lock.jsonreviewed