[material-ui][Autocomplete] Fix ownerState is undefined in styleOverrides for Autocomplete

[sai6855]

closes #43992

before: https://codesandbox.io/p/sandbox/silent-star-dfld6y?from-embed=&workspaceId=e3af0474-6cb1-4121-8a53-58b7b0668e3e

Not able to create sandbox from preview, please copy code from before sandbox and paste it this preview to see the effect.

https://deploy-preview-43994--material-ui.netlify.app/material-ui/react-autocomplete/#combo-box

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

[mui-bot]

Netlify deploy preview

https://deploy-preview-43994--material-ui.netlify.app/

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 1d1cd1b

[sai6855]

Reason for value of ownerState is undefined in styleOverrides is, absence of ownerState key in listBoxprops

useSlot hook doesn't attach ownerState if elementType is an html element , since elementType of ListBoxSlot is 'ul' not a React component (check here ), ownerState was missing in listBoxprops.

material-ui/packages/mui-utils/src/appendOwnerState/appendOwnerState.ts

Lines 43 to 46 in be68b09

	if (elementType === undefined || isHostComponent(elementType)) {
	return otherProps as AppendOwnerStateReturnType<ElementType, OtherProps, OwnerState>;
	}

Attaching ownerState explicity seems to fix the issue

[mnajdova]

The issue is a bit different here. We define the ListboxSlot as slots, but then we don't use it as a slot, but rather as a value inside the as prop. Slots by definition should replace the element that is rendered by default, not just used as an as prop.

The better fix would be to use the AutocompleteListbox on this line instead, at least based on the implementation.

@DiegoAndai we may want to revise the implementation of slots, we need to be consistent into what slots represents (they are replacement for the default parts of the component, they shouldn't behave as an alias to the as/component prop.

[sai6855]

@mnajdova Using AutocompleteListBox as elementType and using ListBoxSLot as slot instead of value to as prop, seems to do the trick. updated code here a4f4d2c

[mnajdova]

@mnajdova Using AutocompleteListBox as elementType and using ListBoxSLot as slot instead of value to as prop, seems to do the trick. updated code here a4f4d2c

Yeah, that is another solution that may work. Let's hold off the merging tough, as the slots inconsistency implementation is what worries me the most.

[DiegoAndai]

they are replacement for the default parts of the component, they shouldn't behave as an alias to the as/component prop.

I agree with @mnajdova. This was an oversight in #41875.

@sai6855's latest changes seem to follow this.

However, this is a breaking change for the (deprecated) ListboxComponent prop. Would this fix still work if we kept the original behavior for the ListboxComponent prop? Something like

// line 685
<ListboxSlot as={ListboxComponent} {...listboxProps}>
// ...

(And remove it from externalForwardedProps)

What do you think, @mnajdova? I know this makes the ListboxComponent prop different from the other *Component props, but I think that's okay. It has been like this for a long time, and that prop is now deprecated, so it's better not to change it.

[mnajdova]

What do you think, @mnajdova? I know this makes the ListboxComponent prop different from the other *Component props, but I think that's okay. It has been like this for a long time, and that prop is now deprecated, so it's better not to change it.

Let's keep the deprecation and add a TODO comment to remember to change it in the next major (or just remove the prop if that's the case).

[DiegoAndai]

@sai6855 let's implement what I described in this comment #43994 (comment)

Let me know if you have any questions about it 😊

[mnajdova]

Looks good, sorry for the delay. Let's merge this to fix the regression 👌