destructing of placement from useOverlayPosition logged to the console undefined

[eran-or]

🐛 Bug Report

let { overlayProps: positionProps, placement } = useOverlayPosition({
targetRef: triggerRef,
overlayRef,
placement: 'left',
offset: props.offset || 8,
isOpen: state.isOpen,
shouldUpdatePosition:true
});
console.log(placement); //yield undefined

🤔 Expected Behavior

I'm new to this library but I'm pretty sure that it should log left
(it reside inside a PopupTrigger, and PopupTrigger has a child of Popover and inside this there's usePopover which destruct placement properly with a value, so Why it's not working on the PopupTrigger with useOverlayPosition)

😯 Current Behavior

But it's undefined

💁 Possible Solution

🔦 Context

💻 Code Sample

🌍 Your Environment

Software	Version(s)
react-spectrum
Browser
Operating System

🧢 Your Company/Team

🕷 Tracking Issue (optional)

[snowystinger]

Can you provide a codesandbox demonstrating the issue? Otherwise it's hard to tell what's going on since it's always appearing defined for the sample of stories I looked at in our storybook.

[eran-or]

codesandbox

There's two console.logs one for each file , as you can see the console.log of the trigger file is yelling undefined

[eran-or]

It will be very helpful if there was a link to codesendbox with each example demo that in the site so we can learn faster.

[snowystinger]

Here is a working example https://codesandbox.io/p/sandbox/festive-mahavira-obc10p?file=%2Fapp%2Fpopover%2FPopover.tsx

With usePopover, you shouldn't need to worry about useOverlyPosition anymore yourself. Sorry that's a bit confusing, probably due to a dead page in our docs that hasn't been pruned yet.

One thing I noticed is that you had React StrictMode turned on. We don't support that yet, but we are working on supporting it. We're tracking it here #779

[LFDanLu]

Closing since this has a working example, feel free to reopen if you have further issues/concerns.

[cedeber]

It's not really a working example, it doesn't use the same hook. useOverlayPosition still returns undefined.

You see it at first load: https://codesandbox.io/p/sandbox/dazzling-antonelli-9xv2kx

[snowystinger]

That probably has more to do with Typescript Strict communicating what to expect. It may be when we update that, we'll decide to return an empty object. For now, you can either turn it into an empty object yourself or use usePopover instead. I advocated for this earlier because it already joins a few hooks for you. For instance, it takes care of turning the returned undefined into an object here

react-spectrum/packages/@react-aria/overlays/src/usePopover.ts

Line 106 in 8d367e0

popoverProps: mergeProps(overlayProps, positionProps),

[cedeber]

Sorry, I am not sure I understand, or I was not clear enough. placement is still an export of useOverlayPosition in usePopover, which returns undefined. Why it doesn't work when I use useOverlayPosition alone ?

[snowystinger]

I think you're asking, why is this https://github.com/adobe/react-spectrum/blob/main/packages/%40react-aria/overlays/src/useOverlayPosition.ts#L70, even though it's defined as required in the types, actually undefined?

1. I think our typescript could communicate this a little more clearly, right now it seems to indicate it's required, but our config seems to be ok with it sometimes being undefined as you're pointing out
2. As for why it's undefined, it's because on first render, we need to measure the trigger and the overlay, then, once we have that, then we can place the actual elements so that they are next to each other. We don't know how to place it on first render, so placement is undefined.

Does that help? or am I still missing your question?

[cedeber]

All good now, thanks for the clarification. Yes, I realized what happened for 2. and also saw that the Tooltip component has a useEnterAnimation hook that wait for placement as isReady param. Looks like I have to do the same.

Would be nice, indeed, to improve PositionAria with placement: PlacementAxis | undefined, if possible?