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

[RFC][base] Customization of unstyled components #28189

Closed
michaldudak opened this issue Sep 6, 2021 · 17 comments
Closed

[RFC][base] Customization of unstyled components #28189

michaldudak opened this issue Sep 6, 2021 · 17 comments
Labels
discussion package: base-ui Specific to @mui/base RFC Request For Comments
Milestone

Comments

@michaldudak
Copy link
Member

馃柤 Background

Currently, there are two ways of providing custom components and their props to Unstyled components:

  1. via the component prop and additional props on the unstyled component,
  2. via the components / componentsProps props

It has several drawbacks:

  1. There are two ways to customize the Root slot: by setting a component, or components.Root
  2. It is not immediately clear which props are used internally by the unstyled component, and which are forwarded to the root
  3. Setting a component creates a polymorphic component, which causes performance problems in the type system and is hard to get right.

馃挕 Proposal

I suggest keeping only the components / componentsProps props and remove the component and the ability to forward the extra props to the root slot.

The unstyled component would accept only the props it needs to work. Everything on top of that would be provided through componentsProps.

馃帀 Advantages

  1. One way to customize a component - simpler code, that's easier to reason about
  2. Improved Typescript performance - thanks to not using polymorphic components
  3. Less confusion about which slots receive props

馃槦 Disadvantages

  1. More code to write in case of simple components with just the root slot:
    - <ButtonUnstyled component={Link} data-foo="bar" />
    + <ButtonUnstyled components={{ Root: Link }} componentsProps={{ root: { 'data-foo': 'bar } }} />
@michaldudak michaldudak added discussion package: base-ui Specific to @mui/base labels Sep 6, 2021
@mnajdova
Copy link
Member

mnajdova commented Sep 7, 2021

I like this, it's much easier to reason about 馃憤 The components that would use the unstyled can add the `component prop and the forwarded props if needed.

Off-topic, should we start referencing the unstyled components as "core"? :)

@flaviendelangle
Copy link
Member

It looks a lot like what we are doing in the DataGrid

@michaldudak
Copy link
Member Author

I remember @eps1lon was in favor of this change when we discussed it. @oliviertassinari, @siriwatknp, @hbjORbj, do you have any opinions on this topic?

@hbjORbj
Copy link
Member

hbjORbj commented Oct 1, 2021

@michaldudak I agree with you. Even if some developers are more used to the first approach, maintaining only one way to customize is much clearer over the long term.

@siriwatknp
Copy link
Member

@michaldudak I'm good with this proposal.

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 4, 2021

I don't think that any developers would enjoy doing <Stack components={{聽Root: 'span' }} />. The main risk I see in not supporting component is that it discourages the creation of small composable components as in https://reach.tech/accordion, which could be our target to maximize flexibility.

Regarding the alternative solutions to the pain, pushing the reasoning:

  1. It seems that we could remove the className prop because we already have the classes prop, for the same consistency argument?
  2. We could solve the pain of TypeScript and polymorphic components by giving up on it, the same way we give up on it with components={{聽Root: X }}?
  3. I guess we could set up a rule of priorities and document it?

@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 17, 2022

Consider #33709, I wonder if the two are equivalent.

I even wonder if components.Root is not meant to replace the first level deep component, while the component prop is meant to replace the leaf component.

#33934 (comment)

@siriwatknp
Copy link
Member

siriwatknp commented Sep 20, 2022

Just checking my understanding again here.

It is not perfect for MUI Base to receive component or slotsProps.{slot}.component prop as a way to override the subcomponents because unstyled components don't know how slots are created.

Let's take ButtonUnstyled as an example, here is the implementation if it support component prop:

const ButtonUnstyled = React.forwardRef(function ButtonUnstyled<
  BaseComponentType extends React.ElementType = ButtonUnstyledTypeMap['defaultComponent'],
>(props: ButtonUnstyledProps<BaseComponentType>, forwardedRef: React.ForwardedRef<any>) {
  ...

  const classes = useUtilityClasses(ownerState);

  const Root: React.ElementType = components.Root ?? 'button';
  const rootProps = useSlotProps({...})

  // This works for styled-components but not for general React components.
  return <Root as={component} {...rootProps}>{children}</Root>;
}) as OverridableComponent<ButtonUnstyledTypeMap>;

@michaldudak I think the question here is "do you want MUI Base to support subcomponents customization?"

In my opinion, maybe not. We can say that "developers can only replace the slots" and it is their responsibility to deal with subcomponents customization, so 馃憤 with your proposal to remove component prop for unstyled components. Here are some examples:

  • A developer using styled for styling

    import ButtonUnstyled from '@mui/base/ButtonUnstyled';
    
    const ButtonRoot = styled('button')({});
    
    export function ReusableButton = ({ component, ...props }) => (
      <ButtonUnstyled slots={{ root: ButtonRoot }} slotsProps={{ root: { as: component, ...props } }}  />
    )
  • A developer using plain CSS

    import ButtonUnstyled from '@mui/base/ButtonUnstyled';
    import './button.css';
    
    export function ReusableButton = ({ component, ...props }) => (
      <ButtonUnstyled slots={{ root: component ?? 'button' }} slotsProps={{ root: props }}  />
    )

@oliviertassinari
Copy link
Member

oliviertassinari commented Sep 20, 2022

From my perspective, with unstyled components, the value of the component prop could be about:

  1. encouraging/optimizing for one DOM element components. For example, why have to write
import ButtonUnstyled from '@mui/base/ButtonUnstyled';
import './button.css';

export function ReusableButton = ({ component, ...props }) => (
  <ButtonUnstyled slots={{ root: component ?? 'button' }} slotsProps={{ root: props }}  />
)

when you could do:

import ButtonUnstyled from '@mui/base/ButtonUnstyled';
import './button.css';

export function ReusableButton = ({ component, ...props }) => (
  <ButtonUnstyled component="span" {...props} />
)

On a related note, the unstyled components that have more that a root slot might not the best DX. For example, it might be great to breakdown the SliderUnstyled into one component for each of its sub components: root, rail, track, thumb.

  1. Maybe for minimizing API surprises when switching between different MUI Core npm packages.

@michaldudak
Copy link
Member Author

michaldudak commented Sep 20, 2022

I think the question here is "do you want MUI Base to support subcomponents customization?"

No. We don't want to assume any particular shape of slot components. Specifically, we can't assume they will have the component or as prop, as they often are host (DOM) components.

@oliviertassinari to me, the biggest value of the component prop in its current shape is its conciseness. It's much cleaner to write (and read) <ButtonUnstyled component="span" /> than <ButtonUnstyled slots={{ root: "span" }} />. Especially, when there is just a root slot.

On a related note, the unstyled components that have more that a root slot might not the best DX. For example, it might be great to breakdown the SliderUnstyled into one component for each of its sub components: root, rail, track, thumb.

I'm happy to discuss the pros and cons of different approaches here. If I understand you correctly, you propose something similar to the pattern Radix UI uses (https://www.radix-ui.com/docs/primitives/components/switch), yes?

In our case, it would be:

<SwitchUnstyled>
  <SwitchTrack component="span" />
  <SwitchThumb component={MyCustomThumb} />
</SwitchUnstyled>

At first glance, this seems like a lot to write - it's much more verbose than <SwitchUnstyled slots={{ track: 'span', thumb: MyCustomThumb }} />. It's also harder to remember what each component is built of, so developers will have to use docs more often to see all the slots. This will be especially problematic with components containing many slots (the Slider is currently winning the competition with 8 slots).

But, I admit, this pattern looks quite pretty, and it's easy to scan.

Note that using hooks can give a similar experience:

const { getInputProps } = useSwitch();

/* ... */

<span className="root">
  <span className="track" />
  <span className="thumb" />
  <input {...getInputProps()} />
</span>

Maybe for minimizing API surprises when switching between different MUI Core npm packages.

It's a great goal, however, we have to be careful to keep the same meaning of the props across different products. So if we have a component prop in Material UI and MUI Base, it should do the same thing in both libraries. Having the component replace the leaf component in Material UI and the whole slot in Base could be even more surprising for developers.

@oliviertassinari
Copy link
Member

oliviertassinari commented Sep 21, 2022

If I understand you correctly, you propose something similar to the pattern Radix UI uses (https://www.radix-ui.com/docs/primitives/components/switch), yes?

@michaldudak Correct, it's more: this could have potential, it could be interesting to test this path (test market validation or disapproval).

this seems like a lot to write

It's what put me on the track that this could have potential. If we look at the arrow of progress, on Material UI/Joy UI, we have tried to abstract API so developers could customize the components but mostly use the same API in their real apps once customized.
On MUI Base, I think that the product bet we are making is: How can we unbundle Material UI/Joy UI for all the developers that want direct access to the underlying building blocks to customize behaviors and styles? So it needs to be more code to right. Developer will create their own one-component abstractions, MUI Base's API is not what they will use in their app.
Another way to look at it: when looking at the traction of Tailwind UI, I think that we can conclude "easy of customization" >>> "boilerplate".

Note that using hooks can give a similar experience:

It helps but I think that it the hook is failing one dimension: you are forced to "wire" the hook inside ONE component. You can't easily break it down into multiple components to later compose. For hooks, I think that the equivalent pattern would multiple hooks per component.


I propose we park this one component/hook vs. multiple components/hooks discussion here as it's not directly related to this issue.

@michaldudak
Copy link
Member Author

We've discussed this topic with @mnajdova, @siriwatknp, and @hbjORbj on the refinement today, and the following options were considered:

1. Regarding the component prop:

1.1. Leave as-is

This would keep the status quo, but the component props in Base and Material UI will mean different things. We've also discussed renaming the component prop to as in Material UI and Joy.

1.2. Remove

This would remove the confusion with the Material UI's component prop and would leave just one place for customization of the root element: the slots.root prop.
Removing the component prop would also likely lead to improved dev-time performance as TypeScript won't need to consider polymorphic components.
However, our users may want polymorphic components. Also, overriding component={...} is shorter to write than slots="{ root: ... }".

1.3. Rename to root

This would remove the confusion with the Material UI's component prop. Plus, it's faster to type than component.

2. Regarding the slots.root

2.1. Leave as-is

It would play well with 1.2 - there would be just one place to customize the root.

2.2. Remove

This could be paired with 1.1 or 1.3 from the options above.
There would be just one place to customize the root. We can argue if "root" is really a slot. With this option, we could treat the root as a container that can contain slots. It would simplify all the components that currently don't have any slots besides the root - they won't need the slots and slotProps at all.

@siriwatknp
Copy link
Member

I vote (1.2) to remove the component prop. This will make all MUI Base components follow slots and slotProps which are able to use a callback to insert props.

Will benefit #32088 and work with Tailwind CSS.

// just an example
<MenuButtonUnstyled
  slotProps={{
    root: ownerState => ({
      className: ownerState.active ? '...' : '...',
    }),
  }}

@oliviertassinari
Copy link
Member

oliviertassinari commented Nov 30, 2022

Comments:

  • 1.1. I suspect that the as prop name is one that we can't use with MUI beyond replacing the component rendered by styled(). Why? I imagine that the styled component will intercept the prop, it won't forward it to MUI Base, so not replace the leaf element but the MUI Base one that is wrapped. So it would break interoperability with emotion, etc.
  • 1.2. I think that the best general direction would be for having as flat as possible components in MUI Base because it's simpler to customize once you copy and paste the demo in the docs, it makes developers feel they have the bare minimum. The styled design system can then decide to abstract or keep the API low level. Assuming this is the general long-term direction (what seems to be done by successful initiatives in the space: Radix, Headless UI, and the HTML spec, <datalist>) then slots={{ root: MyComponent }} might be considered as being more boilerplate for the default use cases than necessary.
  • 1.3. I don't understand this one. I probably miss the context. Why is there a difference between the root prop in MUI Base and the component prop in Joy UI? I thought that they were both about changing the host element.
  • 2.1.
  • 2.2. If the element tree is flat for a given component it could make sense to remove the prop. To see what would be clearer from a DX perspective. In Joy UI and Material UI, I didn't find a case where we add slots/components if it's redundant with the component prop.

Overall, for long-term and structural decisions like this, I think that the developers are the ones that hold the cards. As a developer, I think that I would have a better experience with 1.1 and 2.1, and with the removal of the slot props for all the components where there is only one slot, e.g. ButtonUnstyled.
But I think that without speaking with a diverse set of users, say a couple of influences (qualitative), and getting a couple of hundreds of votes from the community (quantitative), it will be hard to know for sure the direction that would yield the best DX. To see if it's important enough to go through this more expensive user feedback process.

@michaldudak
Copy link
Member Author

Why is there a difference between the root prop in MUI Base and the component prop in Joy UI? I thought that they were both about changing the host element.

It's explained in #34334. In Base, the root/component is an equivalent of slots.root. In Material UI/Joy UI, component is passed to the styled component's as, whereas slots.root replaces the styled component completely.

But I think that without speaking with a diverse set of users, say a couple of influences (qualitative), and getting a couple of hundreds of votes from the community (quantitative), it will be hard to know for sure the direction that would yield the best DX. To see if it's important enough to go through this more expensive user feedback process.

Yes and no :)
I agree it's the community who will ultimately use the components, and they should influence the shape of the API. However, they may not know (and neither do we) which solution will be the best long-term without actually using it in projects. Considering this, we could make a decision that's the easiest to change in the future if needed. (i.e., adding a new prop in the future will not be a problem; removing one will be harder).

@michaldudak michaldudak added this to the MUI Base stable release milestone Jan 5, 2023
@michaldudak
Copy link
Member Author

At the meeting on Tuesday, the @mui/core team decided to remove the component prop.
This will help us achieve several goals:

  • simplification of the API
  • improved dev-time performance thanks to removal of polymorphism
  • improved DX when creating own components based on MUI Base ones (it is currently quite tricky to create a type that extends our props)

We'll treat is as an experiment. If the we receive feedback from the community that the component prop is missed, we'll add it back. Restoring the current behavior won't be a breaking change.

@mnajdova mnajdova changed the title [RFC] Customization of unstyled components [RFC][base] Customization of unstyled components Apr 17, 2023
@michaldudak
Copy link
Member Author

I'm closing the issue as the decision was made and the implementation of the first component is mostly complete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion package: base-ui Specific to @mui/base RFC Request For Comments
Projects
Archived in project
Development

No branches or pull requests

6 participants