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

width is too common of a prop name for withWidth, can we make this prop name customizable? #11952

Closed
2 tasks done
w9 opened this issue Jun 22, 2018 · 3 comments
Closed
2 tasks done
Labels
good first issue Great for first contributions. Enable to learn the contribution process. new feature New feature or request

Comments

@w9
Copy link

w9 commented Jun 22, 2018

withWidth is a fantastic helper for building a markup structure that's responsive to the screen size. Currently, the prop supplied has a fixed name "width". Unfortunately, this name is extremely common in existing React components, and since the name "width" is currently hard-coded, there's no way to change it.

  • 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.
@oliviertassinari oliviertassinari added the new feature New feature or request label Jun 22, 2018
@Skaronator
Copy link
Contributor

muiWidth would be a good name IMO.

@franklixuefei
Copy link
Contributor

franklixuefei commented Jun 27, 2018

IMO this is a good issue. This actually reminds me of something related to HOC vs Render-Props pattern. This issue is a classic example of Naming Collision

Naming collisions. Two mixins that try to update the same piece of state may overwrite one another. The createClass API included a check that would warn you if two mixins had a getInitialState value with the same keys, but it wasn’t airtight.

https://cdb.reacttraining.com/use-a-render-prop-50de598f11ce

Perhaps in the future, we can start gradually trying out render props pattern as a replacement of HOC

@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 30, 2018

@w9 It's funny, you have a track record of using conflicting property names: cssinjs/theming#47.
We could definitely allow people to change the property name. Still, does it worth the effort when users control the properties name?

Perhaps in the future, we can start gradually trying out render props pattern as a replacement of HOC

@franklixuefei We have started documenting this pattern in https://material-ui.com/customization/css-in-js/#render-props-api-11-lines-. I think that documenting how to use the HOC as a render propertry would definitely be a good first step!

Also, considering withStyles, withWidth & withTheme have the same signature, we could make a factory to generate Render Prop components out of these HOCs (but given the simplicity of the approach, we might not need such abstraction, sometimes, it's better to repeat itself for a code easier to understand).

@oliviertassinari oliviertassinari added the good first issue Great for first contributions. Enable to learn the contribution process. label Jun 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Great for first contributions. Enable to learn the contribution process. new feature New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants