second review

Author: oliviertassinari

packages/material-ui/src/styles/createPalette.js

@@ -160,41 +160,35 @@ export default function createPalette(palette) {
     }
 
     if (!color.main) {
-      if (process.env.NODE_ENV !== 'production') {
-        console.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'),
-        );
-      }
-      color.main = indigo[500];
+      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') {
-      if (process.env.NODE_ENV !== 'production') {
-        console.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 intent to do one of the followings?',
-            '',
-            'import { green } from "@material-ui/core/colors";',
-            '',
-            'const theme1 = createMuiTheme({ palette: {',
-            '  primary: green,',
-            '} });',
-            '',
-            'const theme2 = createMuiTheme({ palette: {',
-            '  primary: { main: green[500] },',
-            '} });',
-          ].join('\n'),
-        );
-      }
-      color.main = indigo[500];
+      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);

packages/material-ui/src/styles/createPalette.test.js

@@ -112,23 +112,6 @@ describe('createPalette()', () => {
     });
   });
 
-  describe('getContrastText', () => {
-    it('throws an exception with a falsy argument', () => {
-      const { getContrastText } = createPalette({});
-
-      [
-        [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;
-
-        expect(() => getContrastText(argument), errorMessage).to.throw();
-      });
-    });
-  });
-
   it('should create a palette with unique object references', () => {
     const redPalette = createPalette({ background: { paper: 'red' } });
     const bluePalette = createPalette({ background: { paper: 'blue' } });
@@ -145,24 +128,19 @@ describe('createPalette()', () => {
       consoleErrorMock.reset();
     });
 
-    it('logs an error when an invalid type is specified', () => {
+    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('logs an error when a wrong color is provided', () => {
-      createPalette({ primary: '#fff' });
-      expect(consoleErrorMock.callCount()).to.equal(1);
-      expect(consoleErrorMock.messages()[0]).to.include(
+    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.',
       );
-
-      createPalette({ primary: { main: { foo: 'bar' }} });
-      expect(consoleErrorMock.callCount()).to.equal(2);
-      expect(consoleErrorMock.messages()[1]).to.include(
+      expect(() => createPalette({ primary: { main: { foo: 'bar' } } })).to.throw(
         '`color.main` should be a string, but `{"foo":"bar"}` was provided instead.',
       );
     });