[theme] Warn when palette structure is wrong (#20253)

mui · Mar 24, 2020 · 32d647c · 32d647c
1 parent 81b1cd8
commit 32d647c
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 59 deletions.
diff --git a/packages/material-ui/src/styles/createPalette.js b/packages/material-ui/src/styles/createPalette.js
@@ -132,12 +132,6 @@ export default function createPalette(palette) {
   // Bootstrap: https://github.com/twbs/bootstrap/blob/1d6e3710dd447de1a200f29e8fa521f8a0908f70/scss/_functions.scss#L59
   // and material-components-web https://github.com/material-components/material-components-web/blob/ac46b8863c4dab9fc22c4c662dc6bd1b65dd652f/packages/mdc-theme/_functions.scss#L54
   function getContrastText(background) {
-    if (!background) {
-      throw new TypeError(
-        `Material-UI: missing background argument in getContrastText(${background}).`,
-      );
-    }
-
     const contrastText =
       getContrastRatio(background, dark.text.primary) >= contrastThreshold
         ? dark.text.primary
@@ -159,21 +153,42 @@ export default function createPalette(palette) {
     return contrastText;
   }
 
-  function augmentColor(color, mainShade = 500, lightShade = 300, darkShade = 700) {
+  const augmentColor = (color, mainShade = 500, lightShade = 300, darkShade = 700) => {
     color = { ...color };
     if (!color.main && color[mainShade]) {
       color.main = color[mainShade];
     }
 
-    if (process.env.NODE_ENV !== 'production') {
-      if (!color.main) {
-        throw new Error(
-          [
-            'Material-UI: the color provided to augmentColor(color) is invalid.',
-            `The color object needs to have a \`main\` property or a \`${mainShade}\` property.`,
-          ].join('\n'),
-        );
-      }
+    if (!color.main) {
+      throw new Error(
+        [
+          'Material-UI: the color provided to augmentColor(color) is invalid.',
+          `The color object needs to have a \`main\` property or a \`${mainShade}\` property.`,
+        ].join('\n'),
+      );
+    }
+
+    if (typeof color.main !== 'string') {
+      throw new Error(
+        [
+          'Material-UI: the color provided to augmentColor(color) is invalid.',
+          `\`color.main\` should be a string, but \`${JSON.stringify(
+            color.main,
+          )}\` was provided instead.`,
+          '',
+          'Did you intend to use one of the following approaches?',
+          '',
+          'import { green } from "@material-ui/core/colors";',
+          '',
+          'const theme1 = createMuiTheme({ palette: {',
+          '  primary: green,',
+          '} });',
+          '',
+          'const theme2 = createMuiTheme({ palette: {',
+          '  primary: { main: green[500] },',
+          '} });',
+        ].join('\n'),
+      );
     }
 
     addLightOrDark(color, 'light', lightShade, tonalOffset);
@@ -183,7 +198,7 @@ export default function createPalette(palette) {
     }
 
     return color;
-  }
+  };
 
   const types = { dark, light };
 

diff --git a/packages/material-ui/src/styles/createPalette.test.js b/packages/material-ui/src/styles/createPalette.test.js
@@ -5,14 +5,6 @@ import { darken, lighten } from './colorManipulator';
 import createPalette, { dark, light } from './createPalette';
 
 describe('createPalette()', () => {
-  beforeEach(() => {
-    consoleErrorMock.spy();
-  });
-
-  afterEach(() => {
-    consoleErrorMock.reset();
-  });
-
   it('should create a palette with a rich color object', () => {
     const palette = createPalette({
       primary: deepOrange,
@@ -85,26 +77,11 @@ describe('createPalette()', () => {
       pink.A400,
     );
     expect(palette.text, 'should use dark theme text').to.equal(dark.text);
-    expect(consoleErrorMock.callCount()).to.equal(0);
-  });
-
-  it('logs an error when an invalid type is specified', () => {
-    createPalette({ type: 'foo' });
-    expect(consoleErrorMock.callCount()).to.equal(1);
-    expect(consoleErrorMock.messages()[0]).to.match(
-      /Material-UI: the palette type `foo` is not supported/,
-    );
   });
 
   describe('augmentColor', () => {
     const palette = createPalette({});
 
-    it('should throw when the input is invalid', () => {
-      expect(() => {
-        palette.augmentColor({});
-      }).to.throw(/The color object needs to have a/);
-    });
-
     it('should accept a color', () => {
       const color1 = palette.augmentColor(indigo);
       expect(color1).to.deep.include({
@@ -135,20 +112,37 @@ describe('createPalette()', () => {
     });
   });
 
-  describe('getContrastText', () => {
-    it('throws an exception with a falsy argument', () => {
-      const { getContrastText } = createPalette({});
+  it('should create a palette with unique object references', () => {
+    const redPalette = createPalette({ background: { paper: 'red' } });
+    const bluePalette = createPalette({ background: { paper: 'blue' } });
+    expect(redPalette).to.not.equal(bluePalette);
+    expect(redPalette.background).to.not.equal(bluePalette.background);
+  });
+
+  describe('warnings', () => {
+    beforeEach(() => {
+      consoleErrorMock.spy();
+    });
 
-      [
-        [undefined, 'missing background argument in getContrastText(undefined)'],
-        [null, 'missing background argument in getContrastText(null)'],
-        ['', 'missing background argument in getContrastText()'],
-        [0, 'missing background argument in getContrastText(0)'],
-      ].forEach((testEntry) => {
-        const [argument, errorMessage] = testEntry;
+    afterEach(() => {
+      consoleErrorMock.reset();
+    });
 
-        expect(() => getContrastText(argument), errorMessage).to.throw();
-      });
+    it('throws an exception when an invalid type is specified', () => {
+      createPalette({ type: 'foo' });
+      expect(consoleErrorMock.callCount()).to.equal(1);
+      expect(consoleErrorMock.messages()[0]).to.include(
+        'Material-UI: the palette type `foo` is not supported',
+      );
+    });
+
+    it('throws an exception when a wrong color is provided', () => {
+      expect(() => createPalette({ primary: '#fff' })).to.throw(
+        'The color object needs to have a `main` property or a `500` property.',
+      );
+      expect(() => createPalette({ primary: { main: { foo: 'bar' } } })).to.throw(
+        '`color.main` should be a string, but `{"foo":"bar"}` was provided instead.',
+      );
     });
 
     it('logs an error when the contrast ratio does not reach AA', () => {
@@ -164,11 +158,4 @@ describe('createPalette()', () => {
       );
     });
   });
-
-  it('should create a palette with unique object references', () => {
-    const redPalette = createPalette({ background: { paper: 'red' } });
-    const bluePalette = createPalette({ background: { paper: 'blue' } });
-    expect(redPalette).to.not.equal(bluePalette);
-    expect(redPalette.background).to.not.equal(bluePalette.background);
-  });
 });