Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prototypes are lost when set as theme defaults #12031

Closed
2 tasks done
remcohaszing opened this issue Jul 2, 2018 · 2 comments
Closed
2 tasks done

Prototypes are lost when set as theme defaults #12031

remcohaszing opened this issue Jul 2, 2018 · 2 comments
Labels
bug 🐛 Something doesn't work good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@remcohaszing
Copy link
Contributor

  • This is a v1.x issue (v0.x is no longer maintained).
  • I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

Default props in a theme keep their prototype.

Current Behavior

The prototype in theme props is removed due to the use of deepmerge.

Steps to Reproduce (for bugs)

The following works:

const popoverContainer = document.createElement('div');

<MuiThemeProvider
  theme={createMuiTheme({
    props: {
      MuiPopover: {
        container: () => popoverContainer,
      },
    },
  })}
>
  <App />
</MuiThemeProvider>

The following does not (but should):

const popoverContainer = document.createElement('div');

<MuiThemeProvider
  theme={createMuiTheme({
    props: {
      MuiPopover: {
        // The popover will receive a regular object
        container: popoverContainer,
      },
    },
  })}
>
  <App />
</MuiThemeProvider>

https://codesandbox.io/s/04j9lxymow (based on #12011 (comment))

  1. Pass an HTML element as a MuiPopover prop to <MuiThemeProvider />
  2. Render a <Popover />
  3. Notice the app crashes.

Context

https://material-ui.com/customization/themes/#properties

Your Environment

Tech Version
Material-UI v1.3.0
React v16.3.0
browser Chrome 67
deepmerge v2.1.1
@oliviertassinari
Copy link
Member

The prototype in theme props is removed due to the use of deepmerge.

@remcohaszing Looking at the options of deepmerge, I think that we should be ignoring non "plain object" : https://github.com/KyleAMathews/deepmerge#ismergeableobject.
This implementation is intereseting: https://github.com/jonschlinkert/is-plain-object/blob/master/index.js.

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work good first issue Great for first contributions. Enable to learn the contribution process. labels Jul 2, 2018
@kivlor
Copy link
Contributor

kivlor commented Jul 9, 2018

I've opened a WIP PR here - #12100

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

No branches or pull requests

3 participants