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: change default to 'always' #1505

Closed
tomipaul opened this issue Oct 28, 2017 · 5 comments
Closed

jsx-boolean-value: change default to 'always' #1505

tomipaul opened this issue Oct 28, 2017 · 5 comments

Comments

@tomipaul
Copy link

According to the documentation, the first argument to this rule is either 'always' or 'never'

Current behavior: The default value of the first argument is 'never' so it throws a warning if an attribute has a true value

Proposed behavior: The default value should be changed to always so it warns whenever an attribute is missing its value.

Why: Prop defaulting to true when it has no value is a feature that was added just for consistency with the HTML syntax. According to the react docs; In general, we don’t recommend using this because it can be confused with the ES6 object shorthand.
Fundamentally React's JSX is just syntactic sugar. It's still primarily Javascript, so consistency with the language is important which is why the community has done well to put forward these recommendations. In this wise, I think using never should be a conscious decision not something that comes out of the box

@ljharb
Copy link
Member

ljharb commented Oct 28, 2017

React's docs can recommend whatever they like; omitting ={true} is considered a best practice in the wider community, and for boolean props, it makes the element read more nicely.

jsx has a number of things that break the idea that "consistency with JS" is critical, and this is one of the ways I think the deviation is an improvement.

jsx is html, conceptually, even if it's implemented as syntactic sugar - so consistency with HTML is more important.

@tomipaul
Copy link
Author

@ljharb While I think this is a trivial issue, I don't completely agree with your submission.
I have seen several instances of developers writing something like this

const messages = [ ...]
<MessageList messages />

thinking messages prop will have the array as value (ES6 object shorthand). While it is arguable that these people have not taken time to go through the react docs, I think it's rather becoming a pitfall and unnecessarily so. I'm not saying the rule should be removed, rather I feel the default can be changed to always and anyone can always go ahead to switch it to never. Although to eliminate ambiguity, I'd still recommend people stick to always.

Talking about best practice, I believe best practices are often backed up with reasons that border on readability and efficiency. If it's about what people like, then styleguides don't stand a chance to begin with.

jsx is html, conceptually, even if it's implemented as syntactic sugar - so consistency with HTML is more important.

In another vein, I disagree with this, conceptually is meant to be the key word but it does not cut it. Javascript as a language, its DOM API and the ES6 syntax have all influenced JSX. That we have one or two deviations based on use cases does not prove this wrong.
This is an argument that has gone on for a long time already especially together with the class vs classname argument. Within the react development community and beyond, a lot has been said about JSX being closer to JS than HTML. So if this is your standpoint, it's all good.

facebook/react#2781
facebook/react#2782
https://facebook.github.io/jsx/
https://reactjs.org/docs/introducing-jsx.html#specifying-attributes-with-jsx

@ljharb
Copy link
Member

ljharb commented Oct 29, 2017

Of course jsx is closer to HTML than JS; otherwise why bother with the sugar?

@tomipaul
Copy link
Author

tomipaul commented Nov 2, 2017

I get your point, if we are looking at it superficially from the aspect of how it reads, then I somewhat agree with you but then we have stuffs like dot notation for JSX types and spread attributes. Even the JSX specification draft says

JSX is designed as an ECMAScript feature and the similarity to XML is only for familiarity.
This specification does not attempt to comply with any XML or HTML specification

See these issues;
facebook/jsx#68
facebook/jsx#65
facebook/jsx#23
facebook/react#9707
If these proposals are implemented, JSX will move much more closer to JS.

@ljharb
Copy link
Member

ljharb commented Nov 2, 2017

Then at that time, this can be re-evaluated. In the meantime, I’d say you can just set the rule to “always” if you prefer.

@ljharb ljharb closed this as completed Nov 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants