search

mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.86k stars 32.26k forks source link

[Hidden] Remove deprecated API #44073

Open oliviertassinari opened 3 weeks ago

oliviertassinari commented 3 weeks ago

Steps to reproduce

  1. Open https://mui.com/material-ui/react-hidden/

Context

42305. This component was deprecated in v5.0.0, so on September 16, 2021. The migration path is documented in https://mui.com/material-ui/migration/v5-component-changes/#replace-deprecated-component.

Added to the v7 milestone per https://github.com/mui/material-ui/pull/42305#issuecomment-2133520181.

Your environment

v6.1.3

oliviertassinari commented 3 weeks ago

Let's pause this until we discuss if we want to remove Hidden or revisit its API. Hidden keeps things composable and doesn't force the use of the sx prop for devs not directly using Emotion or Pigment CSS. source

@aarongarciah Tailwind CSS users have their own utils, so I imagine it's really only about CSS Modules users.

Considering that sx is always available, I don't know. It seems that we can't get composition without using an extra DOM node or using React.cloneElement() which both feel: nop?

aarongarciah commented 3 weeks ago

Considering that sx is always available

@oliviertassinari what do you mean by this? I'm assuming you mean sx is always available because we "force" consumers to use our styling solution (Emotion/styled-components/Pigment CSS as of today).

It seems that we can't get composition without using an extra DOM node or using React.cloneElement() which both feel: nop?

I also don't like using either of those two options, and I think Hidden is a weird API, but if we end up pursuing a future in which we don't force consumers to use a styling solution (i.e. offer static CSS files consumers can load), how are you going to offer consumers a way to hide/show stuff that supports responsive values? Our current model for building layouts is components, so although I don't like using an extra dom element or React.cloneElement() [^1], maybe is the least bad option if we don't want to change our model for building layouts.

To be honest, my preferred API for building layouts these days would be to have a set of typed helper functions that just return class names (see https://x.com/devongovett/status/1836765141204885694). But that's a longer discussions.

tl;dr: I'm fine with removing Hidden, but let's dig into this topic more becase there are a lot of interrelated things we need to take into account.

[^1]: Base UI probably already makes heavy use of React.cloneElement, so not sure that's a real problem? Honest question.

oliviertassinari commented 3 weeks ago

I'm assuming you mean sx is always available because we "force" consumers to use our styling solution (Emotion/styled-components/Pigment CSS as of today).

@aarongarciah I assumed that if people use one of the existing components, we force them one of the styling solutions that we support. Today, all support sx.

At least, with JSS from v1 to v4, we never really felt forcing JSS was as a big problem. People were happy to use JSS and customize the component with something else. It didn't seem to truly matter because we designed it to play nicely with CSS modules, emotion, styled components, etc. However, it matters if JSS fails to deliver like emotion does in today's world.

So IMHO, if we could get to one great versatile styling solution, it could be good enough. In the code ejection story, it could be a bit different, maybe we need native Tailwind CSS too, but I don't know, I saw a lot of people say they hate that Shadcn UI uses Tailwind CSS but still uses Shadcn UI anyway because of the rest of the value.

if we end up pursuing a future in which we don't force consumers to use a styling solution (i.e. offer static CSS files consumers can load),

Right, yeah, I don't think Pigment CSS, in its current state with some of the bundlers supports, can replace emotion in Material UI, so it can't truly support it as JSS did. Pigment CSS either needs to have a. .css output mode, b. ALL bundlers supported (sounds not possible), or c. a smart runtime (a successor to emotion) or d. Material UI to support other styling engines.

how are you going to offer consumers a way to hide/show stuff that supports responsive values?

  1. We could leave this up to the user. Propose them to use something CSS variables based, e.g. https://open-props.style/ API.
  2. We could do 1. but built-in.
  3. We could leave this up to the user. Propose them to use Tailwind CSS.
  4. We could do 3. but built-in.
  5. We have them use or styling engine that supports sx
  6. We generate a few class names ahead of time: #7925. Not very versatile but works.
  7. I'm out of idea 😄?

I don't like using an extra dom element or React.cloneElement() 1, maybe is the least bad option if we don't want to change our model for building layouts.

True, the least bad option reasoning is a valid way to look at it.

I'm fine with removing Hidden, but let's dig into this topic more becase there are a lot of interrelated things we need to take into account.

I personally don't mind either direction. The origin of this component: https://github.com/mui/material-ui/issues/6726#issuecomment-298162706.