mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.87k stars 32.26k forks source link

inputProps and InputProps, no duplicate properties #8232

Closed codepb closed 7 years ago

codepb commented 7 years ago

providing both inputProps and InputProps raises warning on compilation

Expected Behavior

No warning on compilation. Have different names for the two properties so it's not clashing, and case isn't required. (perhaps htmlInputProps and inputProps)

Current Behavior

If you provide both an inputProps and an InputProps property on a single component there is a warning on compilation that you are supplying duplicate properties as case is ignored.

Steps to Reproduce (for bugs)

provide both inputProps and InputProps

Context

It's confusing, and having warnings on compilation is less than desirable

Your Environment

Tech Version
Material-UI 1.0.0-beta.10
React 15.6.1
oliviertassinari commented 7 years ago

Your issue has been closed because it does not conform to our issue requirements. Please provide a reproduction test case. This would help a lot 👷 . A live example would be perfect. This codesandbox.io template may be a good starting point. Thank you!

oliviertassinari commented 7 years ago

P.S. This issue sounds linked to your bundling pipeline. It's working fine with the documentation one.

codepb commented 7 years ago

The issue is with duplicate props. It is confusing to have two properties with the same name but different capitalisation doing different things. I have not included a codesandbox example because it is more a point about a confusing API rather than an issue.

I would strongly recommend that all properties start with lowercase as is the convention in react. Requiring InputProps to start with a capital letter does not follow the usual conventions, and having different behaviour for inputProps vs InputProps is confusing.

I don't want to disable the warning, it is in fact correct. It would be preferable if material-ui made a clearer distinction between the two properties.

oliviertassinari commented 7 years ago

I don't want to disable the warning, it is in fact correct. It would be preferable if material-ui made a clearer distinction between the two properties.

I'm not sure if this is something we will move forward to. The naming rule is the following. We prefix Props with the name of the component. In our case, we have one component named input and another one named Input. I don't want to introduce an exception to the rule, then it comes to finding a better wording for our Input component. This is already a tradeoff to reduce the length of the name of the component.

oliviertassinari commented 7 years ago

@codepb So, I completely understand that the situation isn't ideal, it's the consequences of tradeoffs. I can't see any better configuration with those constraints. Thanks for raising this point!

codepb commented 7 years ago

React could do a similar thing though, but instead references html properties by prefixing with html where appropriate. I would suggest that is a better convention to differentiate the properties, as it has more clarity. htmlInputProps references the html element, inputProps references the component. I'm happy to agree to disagree, but I don't see anywhere else where capitalization is used to differentiate in properties, only in component names.

adam-nielsen commented 6 years ago

Because this issue comes up as an early Google result, I'm going to add a link to #10064 which includes a nice workaround:

<TextField
  InputProps={{
    bar: true,
    inputProps: {
      foo: true,
    },
  }}

Seems like deprecating inputProps on the TextField wouldn't be a bad thing as this "workaround" actually makes a bit more sense to me.

dondmcg commented 3 months ago

So this confusing naming convention still exists. Deprecation and introducing new naming convention was never implemented I guess.