Can't use Box to do one off style overrides of external components when there are props collisions

[jonenst]

Duplicates

• I have searched the existing issues

Latest version

• I have tested the latest version

Steps to reproduce 🕹

Link to live example:
https://codesandbox.io/s/resizablebox-prop-collision-cedc3v?file=/src/App.tsx

Current behavior 😯

using

<Box component={MyBox /* MyBox does use the className prop */}
myprop={42 /* myprop is some custom prop of MyBox*/}
sx={ /*using sx to get theme values, or to compose styles, or for consistency.. */
  theme => ({backgroundColor: theme.foo.bar})
}
>
 ...
</Box>

works except if myExternalProp happens to be one of the system prop (it gets swallowed). As an exemple, imagine a CountryBox which take the flag and border props. flag works, but border collides with css borders. In the example, it's ResizableBox from 'react-resizable'. The 'resizeHandle' prop works but the 'width' prop doesn't.

Expected behavior 🤔

Should be able to use pass any props to the component, because system props should be in sx, not in props of Box

Context 🔦

I want to use the mui styling system (to have inheritence, composition, theme access, etc) to do a one off customization of a component accepting a className, without adding dom elements and using nested selectors.

One way to do it is to use styled but this requires creating an intermediate component..

const MyBoxSx = styled(MyBox)({}); /* MyBox does use the className prop */
<MyBoxSx
myprop={42 /* myprop is some custom prop of MyBox*/}
sx={ /*using sx to get theme values, or to compose styles, or for consistency.. */
  theme => ({backgroundColor: theme.foo.bar})
}

Your environment 🌎

No response

[ZeeshanTamboli]

Can you provide a proper reproduction in the CodeSandbox of the issue you are fixing? I see that the props are getting applied to the Box component in the KO example:

      <Box
        component={ResizableBox}
        sx={{
          backgroundColor: (theme) =>
            theme.palette.mode === "light" ? "gold" : "blue"
        }}
        width={50}
        height={50}
      >
        <span>Contents</span>
      </Box>

[github-actions]

Since the issue is missing key information and has been inactive for 7 days, it has been automatically closed. If you wish to see the issue reopened, please provide the missing information.

[jonenst]

Hi,
sorry for not replying sooner, I didn't understand what you meant. Below is video of the reproduction. The width and height props are not forwarded to the external ResizableBox, so it crashes and doesn't implement the dragndrop behavior. You can see this in the console

Warning: Failed prop type: The prop `height` is marked as required in `Resizable`, but its value is `undefined`.
Resizable@https://cedc3v.csb.app/node_modules/react-resizable/build/Resizable.js:30:35

Warning: Failed prop type: The prop `width` is marked as required in `Resizable`, but its value is `undefined`.
Resizable@https://cedc3v.csb.app/node_modules/react-resizable/build/Resizable.js:30:35

Here's a short video showing this:

muisxpropsswallow-2023-06-22_09.21.15.mp4

As I said earlier, this is because some "business" prop names of the external component collide with "system" props. Here it's width and height, but like I said, if it were a component representing a country, its props could be "flag" and "border" but border happens to collide with the css border prop.

[ZeeshanTamboli]

Thanks for opening an issue. This is an interesting use-case.

The Box component's props include all the props of the system prop (sx) directly, as mentioned in the documentation. This is why external props with the same name as sx properties are overridden.

To support sx in a custom component that is not part of MUI, you can follow the instructions provided in the following link: https://mui.com/system/getting-started/custom-components/#using-sx-with-custom-components. However, this also creates a intermediate component because of styled. I don't think there is any other way possible if you want to use the sx prop to do one-off customization for this.

Link to CodeSandbox - https://codesandbox.io/s/resizablebox-prop-collision-forked-s7pf6x?file=/src/App.tsx.

[jonenst]

Hi @ZeeshanTamboli,
thank you for your time looking into this. What is the difference in behavior between your example and mine (passing unstable_styleFunctionSx instead of an empty object to styled()() ) ?

Also, my goal was indeed to avoid styled for a one off customization. I saw in the docs that the Box component's props include all the props of the system prop (sx) directly, so I understood why the code behaved like it did. I just wanted to know if there was a workaround. And also to discuss the consequences of this design choice of duplicating the sx props in the components props.

[ZeeshanTamboli]

What is the difference in behavior between your example and mine (passing unstable_styleFunctionSx instead of an empty object to styled()() ) ?

There is no advantage in my example over yours. That was just another way to do this. I think your example showcasing empty object passed to styled is better. Also now I noticed that my example shown in CodeSandbox is incorrect because the styled is used from @mui/system which already has the sx prop available and thus there is no need for unstable_styleFunctionSx. I noticed that the styled used in the docs is from styled-components which does not have sx prop and so to support sx, unstable_styleFunctionSx is added. Sorry for that.

I saw in the docs that the Box component's props include all the props of the system prop (sx) directly, so I understood why the code behaved like it did.

Note that this is only for "CSS" utility components like Box, Stack and Grid and not for other MUI components. The other components have to pass the sx prop.

also to discuss the consequences of this design choice of duplicating the sx props in the components props.

How would we identify that other non-MUI components, such as ResizeableBox, have identical props with the same names as the sx prop?

Using styled is the only workaround I could come up with, unless someone else has a better solution. cc @mnajdova @siriwatknp @DiegoAndai

[jonenst]

How would we identify that other non-MUI components, such as ResizeableBox, have identical props with the same names as the sx prop?

From what I understand, the only alternative would have been to not have sx props as direct props in Box. I think I read somewhere in an another issue that sx on Box (and other utilities) was added later on and originally Box only allowed direct props.
So maybe this means there should be a component used to do one off wrapping of non-mui components that only takes props in separate toplevel props to avoid name clashes ? Not sure how all this fits in the global design you implemented:

(<Box component={ExtBox} sx={mystyles} {...myprops} />) // sets the classname but clashes on system names 
(<Bix component={ExtBox} sx={mystyles} wrappedProps={myprops} />)  // same without clash ?

What are the downsides of this ?

[ZeeshanTamboli]

So maybe this means there should be a component used to do one off wrapping of non-mui components that only takes props in separate toplevel props to avoid name clashes ?

We could do this:

(<Box component={ExtBox} sx={mystyles} wrappedProps={myprops} />)  // same without clash ?

Also take a look at the API tradeoff documentation.

I added this issue to #34883 for tracking purposes. It might be related to another issue listed in #34883.