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

jsx-boolean-value: enhance configuration to handle false literals #3234

Closed
alexmatson-da opened this issue Mar 5, 2022 · 11 comments · Fixed by #3675
Closed

jsx-boolean-value: enhance configuration to handle false literals #3234

alexmatson-da opened this issue Mar 5, 2022 · 11 comments · Fixed by #3675

Comments

@alexmatson-da
Copy link

alexmatson-da commented Mar 5, 2022

The react/jsx-boolean-value rule appears to only work with boolean props that have been assigned true literals. However, I have a codebase that sometimes contains (as an example) <MyComponent someBooleanProp={false} ... />

Is it possible to add a configuration option for this rule to flag these instances as props that should be removed completely (maybe even with autofix)?

If not, is there perhaps another rule that can help me achieve this?

Thanks!

@ljharb
Copy link
Member

ljharb commented Mar 5, 2022

Why would you want them removed completely? False and undefined are distinctly different, and it’s not possible to know from the consumer side whether the component is sloppily equating the two or not.

@alexmatson-da
Copy link
Author

While I'm sure they exist, off the top of my head I can't picture a use-case where a component intentionally exhibits different behavior depending on if an optional boolean prop passed into it is undefined rather than false.

In our case, optional boolean props get coerced into boolean values (!!boolProp), so boolProp={false} is really redundant.

We also use Typescript, so if we wanted a component with a required boolean prop, then it'd be typed as boolProp: boolean instead of boolProp?: boolean.

In the situations where that doesn't work (i.e., where the prop is required rather than optional), then I think it should be possible to pass an array of prop-names to be treated as exceptions from the rule, which is something react/jsx-boolean-value also does

@ljharb
Copy link
Member

ljharb commented Mar 5, 2022

Any time a boolean prop is required, omitting it is an error. Typescript would enforce this as well.

An eslint rule that allowed an optional false to be omitted also might not make sense, because sometimes a missing boolean defaults to true.

In other words, the assumption you want to rely on is just not possible to rely on, and isn’t even necessarily the majority case.

@ljharb
Copy link
Member

ljharb commented Mar 11, 2022

I don't think we'll pursue this, for the reasons indicated.

@ljharb ljharb closed this as completed Mar 11, 2022
@tylerlaprade
Copy link
Contributor

For what it's worth, I had exactly this issue and came here searching for the same fix as well. Is it possible to reconsider?

@ljharb
Copy link
Member

ljharb commented Jun 15, 2023

@tylerlaprade if the prop is a required prop, and is marked as a boolean (via propTypes and/or TS prop types), why would you need a rule here?

@tylerlaprade
Copy link
Contributor

tylerlaprade commented Jun 16, 2023

@ljharb In my project, we have agreed to use isFoo?: boolean | undefined for all boolean props and treat false the same as undefined so that we can use simpler shorthand syntax. This rule helps me with half of my required fixes for coding style:

<Bar isFoo={true}>    ->    <Bar isFoo>

but not the other half:

<Bar isFoo={false}>    ->    <Bar>

I agree that this second replacement is not correct unless a project has agreed to treat all boolean props in this manner, so it should not be the default behavior of the rule, but it would be useful to us as an option.

@ljharb
Copy link
Member

ljharb commented Jun 23, 2023

Hmm, that specific convention does make sense, but only when it's a boolean literal - because when it's a variable, you'd want to be able to do isFoo={x}.

As an option to this rule, it would only make sense when the mode is set to "never", so it'd need to be an option that only applies to "never" things. That seems kind of weird - what would such an option be named?

@tylerlaprade
Copy link
Contributor

As an option to this rule, it would only make sense when the mode is set to "never", so it'd need to be an option that only applies to "never" things. That seems kind of weird - what would such an option be named?

@ljharb, some possible option name suggestions if defaulting the option to false:

  • preferLiteralShorthand
  • avoidLiteralFalse
  • avoidExplicitFalse
  • falseIsUndefined
  • treatFalseAsUndefined

and some suggestions if defaulting the option to true:

  • preferExplicitFalse
  • preferLiteralFalse
  • falseIsNotUndefined

@ljharb
Copy link
Member

ljharb commented Jun 23, 2023

it's more like assumeUndefinedIsFalse, i think? (boolean options should always default to false)

@tylerlaprade
Copy link
Contributor

Yeah, that's a good one!

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.

3 participants