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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rule option "disallowedFor" for component/dom element prop #2946

Closed
germain-gg opened this issue Mar 15, 2021 · 10 comments 路 Fixed by #3338
Closed

Rule option "disallowedFor" for component/dom element prop #2946

germain-gg opened this issue Mar 15, 2021 · 10 comments 路 Fixed by #3338

Comments

@germain-gg
Copy link

germain-gg commented Mar 15, 2021

Hi all 馃憢

First off thank you so much for this plugin, it has helped me many times, and saved me countless hours of debugging

I would like to put forward an idea to extend the following two rules

  • react/forbid-component-props
  • react/forbid-dom-props

The former accepts an allowedFor option, and I would like to introduce its counterpart dissalowedFor.
It would be particularly interesting for the react/forbid-dom-props rule as the Web platform constantly evolves, and even though it is rare, they sometimes deprecate old attributes.

To give you an example, the <form> tag used to receive an accept argument, however it is now deprecated and that attribute should be set on an <input type="file">.

There are no ways at the moment in eslint-plugin-react to express that. I would be happy to implement that directly but wanted to create this issue first to gather feedback from the community and settled on the proper syntax, naming and behavioura

Syntax

{
  "propName": "accept"
  "disallowedFor": [form],
  "message": "Avoid using the accept attribute on <form>"
}

To ensure backwards compatibility I believe the allowedFor option should take precendance over the disallowedFor option in the case of the following example, only allowedFor would be applied and a warning would be added to the terminal to notify the developer that they have conflicting rules configuration

Edit: as @ljharb pointed out, the following schema would not be valid as the two options would be mutually exclusive

{
  "propName": "accept"
  "disallowedFor": [form],
  "allowedFor": [input]
  "message": "Avoid using the accept attribute on <form>"
}
@ljharb
Copy link
Member

ljharb commented Mar 15, 2021

We wouldn't do a runtime warning like that; the schema would mark the options as mutually exclusive.

That said, your use case seems only for DOM props. What would be the use of it on custom components, which can use PropTypes to forbid specific props?

@germain-gg
Copy link
Author

Thank you for the feedback @ljharb , I have updated the original issue to notify that the two options would be mutually exclusive

You are correct, my use case covers exclusively the DOM props. However I assumed that the logic would be vastly shared between the two rules and that we could kill two birds with one stone. Happy to only implement it where it is needed now

Let me know your thoughts

@greyscaled
Copy link

Bump - We have a use-case where we'd like to be able to ban type="hidden" on <input> elements. That seems like it would be an extension to DOM props

@ljharb
Copy link
Member

ljharb commented May 26, 2021

I think it'd be reasonable to add this option for HTML elements only.

@TildaDares
Copy link
Contributor

I'd like to work on this!

@ljharb
Copy link
Member

ljharb commented Jul 13, 2022

Go for it!

@jacketwpbb
Copy link

I think it'd be reasonable to add this option for HTML elements only.

@ljharb I would like to forbid deprecated props for specific Components, which need disallowedFor option for component.

@ljharb
Copy link
Member

ljharb commented Sep 7, 2022

Why? The component itself can do that with its propTypes.

@jacketwpbb
Copy link

@ljharb Components are written by TS without propTypes, is there any rule can be used in this case

@ljharb
Copy link
Member

ljharb commented Sep 7, 2022

Components should always have propTypes even if they have type annotations, since propTypes are much more powerful.

If TS types prevent you from using a deprecated prop, why do you need a lint rule for it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

5 participants