generoi / genero-design-system

https://gds.generogrowth.com/
MIT License
4 stars 0 forks source link

Prettier options #3

Closed taromorimoto closed 4 years ago

taromorimoto commented 4 years ago

I've always rather use lint instead of prettier because it just ends up messing some code. But if we go with prettier,Β let's discuss the prettier options. πŸ˜„

Here's my thoughts:

printWidth as big as possible (pray) semi: false, if someone want's to use true, then please explain. arrowParens: avoid jsxBracketSameLine: true

In my limited experience with prettier, the jsx formatting is too aggressive, but I'm not sure what can be done about it.

silentnoodlemaster commented 4 years ago

about printWidth, how about 120

henritukiainen commented 4 years ago

My thoughts:

printWidth as small as possible, prefer 80, 120 goes too. (we always disagree on this with Taro :) semi agree with Taro, strongly prefer false arrowParens agree with Taro, prefer avoid jsxBracketSameLine prefer false

Personally I prefer linting to autoformatting, but hopefully prettier isn't as buggy as it used to be so I'm willing to give it a try of course.

henritukiainen commented 4 years ago

And I'm sure we'll have as many opinions on the exact settings as we have developers, so unless you're planning to set up a vote, just pick whatever you think works best :)

oxyc commented 4 years ago

I vote for keeping prettier so we can leave out the stylistic lint part from reviews. But if you have a preference, lets πŸ‘ πŸ‘Ž this comment for example. πŸ‘ means remove prettier as requested.

As for the settings I'm perfectly fine with the ones mentioned. I'm used to having a 12" with split windows so tend to prefer 80 but 120 sounds perfectly fine.

If it turns out we're not happy with how it rewrites JSX we can always turn it off for jsx. A while back Gutenberg ran it's entire codebase through Prettier.

oxyc commented 4 years ago

As far as ASI goes i'm fine without semicolons as long as prettier enforces the edge cases and no one needs to know them :D I've encountered bugs at least twice from third party code where the developer didn't understand the rules of ASI. self invoking anonymous function are less of a thing nowadays though, and i think that was the bug on both my encounters.

toffebjorkskog commented 4 years ago

printWidth at 120 sounds like a good compromise, i think we have used 80. semi i prefer true for non-react code, as minifying a non semi code traditionally can break the code. ( i see it similarly to not having {}Β in a one liner if statement). But if js is anyway transpiled iIam fine with whatever, but it feels weird not having it. arrowParens i prefer these, as it makes it more understandable that it is a function, but i don't have a strong opinion. jsxBracketSameLine looks better with false

taromorimoto commented 4 years ago

The reason I'd like jsxBracketSameLine: true is that you can clearly see when where the tag ends. But I don't have that much faith in prettier JSX that I'm sure it will be bad no matter what we choose... πŸ˜„

4k monitors are becoming a standard so having 80 doesn't make any sense, imo. πŸ™

If I understand correct semi: false will enforce the edge cases. People justify inserting unnecessary characters to the code just because they have gotten used to it. I'm sure Columbus felt weird when he set sail to the new world, but it turn out fine after some time (excluding the natives). πŸ˜‰

If you write a lot of arrow functions, using the brackets if there is no need seems like the same case thing as the semicolons.

I think it will be fun to use prettier, let's see how it goes. πŸ˜„

oxyc commented 4 years ago

But I don't have that much faith in prettier JSX that I'm sure it will be bad no matter what we choose... πŸ˜„

If it's bad we'll just turn it off :) With stencil I guess there shouldnt be much JSX anyways. Maybe storybooks?

4k monitors are becoming a standard so having 80 doesn't make any sense, imo. πŸ™

This doesn't apply if you use split screen editors though. I get 85 chars per line in VSCode and 95 in vim. Haven't had an external monitor since 2009 :D Also see https://github.com/generoi/genero-design-system/pull/4#issuecomment-627939105. But let's go with 120 until we notice prettier messes things up.

I'm sure Columbus felt weird when he set sail to the new world, but it turn out fine after some time (excluding the natives). πŸ˜‰

Haha I was about to say, it turned out fine for the new europeans but there are 0 natives left in Uruguay.

If you write a lot of arrow functions, using the brackets if there is no need seems like the same case thing as the semicolons.

I'm annoyed when I have to add the parens but it would be time to finally learn these damn VSCode shortcut keys... Damn nonsensical emacs bindings.

henritukiainen commented 4 years ago

4k monitors are becoming a standard so having 80 doesn't make any sense, imo. πŸ™

I use three 4k monitors and I still very very strongly prefer having a shorter-rather-than-longer rulers.

As @oxyc said, it makes a huge difference with split editors. For my setup 80 means you can use three editors side-by-side, 120 means you can use two, and going much higher than that pretty much means you can't reliably split the editor without breaking readability on some lines.

And the worst part is that only 1% of lines really are that long, so most of your screen real estate ends up being empty :)

Agree with everyone about giving 120 a try :)