passing empty object to override palette theme sub-object throws error

[bmueller-sykes]

• [x ] This is a v1.x issue.
• [ x] I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

When creating a theme, I would expect that I could do this:

export const sykesTheme = createMuiTheme({
	palette: {
	    primary: {}
	}
})

...and basically have nothing happen (e.g. the empty object overrides no default values, and everything works.

Current Behavior

Steps to Reproduce

If I create a theme like the above, I get the following warnings/errors:

Warning: Material-UI: missing color argument in lighten(undefined, 0.2).
Material-UI: missing color argument in darken(undefined, 0.30000000000000004).
colorManipulator.js:109 Uncaught TypeError: Cannot read property 'charAt' of undefined
    at decomposeColor (colorManipulator.js:109)

Context

Obviously, this isn't an end-of-the-world issue, but I can see developers wanting to create empty objects in their theme files to help remind them of how a theme is structured, and then adding to that theme structure over time.

Perhaps this is working exactly as intended, in which case, close this and ignore.

Thanks!!

Your Environment

Tech	Version
Material-UI	v1.4.2?
React	16.4.1

[oliviertassinari]

@bmueller-sykes Sorry, I don't understand. Could you explain in more detail your use case for writing such code:

export const sykesTheme = createMuiTheme({
	palette: {
	    primary: {}
	}
})

[bmueller-sykes]

Sure! The default theme is pretty complicated, and I doubt a developer is likely to remember each and every piece of it (I know I won't!). But imagine a developer knows she's going to modify the theme over time, so she looks at this page:

https://material-ui.com/customization/default-theme/

...and decides she's likely to modify the palette over time, but she's not quite ready to do so yet. But to prep for that, she creates her own theme, and does this:

export const sykesTheme = createMuiTheme({
	palette: {
	    primary: {
              main: myMainColor,
              dark: myDarkColor,
              light: myLightColor,
              contrastText: myContrastText
            },
      secondary: {
        //main: myMainColor,
        //dark: myDarkColor,
        //light: myLightColor,
        //contrastText: myContrastText
      },
     error: {}
  }
})

...or something like that, so she can easily remember what the variables are when she's ready to go. I realize that she could just as easily do:

/*
      secondary: {
        //main: myMainColor,
        //dark: myDarkColor,
        //light: myLightColor,
        //contrastText: myContrastText
      },
     error: {}
*/

...which is why I said it wasn't that big of a deal. But, given that Object.assign gets used all over the React world, and it's legal to merge an empty object into another object, I was just surprised that what I did threw an error.

Does that answer your question?

Thanks again!! The library is great!!

[oliviertassinari]

@bmueller-sykes Thanks. What would you expect the library to do? Warn about it?

[bmueller-sykes]

If it were up to me, I'd say the library shouldn't do anything, except merge the empty object to the default object. I don't think a warning is necessary or required, since this might well represent a totally legitimate use-case.

[oliviertassinari]

except merge the empty object to the default object.

I disagree, it would require additional code and make type checking harder. I think that we should move with a better warning message.

[bmueller-sykes]

works for me!