search

Giners / mui-places-autocomplete

Google Material Design (Material-UI) styled React component using Google Maps Places Autocomplete
MIT License
34 stars 26 forks source link

Use 'peerDependencies' avoid duplicating modules #7

Closed Giners closed 6 years ago

Giners commented 6 years ago

While incorporating the <MUIPlacesAutocomplete> component into another project of mine I was experiencing errors as mentioned in MUI issue https://github.com/mui-org/material-ui/issues/8742. Specifically during SSR I was seeing the following warnings:

Warning: Failed context type: Invalid context `64a55d578f856d258dc345b094a2a2b3` of type `Jss` supplied to `Component`, expected instance of `Jss`.
Warning: Failed context type: Invalid context `d4bd0baacbc52bbd48bbb9eb24344ecd` of type `SheetsRegistry` supplied to `Component`, expected instance of `SheetsRegistry`.

After reading the MUI issue I was able to resolve the issue by upgrading the MUI dependency in this project/package/module to match the version I use in my other project (MUI v1.0.0-beta.21) (commit: ebc08911db8120de2bcecaf47f2fd645b7a790dc). Yet I don't think/know if this is the correct approach to take long term. Especially as it may affect others who also use this project/package/module.

After reading more about the MUI issue previously mentioned it seems like that it might be better to setup peer dependencies for our project/package/module. This task is to learn more about what peer dependencies are and see if it using them is useful/will help us avoid this problem in the future. I'm presuming we would take one on MUI, if any, but there may be others.

Giners commented 6 years ago

While further working on incorporating <MUIPlacesAutocomplete> component into my other project I was experiencing additional errors after the the bundle was downloaded by the client and the virtual DOM was hydrated (i.e. ReactDOM.hydrate()). The error would manifest itself by MUI components losing their styling. Note that the styling was okay from the SSR but only after the virtual DOM was hydrated did the styling get lost.

The console was also showing us warnings in the following format:

Warning: Prop 'className' did not match. Server: "MuiInput-root-79" Client: ".MuiInput-root-13"

We would get a lot of warnings of this type for different classNames that were generated with MUI. This bug looks similar to behavior in https://github.com/mui-org/material-ui/issues/8522 but does seem to be different in that there isn't an ordering issue (see bug).

After talking to @oliviertassinari he narrowed down the problem to us duplicating modules. Upon further research it is obvious that peer dependencies are needed to prevent us from duplicating modules. As a result we are going to turn this into a bug.

oliviertassinari commented 6 years ago

Have you found who is this duplicated dependency?

Giners commented 6 years ago

I did. :confused: As mentioned I'm just learning and publishing packages is new to me. I'll have a PR up for this very shortly.

oliviertassinari commented 6 years ago

@Giners Great! Also, I plan on adding an escape hatch so people can choose to have predictable class names at the expense of potential clash if used in userland and performance.

Giners commented 6 years ago

@oliviertassinari That would be awesome!

I added you as a collaborator so you can review the PR. :)

Giners commented 6 years ago

Bug was resolved by commit: 7d982f3238d2340f627f20d6cadf4e68176fc2e3