[Discussion] Support some (or maybe all?) non-integer values passed to theme.spacing()

[anikcreative]

N.B.: This issue is a continuation of the discussion in #29526, as requested by @siriwatknp and @oliviertassinari

---

Summary

Should we allow positive, non-integer values to be passed to theme.spacing()? And, if so, should we only allow "half-values" (e.g. 0.5, 1.5, 2.5)? Or should we allow all positive non-integer values (e.g. 0.05, 1.2, etc.)?

Original issue

In #29526, we addressed and resolved an issue (#29479) with non-integer values (e.g. 0.5, 1.2, etc.) being passed as parameters to theme.spacing(). Such values are invalid when spacing is an array, and their use in some MUI components was resulting in errors.

The fix was to replace all instances where MUI components were trying to pass decimal values to theme.spacing() with string interpolation and CSS calc().

So, something like this...

paddingLeft: theme.spacing(1.2),

...was changed to this:

paddingLeft: `calc(${theme.spacing(1)} * 1.2)`,

Current behavior

As per the documentation, spacing can be defined as:

• a number

createTheme({
  spacing: 2,
});

• a function

createTheme({
  spacing: (n) => `${0.25 * n}rem`,
});

• or an array

createTheme({
  spacing: [0, 4, 8, 16, 32, 64, 128, ...],
});

If spacing is defined as a number, non-integer decimal values can be safely passed to theme.spacing() and work as expected. The number in question is a coefficient. If, for instance, this coefficient is K, theme.spacing(1.2) calculates to 1.2 × K. The same can more or less be said for spacing when it is defined as a custom function, as shown above.

If, however, spacing is defined as an array, the parameters passed to theme.spacing() must be valid array indexes. Non-integer numbers are not valid indexes. When spacing is an array, and decimal values are provided to theme.spacing(), the values are ignored and an error message is displayed:

MUI: The `theme.spacing` array type cannot be combined with non integer values.You should either use an integer value that can be used as index, or define the `theme.spacing` as a number. 

The origin of this error message is another previous issue, #23187.

Suggested change

@siriwatknp suggested that, while the fix resolves the existing issue, it might make more sense to simply allow non-integer values to always be passed to theme.spacing(), even if spacing is an array. This would involve changing the logic in createSpacing.ts to do the calculation there, instead of forcing devs to write out the css calc() implementation each time.

2 solutions that I see

1. all of the components should never use theme.spacing(decimal) because we force the warning
2. remove the warning and fix createSpacing to support decimal if spacing array is specified.

option 1 will hurt us in the long term because we might miss it in other components and some people will open the same issues, so I favor option 2.

@oliviertassinari responded that non-integer parameters passed to theme.spacing() make sense for "half values" (e.g. 0.5, 1.5, 2.5), but not for any other values (e.g. 1.2, 2.35, 0.075)

👍 for supporting half values: e.g. 0.5, 1.5. It's very handy when you have to split the spacing in two between margin-top and margin-bottom. It's also easy to compute when spacing is an array, the logic can be: theme.spacing[value * 2] / 2.
👎 for supporting 1.2. I personally think that theme.spacing() could warn against (in all the cases). To me, it doesn't make sense. It's probably better for the developers to hard code the value or uses calc in this case.

[anikcreative]

I like the simplicity of the approach @oliviertassinari suggested (with regards to half values)

when spacing is an array, the logic can be: theme.spacing[value * 2] / 2.

It works as expected, but the results may be a little un-intuitive. Suppose spacing is defined as an array, like so:

createTheme({
  spacing: [0, 4, 8, 16, 32, 64, 128, 256],
});

Integer values passed to theme.spacing() are, effectively, array indexes. Therefore...

• ...theme.spacing(0) => theme.spacing[0] => 0
• ...theme.spacing(1) => theme.spacing[1] => 4
• ...theme.spacing(2) => theme.spacing[2] => 8
• ...theme.spacing(3) => theme.spacing[3] => 16

Now let's see what happens if we use the suggested formula for "half-values", namely: theme.spacing[value * 2] / 2

• ...theme.spacing(0.5) => theme.spacing[1] / 2 => 2
• ...theme.spacing(1.5) => theme.spacing[3] / 2 => 8
• ...theme.spacing(2.5) => theme.spacing[5] / 2 => 32
• ...theme.spacing(3.5) => theme.spacing[7] / 2 => 128

The specific use case mentioned makes a lot of sense.

It's very handy when you have to split the spacing in two between margin-top and margin-bottom.

Suppose some variable n determines the amount of margin or padding, which needs to be equally split between top and bottom. This approach allows us to easily do something like theme.spacing(n / 2) to get the split values. Whatever theme.spacing(n) would have yielded is simply split in half.

In other words, the following...

marginTop: ({ spacing }) => `calc(${spacing(7)} / 2)`,
marginBottom: ({ spacing }) => `calc(${spacing(7)} / 2)`,

...could instead simply become this:

marginTop: 3.5,
marginBottom: 3.5,

That said, it does seem a little strange that...

• ...theme.spacing(1.5) === theme.spacing(2)
• ...theme.spacing(2.5) > theme.spacing(3)
• ...theme.spacing(3.5) > theme.spacing(4)

I'm a little torn on this, but part of me thinks that theme.spacing(2.5), for instance, suggests a value that is "half-way" between theme.spacing(2), and theme.spacing(3). I wonder if this could be confusing to some devs. When spacing is defined as an array, I think of it as a "series of numbers", discrete points on a continuum. If I ask for a value at an "index" that is "between" the indexes of two neighboring points, why is the result not a value that is between its two nearest neighbors?

---

If we only allow positive integer values and positive "half-values", we will have to perform some kind of check and throw an error / warning when invalid values are passed in (in all cases, regardless of whether spacing is a number, function, or an array). This could be as simple as:

// Given spacing(value)...
if (Number.isInteger(value)) {
  // value is an integer; use current functionality...
}
if (Number.isInteger(value * 2)) {
  // value is a half-value; use the formula spacing(value * 2) / 2
}
// warn of invalid value; must be an integer or half-value

A few observations on Number.isInteger():

• Number.isInteger() is part of the ES6 standard and won't work on IE11.
• An expression like Number.isInteger(2.000000000000000000001) evaluates to true, because JavaScript thinks 2.000000000000000000001 == 2
• createUnarySpacing is currently using Number.isInteger(), as per this commit

[madflanderz]

I found this workaround that is good enough for me.
Until this problem is solved by MUI you can use this as well:

    /**
   * Due to a bug in MUI we have to use this function instead of the array like this:
   * spacing: [0, 4, 8, 16, 24, 32, 64, 128, 256, 512, 1024],
   *
   * Source:
   * https://github.com/mui/material-ui/issues/29677
   *
   * Reason:
   * Some components will use non-integer values for spacing and this breaks the spacing array.
   *
   * Solution:
   * Use a function instead of an array and calculate the spacing based on the factor
   *
   * Result:
   * theme.spacing(1)   // 4px
   * theme.spacing(1.5) // 6px
   * theme.spacing(2)   // 8px
   * theme.spacing(2.5) // 12px
   */
  spacing: (factor: number) => {
    const values = [0, 4, 8, 16, 24, 32, 64, 128, 256, 512, 1024, 2048];
    const index = Math.floor(factor);
    const currentSpace = values[index];
    const nextSpace = values[index + 1] || currentSpace * 2;
    const space = currentSpace + (nextSpace - currentSpace) * (factor - index);
    return `${space}px`;
  },

[allicanseenow]

Any official update on this?

[gerhat]

Thank you @madflanderz. Great workaround!