Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

eslint-plugin: Enforce object-shorthand #13400

Merged
merged 4 commits into from
Jan 22, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 17 additions & 63 deletions docs/contributors/coding-guidelines.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,76 +102,30 @@ If an API must be exposed but is clearly not intended to be supported into the f
export { __unstableDoAction } from './api';
```

### Variable Naming
### Objects

Gutenberg inherits [WordPress' naming conventions of camel-casing](https://make.wordpress.org/core/handbook/best-practices/coding-standards/javascript/#naming-conventions):

>Variable and function names should be full words, using camel case with a lowercase first letter. This is an area where this standard differs from the WordPress PHP coding standards.
>
>Constructors intended for use with `new` should have a capital first letter (UpperCamelCase).

However, Gutenberg is more specific about its handling of abbreviations, acronyms, constants, and the ES2015 class construct.

#### Abbreviations and Acronyms

[*Abbreviations*](https://en.wikipedia.org/wiki/Abbreviation) must be written as camel case, with an initial capitalized letter followed by lowercase letters.

[*Acronyms*](https://en.wikipedia.org/wiki/Acronym) must be written with each of its composing letters capitalized. This is intended to reflect that each letter of the acronym is a proper word in its expanded form.

If an abbreviation or an acronym occurs at the start of a variable name, it must be written to respect the camelcase naming rules covering the first letter of a variable or class definition. For variable assignment, this means writing the abbreviation entirely as lowercase. For class definitions, its initial letter should be capitalized.

**Examples:**

```js
// "Id" is an abbreviation of "Identifier":
const userId = 1;

// "DOM" is an acronym of "Document Object Model":
const currentDOMDocument = window.document;

// Acronyms and abbreviations at the start of a variable name are consistent
// with camelcase rules covering the first letter of a variable or class.
const domDocument = window.document;
class DOMDocument {}
class IdCollection {}
```

#### Class Definition

A [`class` definition](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes) must use the UpperCamelCase convention, regardless of whether it is intended to be used with `new` construction.

**Example:**
When possible, use [shorthand notation](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Object_initializer#New_notations_in_ECMAScript_2015) when defining object property values:

```js
class Earth {
static addHuman( human ) {
Earth.humans.push( human );
}

static getHumans() {
return Earth.humans;
}
}

Earth.humans = [];
```

All `@wordpress/element` Components, including stateless function components, should be named using Class Definition naming rules, both for consistency and to reflect the fact that a component may need to be transitioned from a function to a class without breaking compatibility.

**Examples:**
const a = 10;

```js
class MyComponent extends Component {}
// Bad:
const object = {
a: a,
performAction: function() {
// ...
},
};

function MyComponent() {}
// Good:
const object = {
a,
performAction() {
// ...
},
};
```

#### Constants

An exception to camel case is made for constant values which are never intended to be reassigned or mutated. Such variables must use the [SCREAMING_SNAKE_CASE convention](https://en.wikipedia.org/wiki/Snake_case).

In almost all cases, a constant should be defined in the top-most scope of a file. It is important to note that [JavaScript's `const` assignment](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/const) is conceptually more limited than what is implied here, where a value assigned by `const` in JavaScript can in-fact be mutated, and is only protected against reassignment. A constant as defined in these coding guidelines applies only to values which are expected to never change, and is a strategy for developers to communicate intent more so than it is a technical restriction.

### Strings

String literals should be declared with single-quotes *unless* the string itself contains a single-quote that would need to be escaped–in that case: use a double-quote. If the string contains a single-quote *and* a double-quote, you can use ES6 template strings to avoid escaping the quotes.
Expand Down
6 changes: 3 additions & 3 deletions docs/tool/manifest.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,10 @@ function generateRootManifestFromTOCItems( items, parent = null ) {
}

pageItems.push( {
title: title,
slug: slug,
title,
slug,
markdown_source: `${ baseRepoUrl }\/${ fileName }`,
parent: parent,
parent,
} );
if ( Array.isArray( children ) && children.length ) {
pageItems = pageItems.concat( generateRootManifestFromTOCItems( children, slug ) );
Expand Down
2 changes: 1 addition & 1 deletion packages/block-library/src/classic/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ export default class ClassicEdit extends Component {
editor.addButton( 'kitchensink', {
tooltip: _x( 'More', 'button to expand options' ),
icon: 'dashicon dashicons-editor-kitchensink',
onClick: function() {
onClick() {
const button = this;
const active = ! button.active();

Expand Down
2 changes: 1 addition & 1 deletion packages/block-library/src/html/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export const settings = {
],
},

edit: edit,
edit,

save( { attributes } ) {
return <RawHTML>{ attributes.content }</RawHTML>;
Expand Down
4 changes: 2 additions & 2 deletions packages/block-library/src/paragraph/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,9 @@ export const settings = {
'has-drop-cap': dropCap,
} );
const styles = {
backgroundColor: backgroundColor,
backgroundColor,
color: textColor,
fontSize: fontSize,
fontSize,
textAlign: align,
};

Expand Down
4 changes: 2 additions & 2 deletions packages/block-library/src/verse/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export const settings = {
content: nextContent,
} );
} }
style={ { textAlign: textAlign } }
style={ { textAlign } }
placeholder={ __( 'Write…' ) }
wrapperClassName={ className }
onMerge={ mergeBlocks }
Expand All @@ -91,7 +91,7 @@ export const settings = {
return (
<RichText.Content
tagName="pre"
style={ { textAlign: textAlign } }
style={ { textAlign } }
value={ content }
/>
);
Expand Down
8 changes: 4 additions & 4 deletions packages/components/src/button/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,16 @@ const styles = StyleSheet.create( {
fontWeight: 'bold',
fontSize: 13,
alignSelf: 'flex-end',
marginLeft: marginLeft,
marginBottom: marginBottom,
marginLeft,
marginBottom,
},
subscriptActive: {
color: 'white',
fontWeight: 'bold',
fontSize: 13,
alignSelf: 'flex-end',
marginLeft: marginLeft,
marginBottom: marginBottom,
marginLeft,
marginBottom,
},
} );

Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/color-palette/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export default function ColorPalette( { colors, disableCustomColors = false, val
return (
<div className={ classes }>
{ map( colors, ( { color, name } ) => {
const style = { color: color };
const style = { color };
const itemClasses = classnames( 'components-color-palette__item', { 'is-active': value === color } );

return (
Expand Down
4 changes: 2 additions & 2 deletions packages/components/src/form-token-field/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ describe( 'FormTokenField', function() {
TestUtils.Simulate.keyDown(
wrapperElement(),
{
keyCode: keyCode,
keyCode,
shiftKey: ! ! shiftKey,
}
);
Expand All @@ -58,7 +58,7 @@ describe( 'FormTokenField', function() {
function sendKeyPress( charCode ) {
TestUtils.Simulate.keyPress(
wrapperElement(),
{ charCode: charCode }
{ charCode }
);
}

Expand Down
2 changes: 1 addition & 1 deletion packages/edit-post/src/components/header/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export default compose(

return {
openGeneralSidebar: () => openGeneralSidebar( getBlockSelectionStart() ? 'edit-post/block' : 'edit-post/document' ),
closeGeneralSidebar: closeGeneralSidebar,
closeGeneralSidebar,
};
} ),
)( Header );
4 changes: 2 additions & 2 deletions packages/editor/src/components/block-draggable/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ const BlockDraggable = ( { children, clientId, rootClientId, blockElementId, ind
{
( { onDraggableStart, onDraggableEnd } ) => {
return children( {
onDraggableStart: onDraggableStart,
onDraggableEnd: onDraggableEnd,
onDraggableStart,
onDraggableEnd,
} );
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/editor/src/components/inserter/test/menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ const items = [

const DEFAULT_PROPS = {
position: 'top center',
items: items,
items,
debouncedSpeak: noop,
fetchReusableBlocks: noop,
setTimeout: noop,
Expand Down
14 changes: 7 additions & 7 deletions packages/editor/src/editor-styles/ast/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export default function( css, options ) {
*/

function position() {
const start = { line: lineno, column: column };
const start = { line: lineno, column };
return function( node ) {
node.position = new Position( start );
whitespace();
Expand All @@ -50,7 +50,7 @@ export default function( css, options ) {

function Position( start ) {
this.start = start;
this.end = { line: lineno, column: column };
this.end = { line: lineno, column };
this.source = options.source;
}

Expand Down Expand Up @@ -351,8 +351,8 @@ export default function( css, options ) {

return pos( {
type: 'keyframes',
name: name,
vendor: vendor,
name,
vendor,
keyframes: frames,
} );
}
Expand Down Expand Up @@ -382,7 +382,7 @@ export default function( css, options ) {

return pos( {
type: 'supports',
supports: supports,
supports,
rules: style,
} );
}
Expand Down Expand Up @@ -440,7 +440,7 @@ export default function( css, options ) {

return pos( {
type: 'media',
media: media,
media,
rules: style,
} );
}
Expand Down Expand Up @@ -527,7 +527,7 @@ export default function( css, options ) {
return pos( {
type: 'document',
document: doc,
vendor: vendor,
vendor,
rules: style,
} );
}
Expand Down
6 changes: 6 additions & 0 deletions packages/eslint-plugin/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
## 2.0.0 (Unreleased)

### Breaking Changes

- The `esnext` and `recommended` rulesets now enforce [`object-shorthand`](https://eslint.org/docs/rules/object-shorthand)

## 1.0.0 (2018-12-12)

### New Features
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/configs/esnext.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ module.exports = {
'no-useless-computed-key': 'error',
'no-useless-constructor': 'error',
'no-var': 'error',
'object-shorthand': 'error',
'prefer-const': 'error',
quotes: [ 'error', 'single', { allowTemplateLiterals: true, avoidEscape: true } ],
'space-unary-ops': [ 'error', {
Expand Down
4 changes: 2 additions & 2 deletions packages/hooks/src/createHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ function createHooks() {
doingFilter: createDoingHook( filters ),
didAction: createDidHook( actions ),
didFilter: createDidHook( filters ),
actions: actions,
filters: filters,
actions,
filters,
};
}

Expand Down
2 changes: 1 addition & 1 deletion packages/rich-text/src/create.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ export function create( {
if ( typeof text === 'string' && text.length > 0 ) {
return {
formats: Array( text.length ),
text: text,
text,
};
}

Expand Down