Proposed JavaScript Coding Standards Revisions for Prettier Compatibility

(The following standards revision proposal was drafted by @jsnajdr. Jarda maintains the wp-prettier Prettier fork and has been working to introduce it to the Gutenberg repository. You’re invited to provide feedback to this proposal in the comments below, or to join an upcoming JavaScript meeting in #core-js, where this will be discussed)


Across the JavaScript community, the Prettier code formatter has become immensely popular over that last three years since it was originally released. It automatically performs high-quality formatting of your JavaScript code: when you press Save, your code is instantly formatted. This removes a distraction for contributors who write or review code, and allows them to focus on the more valuable aspects of their work without having to discuss the JavaScript WordPress Coding Standards so often. That’s why we’d like to adopt it in the WordPress JavaScript code bases, too.

The official Prettier formatter is very opinionated and has very few options. The reasons are both technical and cultural. The complexity of the formatting algorithm would explode exponentially with too many options and their combinations, and a part of the project vision is to establish an unified JavaScript formatting standard.

The WordPress formatting standard has one major incompatibility with the Prettier convention: it requires spaces inside all kinds of parens — (){}[], inside template strings, JSX attributes and expressions, everywhere:

function Label( { text, icon } ) {
	if ( ! [ 'left', 'right' ].includes( icon ) ) {
		return null;
	}

	return (
		<label className={ `icon-${ icon }` }>
			{ text }
		</label>
	);
}

To teach this convention to Prettier, we had to create a fork that adds an extra option and modified the paren-formatting code, and publish it on NPM under the name wp-prettier (@wordpress/prettier would arguably be even better name!). At this moment, we’ve been using that fork for 2.5 years in the Calypso and Jetpack projects, have maintained and updated it over countless upstream releases, and are confident that we can recommend it to anyone who wants to format their JavaScript code the WordPress way.

In a Gutenberg pull request, we are proposing adopting the WordPress Prettier tool in the project.

After adding support for the WordPress style paren spacing, there remain several very minor cases where Prettier formats JavaScript code slightly differently from what the current WordPress JavaScript Coding Standards recommend. They don’t diverge from the essence and spirit of the WordPress coding standards. Further patching of Prettier would be, on our opinion, not worth the coding and maintenance effort. And in some cases is outright impossible, because the recommendation asks for human judgment that an algorithm cannot implement.

In this post, we propose several small changes of the coding standards that align them fully with the Prettier convention.

Formatting ternaries and binary ops

The standard says that when breaking a long statement, the “operator” should be at the end of line, and doesn’t distinguish between binary and ternary operators. But Prettier does. When breaking a binary operator, it indeed puts the operator at the end of line:

const result =
	partOne +
	PartTwo;

But the parts of a ternary operator are put on the start of the new line (after the indentation):

const result = isRtl
	? 'right'
	: 'left';

Also, the standard recommends that long “lines should be broken into logical groups if it improves readability”. That doesn’t happen with Prettier — it wraps the lines if and only if the line would be longer than maximum line length otherwise. We propose to remove that ambiguous and subjective formulation from the standard.

Wrapping chained n-ary operators

Another difference related to binary operators and the Multi-Line Statements section is that Prettier puts each operand on separate line, even the left side of an assignment. It doesn’t do the “fluid text wrap” style like this:

const result = a + b +
	c + d;

but this:

const result =
	a +
	b +
	c +
	d;

To address all these differences, we propose to reformulate the Multi-Line Statements section of the standards document as follows (the last paragraph about conditionals is unchanged):

Before:

When a statement is too long to fit on one line, line breaks must occur after an operator.

// Bad
var html = '<p>The sum of ' + a + ' and ' + b + ' plus ' + c
	+ ' is ' + ( a + b + c ) + '</p>';
 
// Good
var html = '<p>The sum of ' + a + ' and ' + b + ' plus ' + c +
	' is ' + ( a + b + c ) + '</p>';

Lines should be broken into logical groups if it improves readability, such as splitting each expression of a ternary operator onto its own line, even if both will fit on a single line.

// Acceptable
var baz = ( true === conditionalStatement() ) ? 'thing 1' : 'thing 2';
 
// Better
var baz = firstCondition( foo ) && secondCondition( bar ) ?
	qux( foo, bar ) :
	foo;

When a conditional is too long to fit on one line, each operand of a logical operator in the boolean expression must appear on its own line, indented one extra level from the opening and closing parentheses.

if (
	firstCondition() &&
	secondCondition() &&
	thirdCondition()
) {
	doStuff();
}

After:

When a statement with operators is too long to fit on one line and is broken into multiple lines, line breaks must occur after a binary operator. Each operand with the operator that follows it must be on a separate line.

// Bad
const html = '<p>The sum of ' + a + ' and ' + b + " plus " + c
	+ ' is ' + ( a + b + c ) + '</p>';
 
// Good
const html =
	'<p>The sum of ' +
	a +
	' and ' +
	b +
	' plus ' +
	c +
	' is ' +
	( a + b + c ) +
	'</p>';

On the other hand, when a long statement with a ternary operator is broken into multiple lines, the parts of the ternary operator should be after the line break:

// Bad
const baz = true === conditionalStatement() ?
	'thing 1' : 
	'thing 2';

// Good
const baz =
	true === conditionalStatement()
		? 'thing 1'
		: 'thing 2';

When a conditional is too long to fit on one line, each operand of a logical operator in the boolean expression must appear on its own line, indented one extra level from the opening and closing parentheses.

if (
	firstCondition() &&
	secondCondition() &&
	thirdCondition()
) {
	doStuff();
}

Chained method calls and context

The standard recommends that when one method in a chain “changes the context”, it should add an extra indentation:

elements
	.addClass( 'foo' )
	.children()
		.html( 'hello' )
	.end()
	.appendTo( 'body' );

But Prettier doesn’t know what the context is — it’s an ambiguous concept even for a human. So it indents everything equally:

elements
	.addClass( 'foo' )
	.children()
	.html( 'hello' )
	.end()
	.appendTo( 'body' );

It also adds one extra touch: if the initial identifier is just one or two characters long, it will keep the first method call on the same line:

el.addClass( 'foo' )
	.children()
	.html( 'hello' )
	.end()
	.appendTo( 'body' );

To address these differences, we propose to reformulate the Chained Method Calls section of the standards document as follows. The new formulation removes the requirements that can’t be reasonably implemented, focuses on the main points, i.e., breaking into multiple lines and consistent indentation, and modernizes the jQuery-based example to show more contemporary, functional JavaScript code.

Before:

When a chain of method calls is too long to fit on one line, there must be one call per line, with the first call on a separate line from the object the methods are called on. If the method changes the context, an extra level of indentation must be used.

elements
	.addClass( 'foo' )
	.children()
		.html( 'hello' )
	.end()
	.appendTo( 'body' );

After:

When a chain of method calls is too long to fit on one line, it must be broken into multiple lines where each line contains one call from the chain. All lines after the first must be indented by one tab.

findFocusable( context )
	.filter( isTabbableIndex )
	.map( mapElementToObjectTabbable )
	.sort( compareObjectTabbables )
	.map( mapObjectTabbableToElement );

#javascript, #standards

Proposed Revision to CSS Coding Standards

During the Core Dev Chat on January 16th, 2019, a proposal for a minor change to the lengthier multi-part values section was briefly discussed.

The following is what the current CSS Coding Standards recommend in the property values section:

Multiple comma-separated values for one property should be separated by either a space or a newline, including within rgba(). Newlines should be used for lengthier multi-part values such as those for shorthand properties like box-shadow and text-shadow. Each subsequent value after the first should then be on a new line, indented to the same level as the selector and then spaced over to left-align with the previous value.

Code examples:

	text-shadow: 0 -1px 0 rgba(0, 0, 0, 0.5),
                 0 1px 0 #fff;

	transition: 0.15s color ease-in-out,
                0.15s background-color ease-in-out,
                0.15s border-color ease-in-out;

However, the current recommendation leads to indentation made with spaces or mixed tabs and spaces.

Ideally, indentation made with spaces or mixed tabs and spaces should always be avoided and, to preserve the indentation alignment, all lengthier multi-part values should be in a new line.

Worth noting that, in many cases, new lines and tabs are already used across the WordPress admin stylesheets, for example:

	box-shadow:
		0 0 0 1px #5b9dd9,
		0 0 2px 1px rgba(30, 140, 190, .8);

Proposed change

During dev chat, general consensus was expressed in favor of changing the CSS Coding Standards values section as follows:

Multiple comma-separated values for one property should be separated by either a space or a newline, including within rgba(). Newlines should be used for lengthier multi-part values such as those for shorthand properties like box-shadow and text-shadow, including before the first value. Values should be indented one level in from the property.

Code example:

	box-shadow:
		0 0 0 1px #5b9dd9,
		0 0 2px 1px rgba(30, 140, 190, .8);

The proposed change aims to improve consistency across the admin stylesheets, code readability, and style linting.

Any feedback is welcome! Please share your thoughts in the comments below.

#css, #standards

Proposal for JS Standards Revision: Removing Array/Function Whitespace Exceptions

Coding standards have been a recurring topic in the JavaScript Weekly Chat over the past few months. One rule in particular which has been the focus of much discussion are the exceptions for whitespace in arrays and function calls, which reads:

Always include extra spaces around elements and arguments:

[…]

Exceptions:

  • […] do not include a space around string literals or integers used as key values in array notation
  • Function with a callback, object, or array as the sole argument: No space on either side of the argument
  • Function with a callback, object, or array as the first argument: No space before the first argument
  • Function with a callback, object, or array as the last argument: No space after after the last argument

In the course of our chats, there has been some consensus around removing this “Exceptions” section entirely, though we considered to seek broader feedback on the decision, particularly in how it impacts overlapping PHP standards and existing code.

Why should this exception be removed?

The purpose of a coding standard should be to impose sensible rules for the sake of consistency and readability. In the case of consistency, a developer should be at ease both in writing and reading code.

The very existence of an exception is at odds with these ideals, harming consistency in that:

  • A code reader may not think to expect it if they are not familiar with the exception
  • A code writer may not know to apply it if they are not familiar with the exception. Or worse yet, one familiar with the rule may not know how to apply it.

The arrays and functions exception is notably egregious for being difficult to apply. Take, for example, the following snippet of code:

( function( wp ) {
	wp.foo = foo( 5, {});
	wp.bar(function() {
		console.log( 'Done' );
	});
	wp.baz( x, x + 5 );
	wp.baz(x => x + 5);
} )( window.wp = window.wp || {} );

This code is valid in its use of whitespace, but took considerable effort on the part of a developer well-versed in the standards to understand precisely where the exceptions do and do not apply.

Generally, exceptions increase the barrier to entry for new developers by imposing another prerequisite to becoming productive, increase overhead for existing contributors by requiring careful consideration of their application, and are anti-productive in the review hours wasted enforcing their inevitable mis-use.

Why should this exception not be removed?

All of the above notwithstanding, the exceptions concerning array or object keys at least have some basis in overlap with equivalent rules in the PHP Coding Standards. While I would argue that most all exceptions should be avoided, the focus of the first section is aimed primarily at function arguments, which tends to cause most uncertainty. It may be an agreeable compromise to remove only the exceptions impacting function arguments, leaving still the array and object key exceptions.

Standards changes should be carefully considered, as it has a large impact on existing code which applies the current standard, and on knowledge of developers who have already become familiar with the standards. However, my experience is that these exceptions aren’t particularly well-understood, and in-fact that eliminating them would be more in spirit with how whitespace is otherwise applied (“When in doubt, space it out.”).

What happens next if these exceptions are removed?

If consensus is reached, the following actions should be taken:

  • Our standards documentation are updated to remove the exception language
  • Our ESLint rules are updated to remove the exception
  • All new commits to WordPress core follow the new standards
  • Old code will only be refactored as part of a larger, codebase wide refactoring effort (see related effort at #41057)

What are your thoughts? Please share in the comments below.

#javascript, #standards