[material-ui] Fix slotProps.transition types

[siriwatknp]

closes #45210
closes #40650

Root cause

The existing slotProps below for transition is not correct:

transition: SlotProps<
  React.ElementType<TransitionProps>,
  DialogTransitionSlotPropsOverrides,
  DialogOwnerState
>;

It's because TransitionProps extends React.HTMLAttributes<HTMLElement> that causes the final types to be a union of TransitionProps and HTML props. That's why transition['onExited'] does not exist, see this TypeScript playground

This PR fixes the issue by moving TransitionProps to the second parameter (the TOverrides).

All slotProps.transition should follow:

   transition: SlotComponentProps<
      React.ElementType,
      TransitionProps & DialogTransitionSlotPropsOverrides,
      DialogOwnerState
    >;

The SlotComponentProps must be used instead of SlotProps because transition slot does not support component and sx prop.

---

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

[mui-bot]

Netlify deploy preview

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

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against ce677e0

[aarongarciah]

Looks good. Left a question. I'll let @DiegoAndai do another pass.

[DiegoAndai]

This also closes #40650 👍🏼. @siriwatknp I assigned you to it and added it to the description. Please review it.

This change makes the children optional, but from what I remember, some components will throw if no children are provided. May I ask you to make sure that none of these components will crash if no children are provided? Otherwise, we have to either:

• Make the children required
• Modify the components so they can handle this case

[siriwatknp]

For #40650, I added a comment here.

This change makes the children optional

Do you mean the slotProps.transition? This is expected because the usage of <TransitionSlot {…transitionSlotProps}> always have children in the implementation.

I am not sure where this PR changes the behavior, can you point me to the code?

[siriwatknp]

any slot should be React.ElementType, otherwise user cannot provide their own props interface.

See this TS playground

[DiegoAndai]

For #40650, I added a #40650 (comment).

I agree with your comment 👌🏼

This is expected because the usage of <TransitionSlot {…transitionSlotProps}> always have children in the implementation.

You're right

One final question: In the test files, we're using

const transitionProps =
    typeof slotProps?.transition === 'function'
        ? slotProps.transition(ownerState)
        : slotProps?.transition;

Should we use the mergeSlotProps util instead? Or maybe add an additional test that uses it?

[siriwatknp]

Should we use the mergeSlotProps util instead? Or maybe add an additional test that uses it?

I don't think mergeSlotProps should be touched in this PR as it's not directly related. This PR tests that the component's slotProps.transition has the expected types.

[DiegoAndai]

@siriwatknp I didn't mean modifying mergeSlotProps.

What I propose is to use mergeSlotProps in the .spec tests (for example, this one) instead of manually checking slotProps?.transition === 'function'. I think this is better because that's what we have in our docs, so that's the way I expect users will be extending the slotProps.