Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[base-ui] Inconsistent merge order of classes in mergeSlotProps #39601

Closed
2 tasks done
mj12albert opened this issue Oct 25, 2023 · 0 comments 路 Fixed by #39616
Closed
2 tasks done

[base-ui] Inconsistent merge order of classes in mergeSlotProps #39601

mj12albert opened this issue Oct 25, 2023 · 0 comments 路 Fixed by #39616
Assignees
Labels
bug 馃悰 Something doesn't work package: base-ui Specific to @mui/base

Comments

@mj12albert
Copy link
Member

mj12albert commented Oct 25, 2023

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Steps to reproduce 馃暪

Sandbox: https://codesandbox.io/s/merge-slot-props-classes-order-w9tkzp?file=/src/App.tsx

In the component that uses useSlotProps, you must both:

  1. use useSlotProps without passing getSlotProps, AND
  2. do not explicitly destructure a className prop and pass it into the className parameter of useSlotProps, we want it go to into externalForwardedProps instead
const rootProps = useSlotProps({
  elementType: RootComponent,
  externalSlotProps: slotProps.root,
  externalForwardedProps: other, // `className` prop goes here instead
  additionalProps: {
    ref,
  ,
  ownerState,
  className: [classes.root],
});

Current behavior 馃槸

When getSlotProps is not passed, the className join order is:

const joinedClasses = clsx(
  externalForwardedProps?.className,
  externalSlotProps?.className,
  className,
  additionalProps?.className,
);

When it is passed, the join order is:

const joinedClasses = clsx(
  internalSlotProps?.className,
  additionalProps?.className,
  className,
  externalForwardedProps?.className,
  externalSlotProps?.className,
);

Expected behavior 馃

Disregarding internalSlotProps.className, the order of the rest should be the same.

Context 馃敠

I was investigating why this test broke after refactoring material-next/InputLabel to use useSlotProps

For more background, the test was added in #33205 and #34451

Your environment 馃寧

npx @mui/envinfo
  Don't forget to mention which browser you used.
  Output from `npx @mui/envinfo` goes here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 馃悰 Something doesn't work package: base-ui Specific to @mui/base
Projects
None yet
1 participant