This is not a bug request, but rather, it's a discussion on the reasons why you named 2 prop types of TextField
almost the same with one being upper case.
According to TextField API,
TextField
exposes two prop types with almost same names. InputProps
and inputProps
.
Currently, my Eslint gives me an error for having duplicate prop type.
Is there any reason to name 2 prop types almost the same?
Can't we rename 1 of them to something more clarifying?
This also gives headaches with linting tools.
Eslint or any other linting tool doesn't give me any duplicate error.(react/jsx-no-duplicate-props
).
Eslint throwing duplicate prop error.
Plugin is located here.
Yes, you can set ignoreCase
to false in .eslintrc
but isn't it better to name our props in a more definitive way?
Thank you.
| Tech | Version |
|--------------|---------|
| Material-UI | 1.0.0-beta30 |
| React | ^16.0.0 |
| eslint | ^4.8.0 |
Also, please note that Airbnb eslint style uses jsx-no-duplicate with ignore case of true.
please note that Airbnb eslint style uses jsx-no-duplicate with ignore case of true.
@mehrdaad This is key to the issue. https://github.com/airbnb/javascript/blob/53b2d7d245ba4abefc0429bfda4a46f099b9ace5/packages/eslint-config-airbnb/rules/react.js#L89
Can't we rename 1 of them to something more clarifying?
Input refers to the Input component when
input refers to the input
element.
Input refers to the Input component when input refers to the input element.
I've already read that in docs.
What I'm suggesting is rename one of the prop types for more clarification.
and also not getting an error from Airbnb style eslint rules.
@mehrdaad I have added the waiting for users upvotes
tag. I'm closing the issue as it unsure if changing the property names will make the situation better. So please upvote this issue if you think it would. Suggestions are even better! We will prioritize our effort based on the number of upvotes.
You can suppress this eslint warning for now.
I have found the following solution to the problem: much cleaner:
<TextField
- inputProps={{
- foo: true
- }}
InputProps={{
bar: true,
+ inputProps: {
+ foo: true,
+ },
}}
Add this to your .eslintrc.json
{
...
"rules": {
...
"react/jsx-no-duplicate-props": [1, { "ignoreCase": false }]
...
},
...
}
This will warn when there are two props of same name. Use 2
instead of 1
for error.
I think an even better solution is to have InputProps
do as what is expected, but change inputProps
to NativeInputProps
. That is far more descriptive as to what it does. Having it as the same name, but differing only by the case of the property is prone to errors and simple typos. Changing the native element props to NativeInputProps
keeps with the standard of having the properties staring with an upper case as well as makes it far more descriptive.
I think an even better solution is to have
InputProps
do as what is expected, but changeinputProps
toNativeInputProps
. That is far more descriptive as to what it does. Having it as the same name, but differing only by the case of the property is prone to errors and simple typos. Changing the native element props toNativeInputProps
keeps with the standard of having the properties staring with an upper case as well as makes it far more descriptive.
I've had this same problem and @nghtstr has proposed an ACTUAL solution to a real problem as opposed to ignoring the problem literally by having the linter ignore it and ignoring any testing warnings etc. Temporarily the the property could be copied and have two names with a deprecation warning on the old one and then on a future update finally remove it.
Checkbox
has inputProps
which in the documentation should behave like InputProps
on TextField
Screenshot of Checkbox documentation:
Screenshot of TextField documentation:
I see it this way: InputProps
on TextField
should be inputProps
and InputProps
should be inputAttributes
as the documentation describes anyways.
@oliviertassinari can we deprecate InputProps
and make it inputAttributes
or anything other than a duplicate prop??
@matthewjwhitney What about htmlInputProps
? (like the htmlFor
React API) cc @mui-org/core-contributors
doesn't matter to me as long as it's not a duplicate...
I quite like this suggestion: https://github.com/mui-org/material-ui/issues/10064#issuecomment-373190215 (as did more people than 馃憤 the original issue. So, perhaps we drop the prop?
I quite like this suggestion: #10064 (comment) (as did more people than the original issue. So, perhaps we drop the prop?
The code change looks pretty trivial. Although in my experience moving a prop to the inherited component means for most API readers it doesn't exist.
I quite like this suggestion: #10064 (comment)
@mbrookes This solution requires more effort to be found without landing on this issue. It's non-obvious. A developer needs to look at the Input properties API. It's why we have added inputProps
properties to the TextField component. @eps1lon How would you approach the problem?
@eps1lon How would you approach the problem?
It's important to understand that the text field is a simple abstraction on top of the following components:
We can't do much beyond that. If people skip the very first paragraph then recovering from that is hard.
change this in the api docs:
const inputProps = {
step: 300,
};
-return <TextField id="time" type="time" inputProps={inputProps} />;
+return <TextField id="time" type="time" InputProps={{ inputProps }} />;
Add a link to the Input
API for the InputProps
entry. We do this for all the other components too and it doesn't seem to be an issue for those component props.
Aside:
We could add a functionality to the props table that would expand the immediate props to all available props via prop forwarding. TypeScript users already get this for free via IntelliSense (although without prop description (yet)). Would be nice to see everything in one window instead of opening 2 or 3 tabs to see every possible prop.
Most helpful comment
I have found the following solution to the problem: much cleaner: