@material-ui/system spacing transformation rejects an array passed via theme

[michaelpward]

• The issue is present in the latest release.
• I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

Creating a custom theme with a property "spacing" whose value is an array results in a console.error.
The system "transformation" does not take place. Spacing behaves as it does with no custom theme present.

Setting the “spacing” property as an array per MUI documentation results in error in console:

index.js:1 Material-UI: the theme.spacing value (0,4,8,12,16,32) is invalid. It should be a number or a function.

Expected Behavior 🤔

As per MUI documentation, if the value of the "spacing" property in the theme is an array, the property is value is used as the array index.

Steps to Reproduce 🕹

Steps:

1. Create a theme object with a “spacing” property whose value is an array:

const theme = {
	spacing: [0, 4, 8, 12, 16, 20]
}

2. Pass that theme object to createMuiTheme(theme);
3. Console log shows the error described above.
4. Leveraging a system spacing prop <MyComponent p={2}/> sets padding at 2x8 = 16px when it should leverage the value of the 3rd element of the spacing array.

Context 🔦

I'm attempting to implement a constraint-based design leveraging a custom theme and what styled-system calls "scales" – which the MUI docs indicate should be possible via the spacing property in a custom theme.

Your Environment 🌎

Tech	Version
Material-UI	v4.9.4
React	v16.12
Browser	Crome v80.0.3987.122 (Official Build) (64-bit)

[oliviertassinari]

@michaelpward Thanks for the issue! There is definitely an inconsistency between

• https://material-ui.com/customization/spacing/#custom-spacing
• https://material-ui.com/system/spacing/#transformation

[oliviertassinari]

What do you think of this patch?

diff --git a/docs/src/pages/customization/spacing/spacing.md b/docs/src/pages/customization/spacing/spacing.md
index 1855cbb6f..460597387 100644
--- a/docs/src/pages/customization/spacing/spacing.md
+++ b/docs/src/pages/customization/spacing/spacing.md
@@ -38,7 +38,7 @@ theme.spacing(2); // = 0.25 * 2rem = 0.5rem = 8px

 ```js
 const theme = createMuiTheme({
-  spacing: factor => [0, 4, 8, 16, 32, 64][factor],
+  spacing: [0, 4, 8, 16, 32, 64],
 });

 theme.spacing(2); // = 8
diff --git a/packages/material-ui-system/src/spacing.js b/packages/material-ui-system/src/spacing.js
index c6aac3404..0e58c9cd6 100644
--- a/packages/material-ui-system/src/spacing.js
+++ b/packages/material-ui-system/src/spacing.js
@@ -74,11 +74,18 @@ const spacingKeys = [
   'paddingY',
 ];

-function getTransformer(theme) {
+export function getTransformer(theme) {
   const themeSpacing = theme.spacing || 8;

   if (typeof themeSpacing === 'number') {
-    return abs => themeSpacing * abs;
+    return abs => {
+      if (process.env.NODE_ENV !== 'production') {
+        if (typeof abs !== 'number') {
+          console.error(`@material-ui/system: expected spacing argument to be a number, got ${abs}.`);
+        }
+      }
+      return themeSpacing * abs;
+    };
   }

   if (Array.isArray(themeSpacing)) {
diff --git a/packages/material-ui/src/styles/createSpacing.js b/packages/material-ui/src/styles/createSpacing.js
index cbca65c94..375787370 100644
--- a/packages/material-ui/src/styles/createSpacing.js
+++ b/packages/material-ui/src/styles/createSpacing.js
@@ -1,3 +1,5 @@
+import { getTransformer } from '@material-ui/system/spacing';
+
 let warnOnce;

 export default function createSpacing(spacingInput = 8) {
@@ -6,32 +8,12 @@ export default function createSpacing(spacingInput = 8) {
     return spacingInput;
   }

-  // All components align to an 8dp square baseline grid for mobile, tablet, and desktop.
-  // https://material.io/design/layout/understanding-layout.html#pixel-density
-  let transform;
-
-  if (typeof spacingInput === 'function') {
-    transform = spacingInput;
-  } else {
-    if (process.env.NODE_ENV !== 'production') {
-      if (typeof spacingInput !== 'number') {
-        console.error(
-          [
-            `Material-UI: the \`theme.spacing\` value (${spacingInput}) is invalid.`,
-            'It should be a number or a function.',
-          ].join('\n'),
-        );
-      }
-    }
-    transform = factor => {
-      if (process.env.NODE_ENV !== 'production') {
-        if (typeof factor !== 'number') {
-          console.error(`Expected spacing argument to be a number, got ${factor}`);
-        }
-      }
-      return spacingInput * factor;
-    };
-  }
+  // Material Design layouts are visually balanced. Most measurements align to an 8dp grid applied, which aligns both spacing and the overall layout.
+  // Smaller components, such as icons and type, can align to a 4dp grid.
+  // https://material.io/design/layout/understanding-layout.html#usage
+  const transform = getTransformer({
+    spacing: spacingInput,
+  });

   const spacing = (...args) => {
     if (process.env.NODE_ENV !== 'production') {

Do you want to submit a pull request :)?

[michaelpward]

@oliviertassinari Thanks for the quick response and patch!

While my skills are rising rapidly, I don't think I'm at the level where I'm ready to submit PRs :(

I've hacked your patch in on my end and am not achieving the expected result. No matter what I do the padding always comes out as (value * 8). I cannot get it to use the value from the array.

I'll point out that this is the same when passing a function that should result in a different outcome. The function does not alter the outcome. I still get (value * 8). This is both with and without the patch.

I hope to be able to contribute more than just issues as I progress.