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

inputProps and InputProps, no duplicate properties #8232

Closed
1 task done
codepb opened this issue Sep 16, 2017 · 7 comments
Closed
1 task done

inputProps and InputProps, no duplicate properties #8232

codepb opened this issue Sep 16, 2017 · 7 comments
Labels
discussion out of scope The problem looks valid but we won't fix it (maybe we will revisit it in the future)

Comments

@codepb
Copy link

codepb commented Sep 16, 2017

providing both inputProps and InputProps raises warning on compilation

  • I have searched the issues of this repository and believe that this is not a duplicate.

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 oliviertassinari added the status: waiting for author Issue with insufficient information label Sep 16, 2017
@oliviertassinari
Copy link
Member

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
Copy link
Member

oliviertassinari commented Sep 16, 2017

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

@codepb
Copy link
Author

codepb commented Sep 16, 2017

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
Copy link
Member

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 oliviertassinari added discussion v1 out of scope The problem looks valid but we won't fix it (maybe we will revisit it in the future) and removed status: waiting for author Issue with insufficient information labels Sep 16, 2017
@oliviertassinari
Copy link
Member

@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
Copy link
Author

codepb commented Sep 16, 2017

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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion out of scope The problem looks valid but we won't fix it (maybe we will revisit it in the future)
Projects
None yet
Development

No branches or pull requests

3 participants