[Bug] Wrong theme.shape.borderRadius definition in d.ts

[totszwai]

Duplicates

• I have searched the existing issues

Latest version

• I have tested the latest version

Steps to reproduce 🕹

Link to live example:

Steps:

Current behavior 😯

According to the MUI v5 migration documentation, borderRadius is supposed to be able to take number or string.

https://mui.com/material-ui/migration/v5-component-changes/#%E2%9C%85-update-borderradius-prop-value

However, the definition provided by the latest Mui only takes number.

Expected behavior 🤔

Expect shape.borderRadius to take string.

Note: It should accept also 123rem and not just pixel

Context 🔦

No response

Your environment 🌎

npx @mui/envinfo

  System:
    OS: Linux 5.15 Ubuntu 22.04.1 LTS 22.04.1 LTS (Jammy Jellyfish)
    CPU: (16) x64 Intel(R) Xeon(R) Silver 4108 CPU @ 1.80GHz
    Memory: 10.81 GB / 15.33 GB
    Container: Yes
    Shell: 5.1.16 - /bin/bash
  Binaries:
    Node: 16.15.1 - ~/.config/nvm/versions/node/v16.15.1/bin/node
    npm: 8.11.0 - ~/.config/nvm/versions/node/v16.15.1/bin/npm
  Managers:
    Apt: 2.4.8 - /usr/bin/apt
    Maven: 3.6.3 - /usr/bin/mvn
    pip2: 20.3.4 - ~/.local/bin/pip2
    pip3: 22.0.2 - /usr/bin/pip3
  Utilities:
    Make: 4.3 - /usr/bin/make
    GCC: 11.3.0 - /usr/bin/gcc
    Git: 2.34.1 - /usr/bin/git
  Virtualization:
    Docker: 20.10.12 - /usr/bin/docker
  IDEs:
    Nano: 6.2 - /usr/bin/nano
    VSCode: 1.73.1 - /snap/bin/code
    Vim: 8.2 - /usr/bin/vim
  Languages:
    Bash: 5.1.16 - /usr/bin/bash
    Java: 1.8.0_352 - /usr/bin/javac
    Perl: 5.34.0 - /usr/bin/perl
    Python: 2.7.18 - /usr/bin/python
    Python3: 3.10.6 - /usr/bin/python3
  Browsers:
    Chrome: 106.0.5249.119
    Chromium: 107.0.5304.87

[samkevin1]

I would like to work on this one.

[ZeeshanTamboli]

@totszwai The type definition you are pointing out is for theme.shape.borderRadius which is not the type of borderRadius system prop mentioned in the migration guide.

As stated in the docs, if the borderRadius system prop is a number it will multiply with the theme.shape.borderRadius which is why the type of theme's borderRadius is just a number to support multiplication. See issue #32960. The PR I had created to fix the issue #34076.

Under the hood, it looks like Mui does take string but is just that the typescript definition provided by Mui is incorrect.

Okay, this is something that we have to check about it evaluating string values but we will also have to consider #32960.

[siriwatknp]

Agree with @ZeeshanTamboli, the migration docs refers to the sx prop, not the theme input.

[totszwai]

@siriwatknp but it was the breaking change, we had string value rem value set and it was working fine. Now, after the breaking change introduced by Mui, we cannot apply a string rem value system-wide anymore.

We don't want to use sx props as it is not free and it has a performance penalty.

My point is that, Mui should not impose px value on borderRadius it should also accept rem (in fact, any legit CSS string value).

@ZeeshanTamboli If what I pointed out is wrong, then please point me out to the correct definition that borderRadius accepts number | string, because it is stated in the migration documentation above, that it takes number and string and it is not taking string right now.

[totszwai]

@ZeeshanTamboli I am just following the import definitions, provided by Mui, from VCode. As mentioned before, we were using string value (ex: '0.5rem') just fine before upgrading and we choose to use rem because it scales well with screen any resolution. We just need to change the root em for a specific resolution and everything scaled off the rem.

Starting from @mui/material/styles:

[ZeeshanTamboli]

because it is stated in the migration documentation above, that it takes number and string and it is not taking string right now.

The migration doc above is for sx prop which accepts string | number. And the types you are pointing out to (the screenshots posted) is for the theme input theme.shape.borderRadius used for multiplication with the sx value.

Can you provide a CodeSandbox how the rem string value used to work for theme.shape.borderRadius in Material-UI v4?

[siriwatknp]

@totszwai Thanks for the feedback. To make it supports the string, I think we will need to think about sx prop but I can see the possibility:

// theme
{
  shape: { borderRadius: '2px' },
}

// the sx should transform using CSS calc instead
sx: { borderRadius: 3 }, // borderRadius: calc(2px * 3);

Until it is possible, you will have to continue with your workaround.

[totszwai]

@siriwatknp I'm sorry, I was mistaken, my workaround actually didn't work, I mean while it cancels out the compilation error, it doesn't look like the settings were applied.
But here is a sandbox to prove that sting value with rem used to work just fine in Mui 5.7.

@ZeeshanTamboli here is the sandbox and is not from v4 either, it used to work just fine in version 5.7.
Just uncomment the borderRadius to see it affect the round corner of the button (in fact everything in Mui)

https://codesandbox.io/s/rem-used-to-work-in-5-7-je5em1?file=/demo.tsx

I also found another problem is that, the "system's setting" is overriding the settings we have at root level, for example, I had ButtonBase's root set to 0.2222rem, but then the default shape.borderRadius (if I did not provide anything) is overriding my setting, which should not be the case.

[ZeeshanTamboli]

@totszwai Thanks for the reproduction. As said before, we had to support multiplication with the sx prop borderRadius value so we had removed the string type. But since the string value is getting evaluated on the theme level (as shown in your reproduction), we can consider #35156 (comment) as a possible solution for the sx prop.

Until then, please use // @ts-expect-error comment till we update the types and also change the underlying implementation.

I also found another problem is that, the "system's setting" is overriding the settings we have at root level, for example, I had ButtonBase's root set to 0.2222rem, but then the default shape.borderRadius (if I did not provide anything) is overriding my setting, which should not be the case.

Is this related to this issue? If not, please open a new issue with a reproduction.

[marc0n3]

Hi,
what about adopting a solution similar to SpacingOptions used in the spacing prop?

export type SpacingOptions = number | Spacing | ((abs: number) => number | string) | ((abs: number | string) => number | string) | ReadonlyArray<string | number>;

By doing so, if one needs the raw multiplication, he use the number.
If one wants more control, he will do something like:
spacing: (factor: number) =>`${0.5 * factor}rem`,
but for borderRadius prop.