Netflix / falcor

A JavaScript library for efficient data fetching
http://netflix.github.io/falcor
Apache License 2.0
10.48k stars 446 forks source link

Update to eslint@1 #437

Open pdehaan opened 9 years ago

pdehaan commented 9 years ago

I git cloned and updated to gulp-eslint@1 locally and got a few errors about deprecated rules and other things and now am getting some more ESLint warnings and errors. (see http://eslint.org/docs/user-guide/migrating-to-1.0.0.html for more migration notes).

NOTE: ESLint@1 disabled default rules, so you may want to add the following to your .eslintrc file as well: "extends": "eslint:recommended".

Here is my diff/patch for the .eslintrc file:

diff --git a/.eslintrc b/.eslintrc
index 3c7c1a6..05c8e38 100644
--- a/.eslintrc
+++ b/.eslintrc
@@ -15,8 +15,8 @@
         "no-dupe-keys": [ 2 ],
         "no-duplicate-case": [ 2 ],
         "no-empty": [ 2 ],
-        "no-empty-class": [ 2 ],
-        "no-ex-assign": [ 2 , "always"],
+        "no-empty-character-class": [ 2 ],
+        "no-ex-assign": [ 2 ],
         "no-extra-boolean-cast": [ 2 ],
         "no-cond-assign": [ 2, "always" ],
         "no-extra-semi": [ 2 ],
@@ -43,7 +43,7 @@
         "consistent-return": [ 2 ],
         "curly": [ 2 ],
         "default-case": [ 2 ],
-        "dot-notation": [ [ 2 ], { "allowKeywords": true } ],
+        "dot-notation": [ 2, { "allowKeywords": true } ],
         "eqeqeq": [ 2 ],
         "guard-for-in": [ 2 ],
         "no-alert": [ 2 ],
@@ -121,7 +121,7 @@
         "no-new-object": [ 2 ],
         "no-underscore-dangle": [ 0 ],
         "no-trailing-spaces": [ 2 ],
-        "no-wrap-func": [ 2 ],
+        "no-extra-parens": [ 0 ],
         "semi": [ 2, "always" ],
         "space-after-keywords": [ 2, "always" ],
         "space-before-blocks": [ 2, "always" ],
@@ -129,7 +129,7 @@
         "space-infix-ops": [ 2 ],
         "space-return-throw-case": [ 2 ],
         "space-unary-ops": [ 2, { "words": true, "nonwords": false } ],
-        "spaced-line-comment": [ 2, "always", { "exceptions": [ "-", "+" ] } ],
+        "spaced-comment": [ 2, "always", { "exceptions": [ "-", "+" ] } ],
         "quotes": [ 2, "double", "avoid-escape" ],
         "wrap-regex": [ 2 ]
     }

I had to disable that 'no-extra-parens` rule, since it was throwing a lot of errors after converting to a newer version of ESLint (so maybe the rule wasn't working before or they changed the logic).

➜  falcor git:(master) ✗ gulp lint
[17:23:20] Using gulpfile ~/dev/tmp/falcor/gulpfile.js
[17:23:20] Starting 'lint'...
[17:23:21] 'lint' errored after 465 ms
[17:23:21] ESLintError in plugin 'gulp-eslint'
Message:
    Expected an object to be thrown.
Details:
    fileName: /Users/pdehaan/dev/tmp/falcor/lib/get/getValueSync.js
    lineNumber: 96
[17:23:23]
/Users/pdehaan/dev/tmp/falcor/lib/Model.js
  383:1  warning  Unexpected todo comment  no-warning-comments
  384:1  warning  Unexpected todo comment  no-warning-comments
  385:1  warning  Unexpected todo comment  no-warning-comments

/Users/pdehaan/dev/tmp/falcor/lib/get/getValueSync.js
  96:9  error  Expected an object to be thrown  no-throw-literal

/Users/pdehaan/dev/tmp/falcor/lib/get/onValue.js
  148:13  warning  Unexpected todo comment  no-warning-comments

/Users/pdehaan/dev/tmp/falcor/lib/lru/collect.js
  32:16  error  Unexpected assignment within an 'if' statement  no-cond-assign

/Users/pdehaan/dev/tmp/falcor/lib/response/IdempotentResponse.js
  133:41  error  ensureCollect is already declared in the upper scope  no-shadow

/Users/pdehaan/dev/tmp/falcor/lib/response/ModelResponse.js
  26:5  warning  Unexpected todo comment  no-warning-comments

/Users/pdehaan/dev/tmp/falcor/lib/set/set-json-values-as-json-values.js
   41:5  warning  Unexpected todo comment  no-warning-comments
   50:5  warning  Unexpected todo comment  no-warning-comments
   78:1  warning  Unexpected todo comment  no-warning-comments
  151:1  warning  Unexpected todo comment  no-warning-comments
  169:9  warning  Unexpected todo comment  no-warning-comments

/Users/pdehaan/dev/tmp/falcor/lib/support/create-branch.js
  9:1  warning  Unexpected todo comment  no-warning-comments

/Users/pdehaan/dev/tmp/falcor/lib/support/merge-node.js
   27:16  error  Unexpected assignment within an 'if' statement  no-cond-assign
   36:12  error  Unexpected assignment within an 'if' statement  no-cond-assign
   54:16  error  Unexpected assignment within an 'if' statement  no-cond-assign
  103:9   error  Unexpected assignment within an 'if' statement  no-cond-assign

/Users/pdehaan/dev/tmp/falcor/lib/support/treat-node-as-missing-path-set.js
  13:13  warning  Unexpected todo comment  no-warning-comments

/Users/pdehaan/dev/tmp/falcor/lib/support/wrap-node.js
   9:1  warning  Unexpected todo comment  no-warning-comments
  10:1  warning  Unexpected todo comment  no-warning-comments

✖ 21 problems (7 errors, 14 warnings)
pdehaan commented 9 years ago

Another nice idea would be to create an ESLint shareable config and then publish your Falcor JavaScript styleguide as an npm module (for example, eslint-config-falcor) that could be shared between all your Netflix/falcor-* repos. That way you wouldn't need to copy/paste your .eslintrc files between repos.

See eslint-config-airbnb and eslint-config-standard for more details.

pdehaan commented 9 years ago

FWIW, here's a YAML version of the current eslint@1-friendly .eslintrc file which extends the eslint@1's "eslint:recommended" rules (with duplicate rules removed):

extends:
  - eslint:recommended

env:
  browser: false
  es6: false
  node: true

rules:
  # possible errors
  comma-dangle: 2
  no-empty-character-class: 2
  no-ex-assign: 2
  no-cond-assign: [ 2, always ]

  # this is for variable hoisting, not necessary if we use block scoped declarations
  # no-inner-declarations: [ 2, both ]
  # when IE8 dies
  no-reserved-keys: 0
  use-isnan: 2

  # should we enforce valid documentation comments?
  # i.e., if you do documentation, do it right
  # valid-jsdoc: 2

  # best practices
  block-scoped-var: 1

  # warning for now until we get them fixed
  consistent-return: 2
  curly: 2
  default-case: 2
  dot-notation: [ 2, { allowKeywords: true } ]
  eqeqeq: 2
  guard-for-in: 2
  no-alert: 2
  no-caller: 2
  no-div-regex: 2
  no-eval: 2
  no-extend-native: 2
  no-extra-bind: 2
  no-floating-decimal: 2
  no-implied-eval: 2
  no-iterator: 2
  no-lone-blocks: 2
  no-loop-func: 2
  no-multi-spaces: 2
  no-native-reassign: 2
  no-new-func: 2
  no-new-wrappers: 2
  no-octal-escape: 2
  no-param-reassign: 2
  no-process-env: 2
  no-proto: 2
  no-return-assign: 2
  no-script-url: 2
  no-self-compare: 2
  no-sequences: 2
  no-throw-literal: 2
  no-unused-expressions: 2
  no-warning-comments: 1
  no-with: 2
  radix: 2
  wrap-iife: 2

  # strict mode
  # strict: [ 2, global ]

  # variables
  no-catch-shadow: 2
  no-shadow: 2
  no-shadow-restricted-names: 2
  no-undef-init: 2
  no-undefined: 2
  no-unused-vars: [ 2, { vars: all, args: none } ]
  no-use-before-define: [ 2, nofunc ]

  # node.js
  handle-callback-err: [ 2, ^.*(e|E)rr ]
  no-mixed-requires: 2
  no-new-require: 2
  no-path-concat: 2

  # ES6
  generator-star-spacing: [ 2, after ]

  # stylistic
  camelcase: [ 2, { properties: always } ]
  eol-last: 2
  no-array-constructor: 2
  no-extra-parens: 2
  no-lonely-if: 2
  no-mixed-spaces-and-tabs: [ 2, smart-tabs ]
  no-nested-ternary: 2
  no-new-object: 2
  no-trailing-spaces: 2
  quotes: [ 2, double, avoid-escape ]
  semi: [ 2, always ]
  space-after-keywords: [ 2, always ]
  space-before-blocks: [ 2, always ]
  space-before-function-paren: [ 2, { anonymous: never, named: never } ]
  space-infix-ops: 2
  space-return-throw-case: 2
  space-unary-ops: [ 2, { words: true, nonwords: false } ]
  spaced-comment: [ 2, always, { exceptions: [ "-", "+" ] } ]
  wrap-regex: 2
gfogle commented 8 years ago

Looks like the current rule for no-ex-assign is out of date as well? should just be 2 instead of [2, "always"]?