JavaScript standards about development HOT 35 CLOSED

I like:

• Idiomatic's whitespace, type checking, conditional evaluation, and naming standards
• Consistent quotes. I can't decide if single-quotes or double-quotes is best to start with. As a note, I always want HTML tags to contain double-quotes, so one the occasion when I output HTML, I single-quote it... so, something like... $("#pants").html('<a href="link">text</a>'); That might be an argument to use single-quotes by default. I go back and forth on this.
• Strongly avoiding ternary operators. I think they hurt code readability, and minifying code counteracts the advantage of ternary operators, anyway.

from development.

• I like idiomatic as well (preferring spaces to conform with nearly every OSS library out there)
  • I strongly disagree on comma separating initialized vars (safer to initialize each individually)
  • I dislike camelCase (underscores are so much easier to read) but I'll conform to what everyone has been using so far.
  • I don't feel as strongly about switch statements, it's not like we're building the entire front end in JS.
• I agree on using single quotes by default and saving double quotes for contained html attributes.
• I feel ternary is ok for shorter conditions/definitions rather than initializing a var and updating it within conditions but agree complex ternaries hurt readability.

from development.

• I don't love or hate the "clean, comma separated" initalized vars thing. I don't think it's more readable (nor less readable), but sometimes it does end up looking ridiculous. And sometimes you just end up essentially "re-initializing" the variable later, so... Idiomatic likes it, I don't hate it, so I just go with it. 😄
• I no longer care about camelCase, underscores, or justonelongstring styles of vars and functions. Consistency would be nice, but mostly I use editor features to find instances of vars and functions, I can copy/paste and never have to actually type them out, etc. Usually, I disagree more with what people name their vars and functions than how they format that name.
  • I do think we should avoid dashes in function and variable names. 💁

from development.

So long as there's a good linter we can use Bureau-wide, I'm not too picky about the rules we go with here.

I like starting with something that focuses on readability Node.Js Style Guide or Idiomatic and we can add more rules that emphasize performance as time goes by. Some of those Airbnb articles look great.

from development.

Über Edit:
After thinking about this awhile longer, to date we have not been too picky on explicit stuff as long as we're consistent and the code is readable, consistent, and structured in a logical way. My preferences are not really helpful or constructive to this debate - they're flexible, so I removed them.

Idiomatic is a solid starting point that has been used in the past, and the NodeJS guide brings in a couple of things that are nice as well. Rather than specifics I'm starting to think about this more in value statements which are not prescriptive but indicate preference and help to better frame decisions when trade-offs are in the mix and help us ship quality code.

For example: We value readability over compact code, project consistency over explicit global standards, automated build tools over reliance on manual checking, shipping great software over unshipped perfect software.

from development.

I just added the existing JavaScript style guide to the wiki. This might be useful as a base document.

from development.

I came across this Meta Code Style Guide from Speaking JS today.

@mistergone will appreciate the "Don't Be Clever" section.

from development.

Where do we sit on this @mistergone @anselmbradford @wpears?

from development.

Sorry @ascott1 We need to do some more chatting about it! @mistergone @wpears I can carve out some time in about 30 min if you want to chat?

from development.

No worries @anselmbradford, just didn't want this to get forgotten about!

from development.

@wpears and I had a conversation around this and jotted down these thoughts:

• Readable code: Readability of the code should be a core lens through which these discussions happen. That said, people all have different interpretations and requirements over what readability means, so...
• Emphasize, but not require, official linter settings: CFPB should provide an official standard jshintrc settings file, which CFPB projects should use, but are not required to use.
• Projects should adhere to their chosen standard: Whether a project uses the CFPB official standard or chooses to go its own way (and use custom JShint options, for instance), what should be required is that the project follow its own internal standards project-wide.
• A quick way to verify conformity would be cool: Along the lines of the above, it would be awesome if there was a badge that repositories could display that they are "CFPB-standards compliant" if they use the official jshintrc file. We speculated briefly on how this might be done with the GitHub API.
• Coding conventions that fall outside of a linter should be documented: There are coding conventions that a linter won't pick up (for example, I like to prefix variables/functions with an underscore if they won't be exposed outside of a JS module). These conventions should be captured in the standards document and serve as a reference to what these conventions mean.

from development.

➕ 💯

from development.

Those are awesome, pithy, and concise. If we went no further I think we'd still be in good shape. I'd be interested in helping with badging / linting.

from development.

I'm happy to carve out time to discuss, but what @wpears and @anselmbradford came up with is phenomenal. We can dive into "readability" more, but this is a great set of guidelines.

@ascott1 - I did appreciate the "Don't Be Clever" section! 😄 🚀 🐫

from development.

👍 I'd recommend codifying many of the recommendations into requirements that we can run automated checks against:

• Every project must have a linter config file (either .jshintrc or .eslintrc). Projects can stray from the official CFPB style guide but the difference has to be documented in a config file.
• Every project must be automatically linted as part of its build process (e.g. create a Grunt task for it).
• Don't push any code that has linting errors (e.g. fail Travis and Jenkins builds with style errors).

Coding conventions that fall outside of a linter should be documented.

Check out ESLint. It's AST-based so you can check against a lot of style rules and if they don't support a rule, you can write a plugin to enforce it. Here's a sample .eslintrc we're using with dash.

People don't RTFM so I find it's best to favor code over documentation.

from development.

Agreed @contolini

The reason we included that phrase is that it is easier to tell people to write down conventions than to tell them to use ESLint AND write their own plugins, even though that is ideal.

Saying everyone SHOULD use ESLint would be pretty cool, but seemed outside the scope of our hangout. I'm definitely in agreement that something like a pre-commit git hook (or grunt task) wouldn't be too hard of a sell and would definitely lead to cleaner code in repos.

from development.

I agree that having a codified ESLint file is a good idea. Many of our projects already use JSHint, so converting and cleaning things up to match our recommendations should be a reasonably straight forward process.

from development.

+1 for creating a ESLint file that will represent the consensus.

from development.

I just want to 👍 the sentiments of an "official" linting file, with variations for folks who want to do something different (as long as they document it), and I like a git hook for it as well. I don't know anything about ESLint and such, but I'm ready to learn.

from development.

I haven't played around with it much, but this pull request adds ESLint to eRegs. There will need to be a fair amount of cleaning up to match our standards once we've settled on them.

from development.

👍 👍 👍 to the general direction of this thread.

from development.

I'd add to this discussion to usejsdoc.org ... especially since ESLint has a rule for it http://eslint.org/docs/rules/valid-jsdoc.html

from development.

+1 for JSDoc

from development.

I would just add that I hope we can move in the direction of ES6 sooner rather than later. Tools like https://babeljs.io/ can help with that. Would be nice if we could consider using TypeScript or AtScript as well. http://techcrunch.com/2015/03/05/microsoft-and-google-collaborate-on-typescript-hell-has-not-frozen-over-yet/

from development.

I'm warmer to the idea of using ES6 over other compile-to-JS options. In general, the smaller the project, the more free you are to experiment, but I think a flagship project done in TypeScript would attract some trouble. There's some organizational cost to experimentation in new devs having to learn syntax, etc which isn't always justified, even if the language is "better".

ES6 is a little bit different in that the changes are less drastic and it's where Javascript as a whole is moving, so I think it's both easier to grasp and easier to sell to devs joining the project.

from development.

Just wanted to say +1 to the standards discussion above. I think one of difficulties in coming up with shared standards is that some issues have more than one valid position, so scoping the standards at the project level and documenting/enforcing them internally with ESLint (and possibly other things like .editorconfig files) seems like a good solution.

from development.

I think if Jenkins/Travis failed on errors in the ESLint file, but not warnings (I suspect that'd be the default behavior anyway?), we can put all the absolute essential do-not-dos at error level and all the stylistic stuff at warning level. Cfgov-refresh now has this comprehensive ESLint file where I went through all the settings and explicitly laid them all out. It's a mixture of existing coding standards in the project and what made sense to do (based on browser compatibility concerns, etc.).

from development.

@anselmbradford nice work on that ans!

I'm parsing through it, but so far, so good!

from development.

Also, not saying that has to be the CFPB standard ESLint file, but can be a starting point for seeing what can and can't be linted. The settings chosen were what made sense for cfgov-refresh.

from development.

Per @virginiacc comment, here's an example .editorconfig file in an existing project.

from development.

Have you all seen https://github.com/feross/standard? It seems to be getting a lot of attention lately. I'm not crazy about omitting semicolons, but otherwise it's pretttty interesting.

from development.

@ascott1 Yeah, takes care of the badge idea lol 😄

I wonder what they do with "No unused variables – this one catches tons of bugs!". I didn't like how JSHint handled that. It would count unused method parameters as unused variables (at least in the implementation I was using). IMO it's helpful code documentation to have unused parameters so you know if a method is receiving an argument from an event, for example. E.g.:

btn.addEventListener('click', handleClick, false);
function handleClick(evt) {
   // evt event isn't used, but nice to know it's being added!
}

ESLint has a nice happy medium for allowing some unused parameters for exactly this case.

from development.

Oh, that's interesting @anselmbradford. I hadn't considered that. I think for our purposes a simple custom ESLint file with an associated grunt task makes the most sense (unless we're all ready to abandon semicolons).

from development.

I came across this guide over the weekend, which I really like the simplicity of: https://github.com/bevacqua/js/blob/master/README.md

from development.

Now that the initial standard is in the master branch, let's close this issue and move further discussion and edits to new issues.

from development.