-
Notifications
You must be signed in to change notification settings - Fork 0
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
Multiline JSX elements must be wrapped in parentheses #1
Comments
I like it for an uniform way of represent components no matter if we a single component or multiple components side-by-side e.g. when fiber comes and it is legal to return multiple roots, in that case I think it is really useful to use return (
<Component1 />
<Component2 />
); So if we update a render in a component from returning a single root to multiple roots you'll have to update the indentation level, and you become author of all the lines. If we only use this form everything looks the same no matter what, and it doesn't matter what happens to the component in the future, it will keep the same indentation level and I think it is a good thing to only use a single coding style altogether, no matter the context / use case. When you put in the return statement without the parenthesis the continuation indentation of props becomes less significant, in your own example the first prop almost align with the props which I don't like. If we use the parenthesis the props and children are always a level of indentation apart and the root component align start and end always align, and the start is not skewed by the return statement. return <RootComponent
prop1={value1}
prop2={value2}
>
<div key="key1">
{content}
</div>
<div key="key2" />
</RootComponent>; vs. return (
<RootComponent
prop1={value1}
prop2={value2}
>
<div key="key1">
{content}
</div>
<div key="key2" />
</RootComponent>
); |
You won't be able to return multiple roots like that. You'll be able to return arrays. So it'll be like this:
So you get what you want for free (no additional syntax that's not required).
I don't really get the argument for this, i think it's overrated.
I disagree, it's a good thing to have a visual distinction between a component returning an array of elements in contrast to one returning a single element. Also the coding style wouldn't be the same, your single components would use:
and your components returning arrays would use:
this further invalidates your previous argument about uniform syntax and indentation levels.
That's a minor aestethical thing that you'd only encounter in very specific situations based on your tab width. Not a strong argument IMO. |
I'd like to once and for all put the "optimize syntax for diffs" argument to rest. You should not be enforcing syntax to generate prettier diffs. Syntax should be optimized for writability, readability, usability and understandability, not the output of a diff. No matter what you do, you don't loose diff information, all it does is make it slightly harder to blame a specific line on a specific person/commit, and that can never take precedence over code writability, readability, usability and understandability. |
I disagree, I do think "optimize for diffs" is a great property. I like this better: render() {
return (
<Component>
...
</Component>
);
} than: render() {
return <Component>
...
</Component>;
} |
Optimizing for diffs can never be the primary goal. You can't disagree with that, if you do, we have a problem. In cases where it doesn't hurt the primary goal, of course we should do it. |
Obviously optimizing for diffs is not the primary goal - but picking a format rule that plays nicely with diffs can be a goal (and IMO should also be a goal). There's plenty of arguments for and against this rule. In general my personal view is that I actually quite like this rule - code being uniform is quite nice. In general I agree with most of what @thetrompf has mentioned. I never used to set up my components before I enabled this rule, but having used it for a while I actually quite like that I only have to train my eyes to look for mismatched parenthesis locations vs. open/close tags (the argument of The one case I would argue against this rule is this: const el = this.someFunction((
<Component>
....
</Component>
)); Which to be honest should probably (according to the rest of our linting rules) be written like this: const el = this.someFunction(
(
<Component>
....
</Component>
),
); Now bottom line here for me is: We're discussing whether or not to hit 1-2 extra keys every once and a while - in order to gain some more uniformity of our code base. For that alone, I can't really seem to come up with a compelling argument that says we should not do it. Part of making code readable and easily understandable is to accept that we're writing code not for the computer, not for ourselves, but for the next many many people that are going to read and attempt to understand it. Every little bit we can do to aid this goal - should be done, especially when it's at such a low cost as this. |
Problem is, you do not gain any uniformity by adding this. You still have to look for open and close tags, and you still have a problem once you can return arrays. It doesn't really do anything for us. You're pretty much using the same arguments for keeping it that i'm using for removing it. |
Another argument against adapting code style to generate better diffs: https://shkspr.mobi/blog/2014/11/why-i-vertically-align-my-code-and-you-should-too/. We should not fuck over what i'm coining "code usability" because our tools suck. Also, shouldn't |
I think that sucks. Unnecessary syntax, for no apparent reason.
vs.
The text was updated successfully, but these errors were encountered: