Skip to content
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
82 changes: 72 additions & 10 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,12 @@ npm run lint
function fine() { return 'but don’t push it!'; }
```

- [4.11](#4.11) <a name="4.11"></a> Line comments should appear above the line they are commenting, rather than at the end of the line or below it.

> Why? End-of-line comments can be harder to read since they lead to longer lines. Comments above the line you are documenting provides a more natural experience when reading the code, as it follows how we typically read other forms of text.

ESLint rule: [`line-comment-position`](http://eslint.org/docs/rules/line-comment-position.html)

[↑ scrollTo('#table-of-contents')](#table-of-contents)


Expand Down Expand Up @@ -1593,9 +1599,35 @@ npm run lint

- [12.2](#12.2) <a name="12.2"></a> Limit use of generators and proxies, as these don’t transpile well (or at all) to ES5.

- [12.3](#12.3) <a name="12.3"></a> Prefer binary, octal, and hexadecimal literals over using `parseInt`.

ESLint rule: [`prefer-numeric-literals`](http://eslint.org/docs/rules/prefer-numeric-literals.html)

```js
// bad
const twoSixtyFiveInHex = parseInt('1F7', 16);

// good
const twoSixtyFiveInHex = 0x1F7;
```

- [12.4](#12.4) <a name="12.4"></a> Always provide a description when creating a `Symbol`.

> Why? You will get a more descriptive representation of that symbol in most developer tools.

ESLint rule: [`symbol-description`](http://eslint.org/docs/rules/symbol-description.html)

```js
// bad
const badSymbol = Symbol();

// good
const goodSymbol = Symbol('Good!');
```

### Destructuring

- [12.3](#12.3) <a name="12.3"></a> Use object destructuring to retrieve multiple properties from an object.
- [12.5](#12.5) <a name="12.5"></a> Use object destructuring to retrieve multiple properties from an object.

> Why? Destructuring removes a lot of boilerplate and encourages you to refer to properties by the same name everywhere they are referenced.

Expand All @@ -1620,7 +1652,7 @@ npm run lint
}
```

- [12.4](#12.4) <a name="12.4"></a> Use array destructuring rather than manually accessing items by their index. If your array has more than a few entries, and you are selecting only a small number of them, continue to use index notation.
- [12.6](#12.6) <a name="12.6"></a> Use array destructuring rather than manually accessing items by their index. If your array has more than a few entries, and you are selecting only a small number of them, continue to use index notation.

```js
const array = [1, 2];
Expand All @@ -1639,7 +1671,7 @@ npm run lint
const fifthLong = longArray[4];
```

- [12.5](#12.5) <a name="12.5"></a> If you need to return multiple values from a function, return them using an object rather than an array.
- [12.7](#12.7) <a name="12.7"></a> If you need to return multiple values from a function, return them using an object rather than an array.

> Why? Call sites that use destructuring to access your return values need to care about the ordering when returning an array, making them fragile to change.

Expand All @@ -1661,7 +1693,7 @@ npm run lint
const {left, top} = positionForNode(node);
```

- [12.6](#12.6) <a name="12.6"></a> You can create highly readable functions by using one positional argument, followed by a destructured object. You can even provide different local names for the destructured arguments to have both an expressive external API and concise internal references.
- [12.8](#12.8) <a name="12.8"></a> You can create highly readable functions by using one positional argument, followed by a destructured object. You can even provide different local names for the destructured arguments to have both an expressive external API and concise internal references.

```js
// fine, but too many positional arguments, so it's hard for call sites to know what to do
Expand All @@ -1688,7 +1720,7 @@ npm run lint

### Classes

- [12.7](#12.7) <a name="12.7"></a> Use classes with care: they do not behave in exactly the way you would expect in other languages, and JavaScript provides many mechanisms (closures, simple objects, etc) that solve problems for which you might use a class in another language. The rule of thumb is: use the right tool for the job!
- [12.9](#12.9) <a name="12.9"></a> Use classes with care: they do not behave in exactly the way you would expect in other languages, and JavaScript provides many mechanisms (closures, simple objects, etc) that solve problems for which you might use a class in another language. The rule of thumb is: use the right tool for the job!

```js
// bad
Expand Down Expand Up @@ -1731,7 +1763,7 @@ npm run lint
const result = takeAction({first: 'foo', second: 'bar'});
```

- [12.8](#12.8) <a name="12.8"></a> If you want to use constructor functions, use `class` syntax. Avoid creating them by manually updating the prototype.
- [12.10](#12.10) <a name="12.10"></a> If you want to use constructor functions, use `class` syntax. Avoid creating them by manually updating the prototype.

> Why? `class` syntax is more concise and will be more familiar for developers trained in other languages.

Expand All @@ -1755,7 +1787,7 @@ npm run lint
}
```

- [12.9](#12.9) <a name="12.9"></a> If you are subclassing, your subclass’s constructor should always call `super` before referencing `this`.
- [12.11](#12.11) <a name="12.11"></a> If you are subclassing, your subclass’s constructor should always call `super` before referencing `this`.

> Why? If your forget to call `super` in your subclass constructor, your object will be uninitialized and calling `this` will result in an exception.

Expand All @@ -1781,7 +1813,7 @@ npm run lint
}
```

- [12.10](#12.10) <a name="12.10"></a> When declaring static members or properties, prefer the `static` keyword to direct assignment to the class object. Put `static` members at the top of your class definition.
- [12.12](#12.12) <a name="12.12"></a> When declaring static members or properties, prefer the `static` keyword to direct assignment to the class object. Put `static` members at the top of your class definition.

> Why? Using the `static` keyword is more expressive and keeps the entire class definition in one place.

Expand All @@ -1805,9 +1837,39 @@ npm run lint
}
```

- [12.13](#12.13) <a name="12.13"></a> Instance methods that don’t refer to `this` don’t need to be instance methods. If they relate to the class, make them static methods; otherwise, make them functions in scope.

> Why? This pattern is generally a sign that you are providing a bad public API for the class, and should either hide this method (if it’s an implementation detail) or export it as a utility method.

ESLint rule: [`class-methods-use-this`](http://eslint.org/docs/rules/class-methods-use-this.html)

```js
// bad
class BadClass {
badMethod(string) {
console.log(string.toUpperCase());
}

anotherMethod() {
return this.badMethod('oops!');
}
}

// good
function goodFunction(string) {
console.log(string.toUpperCase());
}

class GoodClass {
anotherMethod() {
return goodFunction('oops!');
}
}
```

### Modules

- [12.11](#12.11) <a name="12.11"></a> Always use modules (`import`/ `export`) over a non-standard module system (CommonJS being the most popular of these).
- [12.14](#12.14) <a name="12.14"></a> Always use modules (`import`/ `export`) over a non-standard module system (CommonJS being the most popular of these).

> Why? Modules are the future, so let’s get a head start. You can always transpile to a preferred module system.

Expand All @@ -1821,7 +1883,7 @@ npm run lint
export default feelGoodAboutIt();
```

- [12.12](#12.12) <a name="12.12"></a> Avoid complex relative import paths. It is usually fairly easy and much clearer to add the root of your project to the load path.
- [12.15](#12.15) <a name="12.15"></a> Avoid complex relative import paths. It is usually fairly easy and much clearer to add the root of your project to the load path.

> Why? Relative paths are fragile and hard to parse for humans.

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
"babel-preset-shopify": "file:./packages/babel-preset-shopify",
"chai": "3.x",
"coveralls": "2.x",
"eslint": "3.3.x",
"eslint": "3.7.x",
"eslint-plugin-shopify": "file:./packages/eslint-plugin-shopify",
"isparta": "4.x",
"istanbul": "0.x",
Expand Down
16 changes: 8 additions & 8 deletions packages/babel-preset-shopify/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,21 @@
]
},
"dependencies": {
"babel-plugin-transform-es2015-destructuring": "^6.9.0",
"babel-plugin-transform-es2015-destructuring": "^6.16.0",
"babel-plugin-transform-es2015-function-name": "^6.9.0",
"babel-plugin-transform-es2015-modules-commonjs": "^6.14.0",
"babel-plugin-transform-es2015-parameters": "^6.11.4",
"babel-plugin-transform-es2015-modules-commonjs": "^6.16.0",
"babel-plugin-transform-es2015-parameters": "^6.17.0",
"babel-plugin-transform-es2015-shorthand-properties": "^6.8.0",
"babel-plugin-transform-es2015-spread": "^6.8.0",
"babel-plugin-transform-es2015-sticky-regex": "^6.8.0",
"babel-plugin-transform-es2015-unicode-regex": "^6.11.0",
"babel-plugin-transform-export-extensions": "^6.5.0",
"babel-plugin-transform-inline-environment-variables": "^6.5.0",
"babel-preset-es2015": "^6.9.0",
"babel-preset-es2016": "^6.11.3",
"babel-preset-es2017": "^6.14.0",
"babel-preset-react": "^6.5.0",
"babel-preset-stage-2": "^6.5.0",
"babel-preset-es2015": "^6.16.0",
"babel-preset-es2016": "^6.16.0",
"babel-preset-es2017": "^6.16.0",
"babel-preset-react": "^6.16.0",
"babel-preset-stage-2": "^6.17.0",
"semver": "^5.3.0"
}
}
1 change: 1 addition & 0 deletions packages/chai-jscodeshift/test/test-helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ global.expect = expect;
global.assert = assert;
global.sinon = sinon.sandbox.create();

// eslint-disable-next-line mocha/no-top-level-hooks
afterEach(() => {
sinon.restore();
});
6 changes: 3 additions & 3 deletions packages/esify/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@
},
"bin": "./bin/esify",
"dependencies": {
"babel-register": "^6.9.0",
"babel-register": "^6.16.3",
"chalk": "^1.1.3",
"glob": "^7.0.5",
"glob": "^7.1.1",
"js-codemod": "cpojer/js-codemod",
"jscodeshift": "^0.3.20",
"jscodeshift": "^0.3.29",
"shopify-codemod": "latest",
"shopify-decaf": "latest"
}
Expand Down
2 changes: 1 addition & 1 deletion packages/eslint-plugin-shopify/lib/config/all.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ module.exports = {
},

parserOptions: {
ecmaVersion: 7,
ecmaVersion: 2017,
sourceType: 'module',
ecmaFeatures: {
jsx: true,
Expand Down
2 changes: 1 addition & 1 deletion packages/eslint-plugin-shopify/lib/config/ava.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ module.exports = {
},

parserOptions: {
ecmaVersion: 7,
ecmaVersion: 2017,
sourceType: 'module',
},

Expand Down
2 changes: 1 addition & 1 deletion packages/eslint-plugin-shopify/lib/config/esnext.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ module.exports = {
},

parserOptions: {
ecmaVersion: 7,
ecmaVersion: 2017,
sourceType: 'module',
ecmaFeatures: {
experimentalObjectRestSpread: true,
Expand Down
8 changes: 1 addition & 7 deletions packages/eslint-plugin-shopify/lib/config/flow.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ module.exports = {
},

parserOptions: {
ecmaVersion: 7,
ecmaVersion: 2017,
sourceType: 'module',
},

Expand All @@ -17,12 +17,6 @@ module.exports = {
'shopify',
],

settings: {
flowtype: {
onlyFilesWithFlowAnnotation: true,
},
},

rules: merge(
require('./rules/flowtype')
),
Expand Down
4 changes: 4 additions & 0 deletions packages/eslint-plugin-shopify/lib/config/rules/ava.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ module.exports = {
'ava/max-asserts': ['warn', 5],
// Ensure no test.cb() is used.
'ava/no-cb-test': 'warn',
// Ensure that async tests use await
'ava/no-async-fn-without-await': 'off',
// Ensure tests do not have duplicate modifiers.
'ava/no-duplicate-modifiers': 'error',
// Ensure no tests have the same title.
'ava/no-identical-title': 'error',
// Ensure no tests are written in ignored files.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ module.exports = {
'array-callback-return': 'error',
// Treat var statements as if they were block scoped
'block-scoped-var': 'warn',
// Enforce that class methods utilize this
'class-methods-use-this': 'warn',
// Specify the maximum cyclomatic complexity allowed in a program
'complexity': 'off',
// Require return statements to either always or never specify values
Expand Down Expand Up @@ -94,6 +96,8 @@ module.exports = {
'no-proto': 'error',
// Disallow declaring the same variable more than once
'no-redeclare': 'error',
// Disallow certain object properties
'no-restricted-properties': 'off',
// Disallow use of assignment in return statement
'no-return-assign': 'error',
// Disallow use of javascript: urls.,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ module.exports = {
'prefer-arrow-callback': ['error', {allowNamedFunctions: true}],
// Suggest using of const declaration for variables that are never modified after declared
'prefer-const': 'warn',
// Disallow parseInt() in favor of binary, octal, and hexadecimal literals
'prefer-numeric-literals': 'error',
// Suggest using the rest parameters instead of arguments
'prefer-rest-params': 'error',
// Suggest using the spread operator instead of .apply()
Expand All @@ -51,6 +53,10 @@ module.exports = {
'rest-spread-spacing': ['warn', 'never'],
// Disallow generator functions that do not have yield
'require-yield': 'error',
// Sort import declarations within module
'sort-imports': 'off',
// Require symbol descriptions
'symbol-description': 'warn',
// Enforce spacing around embedded expressions of template strings
'template-curly-spacing': ['warn', 'never'],
// Enforce spacing around the * in yield* expressions
Expand Down
24 changes: 23 additions & 1 deletion packages/eslint-plugin-shopify/lib/config/rules/flowtype.js
Original file line number Diff line number Diff line change
@@ -1,20 +1,42 @@
// see https://github.com/gajus/eslint-plugin-flowtype

module.exports = {
// Enforces a particular style for boolean type annotations.
'flowtype/boolean-style': ['warn', 'boolean'],
// Marks Flow type identifiers as defined.
'flowtype/define-flow-type': 'error',
// Enforces consistent use of trailing commas in Object and Tuple annotations.
'flowtype/delimiter-dangle': ['warn', 'always-multiline'],
// Enforces consistent spacing within generic type annotation parameters.
'flowtype/generic-spacing': ['warn', 'never'],
// Checks for duplicate properties in Object annotations
'flowtype/no-dupe-keys': 'error',
// Warns against weak type annotations any, Object and Function.
'flowtype/no-weak-types': ['error', {
any: false,
Object: false,
Function: true,
}],
// Requires that all function parameters have type annotations.
'flowtype/require-parameter-type': 'off',
// Requires that functions have return type annotation.
'flowtype/require-return-type': 'off',
// Makes sure that files have a valid @flow annotation.
'flowtype/require-valid-file-annotation': ['warn', 'always'],
// Enforces consistent use of semicolons after type aliases.
'flowtype/semi': ['warn', 'always'],
// Enforces sorting of Object annotations
'flowtype/sort-keys': 'off',
// Enforces consistent spacing after the type annotation colon.
'flowtype/space-after-type-colon': ['warn', 'always'],
// Enforces consistent spacing before the type annotation colon.
'flowtype/space-before-type-colon': ['warn', 'never'],
// Enforces consistent spacing before the opening < of generic type annotation parameters.
'flowtype/space-before-generic-bracket': ['warn', 'never'],
// Enforces a consistent naming pattern for type aliases.
'flowtype/type-id-match': ['warn', '^([A-Z][a-z0-9]+)+(Type|Props|State|Context)$'],
'flowtype/type-id-match': ['warn', '^([A-Z][a-z0-9]*)*(Type|Props|State|Context)$'],
// Enforces consistent spacing around union and intersection type separators (| and &).
'flowtype/union-intersection-spacing': 'warn',
// Marks Flow type alias declarations as used.
'flowtype/use-flow-type': 'error',
// Checks for simple Flow syntax errors.
Expand Down
Loading