Skip to content

Commit ef881eb

Browse files
authored
Fixup null reference access in Breadcrumbs computeVisibleItems. (#2146)
* Fixup null reference access in Breadcrumbs `computeVisibleItems`. TL;DR the `current` field on a ref is `T | null` not `T` and we must guard against invalid access by first interrogating `current`. Since refs are initialized as null and can become null at runtime, blind access to `current` can lead `TypeError`s (as cited in #1979). In practice it seems this _can happen reliably_ in situations where a `Breadcrumb` component is being unmounted as part of a larger component tree. Unfortunately, I have not been able to reduce this error case to a small reproduction, therefore I have not been able to add a test explicitly to signal regressions. Still, and this is quite troubling, the Typescript compiler should be flagging invalid access such as this. I'm not sure if something has been disarmed in the tsconfig to assume `T | null` is `T` in this case, but a similar access in a [TS playground](https://www.typescriptlang.org/play?ts=3.8.3#code/JYWwDg9gTgLgBAJQKYEMDG8BmUIjgcilQ3wG4AoczAVwDsNgJbElMBJWzJI+pACgCUcAN7k4cMXDRMAzvCKY4AXhboYAOmoykyTAB4AEgBUAsgBkAqmeByAogBskIJLRgA+PrWr37AiuMkAekCWTBkpFGYAIyQ4Lx84FHk6GFAkdUlpWjkpAAtgewATHmU4BXU0aigeDTR8op4KSSIYKuZ4+woAX3IgA) is flagged as an error. This aims to fix #1979. * Rephrase comment about why we're coercing the type inference.
1 parent f886178 commit ef881eb

File tree

1 file changed

+8
-2
lines changed

1 file changed

+8
-2
lines changed

packages/@react-spectrum/breadcrumbs/src/Breadcrumbs.tsx

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,14 @@ function Breadcrumbs<T>(props: SpectrumBreadcrumbsProps<T>, ref: DOMRef) {
5656

5757
let updateOverflow = useCallback(() => {
5858
let computeVisibleItems = (visibleItems: number) => {
59-
let listItems = Array.from(listRef.current.children) as HTMLLIElement[];
60-
let containerWidth = listRef.current.offsetWidth;
59+
// Refs can be null at runtime.
60+
let currListRef: HTMLUListElement | null = listRef.current;
61+
if (!currListRef) {
62+
return;
63+
}
64+
65+
let listItems = Array.from(currListRef.children) as HTMLLIElement[];
66+
let containerWidth = currListRef.offsetWidth;
6167
let isShowingMenu = childArray.length > visibleItems;
6268
let calculatedWidth = 0;
6369
let newVisibleItems = 0;

0 commit comments

Comments
 (0)