[Select]: sometimes, brief flash of no items (resulting in undefined ref on initial render at top of viewport)

[colmanhumphrey]

Bug report

Current Behavior

After rendering a select near the top of the screen, on first open I'm getting TypeError: Cannot read properties of undefined (reading 'ref') on Chrome, items[0] is undefined on Firefox. This is coming from:

primitives/packages/react/select/src/Select.tsx

Line 594 in f0a7495

const isFirstItem = selectedItem === items[0].ref.current;

And no surprise, but only when I use the ScrollUpButton: if I remove that, no more errors.

If I just refresh the page, now everything works again, and I can use the select.

Note that this only happens when I generate a component that holds the select menu using a button.

Expected behavior

There's no behaviour lacking (just the same behaviour as when I hit refresh), just want to avoid the error.

Reproducible example

Sadly I actually could not reproduce this. I copied as much as I could, and it looks something like https://stackblitz.com/edit/test-radix-select?file=pages/index.js (but this example works...).

Here's a demo of this example of the action I'm taking. In our site, at the point where I open the menu, the undefined ref error appears:

CleanShot.2022-11-18.at.01.27.16.mp4

You'll see in the example code I have a bunch of forwarded refs, which I thought might be the solution, but it doesn't seem to make a difference. I can try and recreate our env more closely, but I'm not sure of the issue basically.

Suggested solution

One possibility is maybe to make the comparison at

primitives/packages/react/select/src/Select.tsx

Line 594 in f0a7495

const isFirstItem = selectedItem === items[0].ref.current;

check if the ref exists before reading current.

Additional context

I realise this is a frustrating one to help with without a reproducible example. I would however be glad to try any debugging you can recommend instead too.

Your environment

Software	Name(s)	Version
Radix Package(s)	Nearly all of them!	All at latest
React	NextJS	12.1.6
Browser	Chrome, Firefox	C: 107 and 108, F: 107
Node	n/a	16
npm/yarn	yarn	1.22.19
Operating System	MacOS	13.0

[colmanhumphrey]

OK while I can't repro this exactly, I have at least discovered the cause in our codebase: getItems() is sometimes empty:

primitives/packages/react/select/src/Select.tsx

Line 552 in f0a7495

const items = getItems();

That's because useCollection is returning nothing, as the collectionNode in collection is null:

primitives/packages/react/collection/src/Collection.tsx

Lines 109 to 110 in f0a7495

	const collectionNode = context.collectionRef.current;
	if (!collectionNode) return [];

However, I think this just immediately gets re-rendered, and the node does exist, so generally this isn't a problem. Except of course if any of the code in Select.tsx assumes the items array isn't empty. There are four such assumptions within Select, but our code I think only hits the line I referenced above:

primitives/packages/react/select/src/Select.tsx

Line 594 in f0a7495

const isFirstItem = selectedItem === items[0].ref.current;

For now we're fixing this locally by changing all refs to ?ref, e.g: the above becomes:

const isFirstItem = selectedItem === items[0]?.ref.current

Because there's an immediate re-render, I think this is fine... at least it's causing us no issues. I am happy to make this PR, but that's only if:

• this is actually an issue with this library and not something weird on our end
• this is the way you'd like to address it

Let me know if I can help any more with this!

[pavelloz]

Thank you @colmanhumphrey for your investigation.
I can observe this bug as well.

Patching things in node_modules/ is less than ideal, so i hope to find a different way of working around it.

[iofjuupasli]

Everything the same for me. The reproduction steps are really weird - when I click on second input first and then to the Select, it works. But when it's focused on the first input and then clicking the Select crashes. But I can't find any connection between these things. I thought there is something about focus/blur, but there is no any special behavior regarding that in these inputs.
Debugging the Select code gave me the same empty items.

I confirm that adding check on items.length > 0 works for me. So with patch-package and this @radix-ui+react-select+1.1.2.patch it works:

diff --git a/node_modules/@radix-ui/react-select/dist/index.js b/node_modules/@radix-ui/react-select/dist/index.js
index f81628e..206c759 100644
--- a/node_modules/@radix-ui/react-select/dist/index.js
+++ b/node_modules/@radix-ui/react-select/dist/index.js
@@ -435,14 +435,14 @@ const $1345bda09ffc1bc6$var$SelectContentImpl = /*#__PURE__*/ $cg2C9$react.forwa
             const itemMiddleToContentBottom = fullContentHeight - contentTopToItemMiddle;
             const willAlignWithoutTopOverflow = contentTopToItemMiddle <= topEdgeToTriggerMiddle;
             if (willAlignWithoutTopOverflow) {
-                const isLastItem = selectedItem === items[items.length - 1].ref.current;
+                const isLastItem = items.length > 0 && selectedItem === items[items.length - 1].ref.current;
                 contentWrapper.style.bottom = "0px";
                 const viewportOffsetBottom = content.clientHeight - viewport.offsetTop - viewport.offsetHeight;
                 const clampedTriggerMiddleToBottomEdge = Math.max(triggerMiddleToBottomEdge, selectedItemHalfHeight + (isLastItem ? viewportPaddingBottom : 0) + viewportOffsetBottom + contentBorderBottomWidth);
                 const height = contentTopToItemMiddle + clampedTriggerMiddleToBottomEdge;
                 contentWrapper.style.height = height + 'px';
             } else {
-                const isFirstItem = selectedItem === items[0].ref.current;
+                const isFirstItem = items.length > 0 && selectedItem === items[0].ref.current;
                 contentWrapper.style.top = "0px";
                 const clampedTopEdgeToTriggerMiddle = Math.max(topEdgeToTriggerMiddle, contentBorderTopWidth + viewport.offsetTop + (isFirstItem ? viewportPaddingTop : 0) + selectedItemHalfHeight);
                 const height = clampedTopEdgeToTriggerMiddle + itemMiddleToContentBottom;
diff --git a/node_modules/@radix-ui/react-select/dist/index.module.js b/node_modules/@radix-ui/react-select/dist/index.module.js
index 9be08fc..d8ed78b 100644
--- a/node_modules/@radix-ui/react-select/dist/index.module.js
+++ b/node_modules/@radix-ui/react-select/dist/index.module.js
@@ -397,14 +397,14 @@ const $cc7e05a45900e73f$var$SelectContentImpl = /*#__PURE__*/ $01b9c$forwardRef(
             const itemMiddleToContentBottom = fullContentHeight - contentTopToItemMiddle;
             const willAlignWithoutTopOverflow = contentTopToItemMiddle <= topEdgeToTriggerMiddle;
             if (willAlignWithoutTopOverflow) {
-                const isLastItem = selectedItem === items[items.length - 1].ref.current;
+                const isLastItem = items.length > 0 && selectedItem === items[items.length - 1].ref.current;
                 contentWrapper.style.bottom = "0px";
                 const viewportOffsetBottom = content.clientHeight - viewport.offsetTop - viewport.offsetHeight;
                 const clampedTriggerMiddleToBottomEdge = Math.max(triggerMiddleToBottomEdge, selectedItemHalfHeight + (isLastItem ? viewportPaddingBottom : 0) + viewportOffsetBottom + contentBorderBottomWidth);
                 const height = contentTopToItemMiddle + clampedTriggerMiddleToBottomEdge;
                 contentWrapper.style.height = height + 'px';
             } else {
-                const isFirstItem = selectedItem === items[0].ref.current;
+                const isFirstItem = items.length > 0 && selectedItem === items[0].ref.current;
                 contentWrapper.style.top = "0px";
                 const clampedTopEdgeToTriggerMiddle = Math.max(topEdgeToTriggerMiddle, contentBorderTopWidth + viewport.offsetTop + (isFirstItem ? viewportPaddingTop : 0) + selectedItemHalfHeight);
                 const height = clampedTopEdgeToTriggerMiddle + itemMiddleToContentBottom;

[colmanhumphrey]

Patching things in node_modules/ is less than ideal, so i hope to find a different way of working around it.

Agreed. In fact for now I just pasted the entire file into our site and edited it there. Not ideal, but slightly nicer than editing within node_modules

[colmanhumphrey]

Just tried the latest version after the popper update (#1853):

• I didn't get a chance to recreate this bug exactly, but still saw some weird behaviour with the menu just disappearing on scroll up
• in popper mode this problem goes away (which of course makes sense, since it's a scroll issue)

My sense is that this issue isn't quite solved just yet, but I'm not sure

[thiagogrugel]

Just tried the latest version after the popper update (#1853):

• I didn't get a chance to recreate this bug exactly, but still saw some weird behaviour with the menu just disappearing on scroll up
• in popper mode this problem goes away (which of course makes sense, since it's a scroll issue)

My sense is that this issue isn't quite solved just yet, but I'm not sure

I have this problem with the updated package.
This issue is not fixed yet.

For me, to reproduce the problem:

• Create a select with many items on the top of the page (like padding 10px).
• Select an item below, so, when you click on the select again some items are not showing at the top because of overflow in the screen size.
• Minimize the browser.
• Open the browser again, and click on the select... it will show the error.

I'm using:
"@radix-ui/react-popover": "^1.0.5",
"@radix-ui/react-primitive": "^1.0.2",
"@radix-ui/react-select": "^1.2.1",

[benoitgrelard]

@thiagogrugel, do you have a reproducible sandbox?

I am unable to reproduce this in our storybook using a similar setup to what you describe:

CleanShot.2023-05-30.at.16.52.14.mp4

[pedablio]

I am facing this problem and i managed to reproduce.

https://codesandbox.io/s/radix-select-ref-error-9s366j

Step 1: Select one of the last items
Step 2: Put focus on the text field
Step 3: Try selecting another option

It looks like the event occurs at the same time as formik's onBlur

[benj-dobs]

I've gone ahead and raised a PR: #2623

I haven't combed through the code to make sure it's a perfect solution, but it definitely fixes the crash, and the surrounding logic looks correct, so I think it's worth having.

[benj-dobs]

Just so it doesn't get lost: @pawelshopstory provided this reproduction in #2535, which is the simplest so far - all it requires is setState in the text input's onBlur: https://codesandbox.io/s/crazy-dream-mlrwc2