Skip to content

Instantly share code, notes, and snippets.

@kadamwhite
Last active December 27, 2015 10:29
Show Gist options
  • Save kadamwhite/7311569 to your computer and use it in GitHub Desktop.
Save kadamwhite/7311569 to your computer and use it in GitHub Desktop.
Towards Consistent JavaScript Code Style in WordPress

JavaScript Coding Standards

The core WordPress PHP files become cleaner and easier to read with every release, thanks in part to our strong standards for PHP code style. Our JavaScript, on the other hand, hasn't gotten nearly enough love. This post is intended to open up the recent discussion around JavaScript style to the greater community so we can make up for lost time.

Don't we already have a style guide for JavaScript?

Back in March, @tommcfarlin added a set of coding standards for JavaScript to the developer handbook. These WordPress JS coding standards were a great work-in-progress, but weren't fully comprehensive (leading to some comment threads clarifying various areas). More importantly, without any clear implementation plan the style guide failed to gain traction.

At WordCamp Boston's core contributor day I revisited this style guide with @mattwiebe and @gnarf37. We believe it is important to identify and implement conventions for JS style ASAP because syntax issues in the JS within WordPress may hide latent bugs, and inconsistent code discourages contribution. Focusing on implementation lead us to look for an existing, proven JS style guide with a .jshintrc file (a set of configuration options for the JSHint code quality tool) which we could adopt largely as-is: Getting JSHint in place lets us see the biggest issues in our JS, so we can begin tackling them incrementally (perhaps in the same manner as the inline docs effort).

After looking at Idiomatic JS and several other widely-adopted JS style guides, we feel the jQuery Foundation's jQuery Core JavaScript Style Guide guide is the closest match for what we need in WordPress.

Adopting the jQuery Core Style Guide

jQuery's guide shared WordPress core's love of white space—the same "when in doubt, space it out" mantra from the existing JS style page. Moreover, jQuery's code conventions have been referenced in trac tickets as an example of how we should be writing our code. Adopting their guide wholesale capitalizes on our stylistic similarities, and will let us adopt their .jshintrc and any future code quality tools they write with minimal changes.

Adopting this guide means making two big changes to the way that we have been writing JS:

1. Strict Equality

We need to move to using === instead of ==. We've been getting away with using the double-equals in WordPress, but enforcing triple-equals ("threequals") will avoid the unpredictability of type coercion. To quote the aforementioned Idiomatic.js, evaluate for the most accurate result. It is not intuitive that "10" == 10 will be true despite the string/number type difference, and unituitive code should be avoided.

2. Consistent curly braces

Every widespread JS style guide prefers enclosing single-line statements in curly braces, and we should too. If you always use brackets, adding other statements to a block later is trivial; if they are not present, adding new statements can lead to bugs. We don't require braces for single-line conditionals in our PHP standards, but it is noted that having them "allows for fewer line edits for debugging or additional functionality later." They also make the presence of blocks more obvious, particularly when the "single-line" statement is a complex jQuery operation.

Consider:

// Two lines, one statement
if ( someString === 'desired value' )
    $( '.class-name' ).show().value( someString + '!' )
        .on( 'click', doSomething );

// Only three characters more, easier to "eyeball," and easier to add
// additional statements to the conditional block later on
if ( someString === 'desired value' ) {
    $( '.class-name' ).show().value( someString + '!' )
        .on( 'click', doSomething );
}

We believe making these big changes and the other smaller consistencies found in the jQuery style guide, will improve the reliability of our JavaScript and should be integrated across the JS files in WordPress.

Exceptions

There are two specific areas where we plan to deviate from the jQuery Core Style Guide:

1. We will use Single Quotes for strings

jQuery's guide specifies double quotes for string declaration. The existing WordPress JS uses single quotes, and we prefer to keep this as-is: JS strings in WordPress will continue to use single quotes.

2. Iterator variables declaration

jQuery specifies that iterators should be initialized when the variable is declared, at the top of the function; this is done as a minified file size optimization, but is extremely unintuitive for inexperienced JS devs. To quote Aaron Jorbin, who raised this issue, "I would rather optimize for developer happiness." Example below.

// Confusing
var i = 0;

// A bunch of lines of other code

for ( ; i < 100; i++ ) {
    // Expressions -- but how many? Where did `i` get declared, again!?
}


// Preferred / More readable
var i;

for ( i = 0; i < 100; i++ ) {
    // Expressions
}

Next Steps

@nacin has landed an adaption of jQuery's .jshintrc file in Core, and once we've gotten some comments with feedback to this post I will update the Handbook's JS standards page with examples of the jQuery conventions.

Implementing these standards will need to be done in pieces. Much of our code is pretty close, but JSHint can't even fully process some of the older files in our system due to the number of errors. We'd like to propose setting up some JSHint Compliance sprints to work through the codebase in a structured way.

Please leave a comment below if you would like to be involved in this process! Based on a quick poll of known interested parties we will be meeting in IRC tomorrow at [time]November 6, 2013 19:00 UTC[/time], where we can continue the conversation and brainstorm how best to break up the work.

@kadamwhite
Copy link
Author

Comments from Jorbin via Skype, when polled on what might be missing here:

Perhaps the path to getting our JS into compliance? Even if it's just scheduling an IRC meeting to make it happen.

We may also want to say that we aren't going to make changes just to make changes, While style guides are important, they aren't the end all be all

Nacin has something he likes to quote for that

http://www.linuxfoundation.org/content/41-pitfalls

@tommcfarlin
Copy link

While working on the style guide, do we also want to offer more performant ways of doing things?

For example:

var i, l;
for ( i = 0, l = array.length; i < l; i++ ) {
   // Expressions...
}

Is more performant than:

var i, l;
for ( i = 0; i < array.length; i++ ) {
   // Expressions...
}

Especially as the dataset is larger.

Or is this going to be strictly styles?

@kadamwhite
Copy link
Author

@tommcfarlin I think keeping the example simple is best here, since what we're trying to illustrate is the discrepancy from the style used in jQuery. I definitely agree that there are some less styleguide-oriented practices that we should adopt, but I do think I want to keep this discussion centered on styling.

@Protonk
Copy link

Protonk commented Nov 5, 2013

I would remove the single/double quote example as it seems like it is being set up as 'confusing' or 'preferred' when we really mean the iterator declaration is either confusing or preferred.

@Protonk
Copy link

Protonk commented Nov 5, 2013

There are two key areas where the jQuery style guide differs from our existing practices, both of which are enforcable with JSHint options:
...

This may sound redundant, but is it possible to make this section a bit more clear? Or perhaps more closely linked to the subject?

We have two big sections in this post. One on current style in WP codebase which will change as we adopt jQuery style guide. The other on current style which we're choosing to maintain. It seems like we're using both sections to talk about the specific benefits (or lack of downsides in the case of ' vs "), which feels somewhat superfluous. It feels like we're arguing for the adoption of eqeqeq and curly rather than noting the specific change in this project.

Specifically if we look at the jshintrc addition we see some explanation of why the choices were made. Even a tiny note (linking to that file/rev) noting that choices were made to follow notionally the PHP style as well as to conform as much as reasonably possible to past practices might be helpful. You mention this at the top when comparing idiomaticjs to jQuery but could expand on it a bit later. Like in this case most JS devs would agree on eqeqeq but it's valuable to point out that PHP has a...similar == vs === and WP has already adopted === for PHP.

@mattwiebe
Copy link

WP has already adopted === for PHP.

If this is the case, it is not reflected on the WP PHP Coding Standards page. All code examples on said page use non-strict comparisons.

@beaulebens
Copy link

I don't think anyone would argue that we don't need a standard, so we should just focus on the proposal of jQuery's style, and why that's the suggestion:

I'd personally focus on:

  • we need a standard, and what we have right now doesn't really cut it for various reasons (and what those are)
  • we propose jQuery's standard, and why it's a good one to use
  • what differences that would result in compared to our current style
  • a roadmap to bringing code into line with the new style (including .jshintrc, proposed sprints to reformat etc)

In general though, I'm in agreement with the idea. The only part I'd expect there to be any real pushback on would be the curly braces thing -- I can see an argument there that if we're going to require it in JS, we should require it in PHP as well, and I think that horse gets beaten periodically in both directions :)

@tollmanz
Copy link

tollmanz commented Nov 5, 2013

Great start on this @kadamwhite!

jQuery and WordPress go back to 2007, and all the jQuery Foundation sites run on WordPress.

I am not sure if this is a good reason for adopting jQuery's standard. I think it distracts from the main point, and could serve as a distraction. I think you should omit this.

Regarding eqeqeq, I find this to be really unclear. That you are bringing it up and that jQuery advocates the strict comparison, I assume you are arguing that we should adopt the non-strict comparison. Either way, I think that we should stick with jQuery's strict compare recommendation and, if you intend to stray from this, a much better rational is required. As @Protonk suggested, I think this whole section needs clarification. I would offer suggestions, but I am not sure what the intent is.

I definitely agree that there are some less styleguide-oriented practices that we should adopt, but I do think I want to keep this discussion centered on styling.

While I agree with this in principle, I think a great style guide offers rational for why style decisions were made, which can cross into best practices. The Idiomatic JS guide that you linked to clearly outlines the rule and backs it with clearer rational. I think clear rational could lead to better adoption. Explaining to the developer why she should do things this way may reduce defensiveness and encourage adoption. Additionally, 5 years down the line, if the original authors of the guide are no longer in the community, the reasons for the guidelines are clearly articulated and future changes can be considered with full understand of the original decisions. Documenting the documents FTW!

@beaulebens' outline is a great one and I think this doc does that for the most part. As I stated earlier, I think things get foggy in the middle. I do think this could be strengthened by more reasons why the jQuery standard is a good one based on the merits of the standard itself, instead of jQuery's status in the web development community. Do not get me wrong, I see that as a value point to consider, but I think we need to evaluate the quality of the standard as well.

@kadamwhite
Copy link
Author

Updated based on the feedback—thanks, all!

@aaronjorbin
Copy link

We'll be picking a time for an IRC chat later this week, where we can continue to brainstorm how best to break up the work to get all these files in line.

I would just pick a time rather than saying one would be picked later on. Set the next step up when this is published.

@Protonk
Copy link

Protonk commented Nov 5, 2013

@mattweibe

If this is the case, it is not reflected on the WP PHP Coding Standards page. All code examples on said page use non-strict comparisons.

Ah, my mistake. Hopefully the rest of the message stands apart from that. :|

@kadamwhite
Copy link
Author

Pinging IRC for help getting this posted

@kadamwhite
Copy link
Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment