[Popper] Convert code to typescript

[danhuynhdev]

• I have followed (at least) the PR section of the contributing guide.

Resolve #34718

[danhuynhdev]

Resolve #34718

[mui-bot]

	Messages
📖	Netlify deploy preview: https://deploy-preview-34771--material-ui.netlify.app/

Details of bundle changes

Generated by 🚫 dangerJS against d6b14be

[danhuynhdev]

Not sure how to resolve this any. Seems like anchorEl has the type HTMLElement | VirtualElement | (() => HTMLElement | VirtualElement) but the current code treat it as HTMLElement | (() => HTMLElement)

[danhuynhdev]

while the original signature put it as VirtualElement | (() => VirtualElement). Is there a potential bug here?

[michaldudak]

You can define the resolveAnchorEl as an overloaded function:

function resolveAnchorEl(anchorEl: VirtualElement | (() => VirtualElement)): VirtualElement;
function resolveAnchorEl(anchorEl: HTMLElement | (() => HTMLElement)): HTMLElement;
function resolveAnchorEl(anchorEl: VirtualElement | (() => VirtualElement) | HTMLElement | (() => HTMLElement)): HTMLElement | VirtualElement {
  return typeof anchorEl === 'function' ? anchorEl() : anchorEl;
}

and define a type guard checking if an element is a DOM element;

function isHTMLElement(element: HTMLElement | VirtualElement): element is HTMLElement {
  return (element as HTMLElement).nodeType !== undefined;
}

then, after resolving the anchorEl check if it's an HTMLElement:

-if (resolvedAnchorEl && resolvedAnchorEl.nodeType === 1) {
+if (resolvedAnchorEl && isHTMLElement(resolvedAnchorEl) && resolvedAnchorEl.nodeType === 1) {

Similarly, in PopperUnstyled body, where container is defined, check if we're dealing with HTMLElement or VirtualElement. You can pass in null to ownerDocument if anchorEl is a VirtualElement.

[danhuynhdev]

I resolved this issue

[danhuynhdev]

Added this to pass type check here https://github.com/danhuynhdev/material-ui/blob/2b0f1d9684abd308de1e3c05e8bec4aaa13993f5/packages/mui-base/src/PopperUnstyled/PopperUnstyled.test.tsx#L38
But then I still have to pass ownerState as any. Not sure how to properly handle this as I'm not familiar with ownerState. From other places, it seems like ownerState isn't passed in as a prop but constructed inside the component. But I do not see that in this component.

[michaldudak]

Yeah, something seems to be off here. You should not have ownerState as a prop. I'll try to correct the code in the separate PR and you'll be able to rebase on top of my fixes.

[danhuynhdev]

Have you taken a look a what's wrong here? Happy to fix that myself if I can understand the context here.

[michaldudak]

It turned out to be a bigger problem that affects several components, and we haven't figured out what would be the best solution here.
In short, we generally don't want to expose ownerState as a prop, because users of the components should not manually set it. However, in some cases, when an MUI Base component is used in a slot of another component, it will receive the ownerState (as all slot components do). The type of this ownerState is not PopperUnstyledOwnerState, though - it's a state of a component that owns the slots, so it's impossible to type this strongly.

For now, let's define ownerState?: object in PopperUnstyledOwnProps. When we decide how to solve the problem for good, we'll revisit this part. This should allow you to drop as any from tests.

[danhuynhdev]

Sorry for the late response. I have updated the code but unfortunately, ownerState?: object isn't compatible with downstream type, as they expect a concrete interface. So I have to put it as any again, i guess it's the same as object here as we cannot guarantee typesafe-ness here.

[michaldudak]

Thanks, this looks pretty good! There's a problem with the CI build, though. It needs the yarn docs:api to be run, but executing this script causes the description of the direction prop to be lost for some reason. Let me investigate it, and I'll do the final review when everything is fine.

[michaldudak]

The direction prop is correctly removed from the docs, as it's not a part of the Popper API, so all is good. Could you please run yarn docs:api and commit the changes?

[michaldudak]

I corrected and simplified the code in a few places. I think it's good to go now. Thanks for your contribution, @danhuynhdev!