Style Guide

[RyanCCollins]

Let's leave this as a running dialog about stylistic choices so that we discuss our preferences and what the best practices are that we should follow on future projects.

[RyanCCollins]

Note in #41 we should definitely be using .js vs .jsx. This will mean altering the generators and makes sense for consistency and best-practice sake.

[RyanCCollins]

For discussion

1. JSX Alignment
2. Comma Dangle

See 40539d7...18c2b2f for original comment.

[RyanCCollins]

Also, function vs. const for stateless functions. I'd like to hear what the advantages are to one over the other. In the past, I favored const / arrow function for brevity sake but I think that function is still the recommended best-practice due to the fact that const relies on function name inference.

Would like to hear what you think, @karatechops.

const Foo = ({ bar }) => <div>{bar}</div>;
vs
export default function Foo({ bar }) {
  return (
    <div>{bar}</div>
  );
}

[karatechops]

I agree with the use of comma dangles, let's go ahead and implement those. I'm also all for using fat arrow implicit return functions for stateless components, just not looking forward to the refactor. 😄

We definitely have a bit of stylistic freedom with this project so these discussions are a helpful forum for getting on the same page, it's also a good opportunity to establish a comfortable/efficient pattern and potentially establish a pattern for future OSS devs.

[RyanCCollins]

I see a Grommet Style guide in the pipeline 😎

Yeah, I don't think major refactors are worth it at this time for sure. We can start fresh on the next project with all of our combined stylistic wisdom

[karatechops]

I noticed we have different ternary styles - I prefer this style as it's very easy to identify the two operations -

const assets = (this.props.assets && this.props.assets.length > 0) 
    ? this.props.posts.map((asset) => 
      <AssetTile asset={asset} />)
    : undefined;

@RyanCCollins thougths?

[RyanCCollins]

Just curious about the underscore for class methods. I've been following this pattern since picking up the code, but am just wondering if there is a reason for it? Actually, a friend of mine (@ghoshabhi) asked me about this the other day after looking through the grommet-cms code and it got me thinking about it.

Here is what the AirBnb style-guide says, not that we always need to follow it. Just food for thought and curious where it comes from.

Do not use underscore prefix for internal methods of a React component.

Why? Underscore prefixes are sometimes used as a convention in other languages to denote privacy. But, unlike those languages, there is no native support for privacy in JavaScript, everything is public. Regardless of your intentions, adding underscore prefixes to your properties does not actually make them private, and any property (underscore-prefixed or not) should be treated as being public. See issues #1024, and #490 for a more in-depth discussion.

[karatechops]

The styling follows grommet core's styling which essentially adds underscores to a component's methods. In my personal JS projects outside of React, I like the pattern of using underscores for private methods but that doesn't make much sense once you're in the React eco-system.

I lean towards AirBnB's setup for most styling however it'd be a pain to remove the underscores at this point.

[RyanCCollins]

Yeah, I agree with that!! No sense in making drastic changes this late in the game, We can bookmark these decisions for future projects.