Material-ui: Eslint giving error for duplicate props

Created on 27 Jan 2018  路  16Comments  路  Source: mui-org/material-ui

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.

Expected Behavior

Eslint or any other linting tool doesn't give me any duplicate error.(react/jsx-no-duplicate-props).

Current Behavior

Eslint throwing duplicate prop error.

Plugin is located here.

Temporal solution

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.

Your Environment

| Tech | Version |
|--------------|---------|
| Material-UI | 1.0.0-beta30 |
| React | ^16.0.0 |
| eslint | ^4.8.0 |

TextField question

Most helpful comment

I have found the following solution to the problem: much cleaner:

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

All 16 comments

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 wheninput 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 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'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:
image
Screenshot of TextField documentation:
image

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.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

rbozan picture rbozan  路  3Comments

finaiized picture finaiized  路  3Comments

reflog picture reflog  路  3Comments

sys13 picture sys13  路  3Comments

ghost picture ghost  路  3Comments