Upgrade ESLint and dependencies, fix new lint errors, switch Travis t…

…o Yarn (facebook#8309)

* Update ESLint to 3.10.2

Also pull in fbjs for extending properly, per @zpao. This also disables consistent-return, which has about 80 failing cases in React currently. If we'd like to turn this back on, we should do it separately and fix all the call sites properly (rather than just adding 'return undefined;' everywhere, which adds no value.

Fixes to all existing lint errors plus an update for yarn.lock to follow.

* Update yarn.lock after the eslint update.

* Fix all new eslint failures

Unfortunately I had to add three eslint-disable-next-line instances. All have explanations inline.

* Switch Travis to use yarn instead of npm

.eslintrc.js

@@ -5,69 +5,51 @@ const OFF = 0;
 const ERROR = 2;
 
 module.exports = {
-  parser: 'babel-eslint',
-
-  extends: './node_modules/fbjs-scripts/eslint/.eslintrc.js',
+  extends: 'fbjs',
 
   plugins: [
     'react',
     'react-internal',
   ],
 
-  ecmaFeatures: {
-    modules: false,
-  },
-
   // We're stricter than the default config, mostly. We'll override a few rules
   // and then enable some React specific ones.
   rules: {
     'accessor-pairs': OFF,
     'brace-style': [ERROR, '1tbs'],
     'comma-dangle': [ERROR, 'always-multiline'],
-    'consistent-return': ERROR,
+    'consistent-return': OFF,
     'dot-location': [ERROR, 'property'],
     'dot-notation': ERROR,
     'eol-last': ERROR,
     'eqeqeq': [ERROR, 'allow-null'],
     'indent': [ERROR, 2, {SwitchCase: 1}],
     'jsx-quotes': [ERROR, 'prefer-double'],
+    'keyword-spacing': [ERROR, {after: true, before: true}],
     'no-bitwise': OFF,
     'no-inner-declarations': [ERROR, 'functions'],
     'no-multi-spaces': ERROR,
     'no-restricted-syntax': [ERROR, 'WithStatement'],
     'no-shadow': ERROR,
     'no-unused-expressions': ERROR,
     'no-unused-vars': [ERROR, {args: 'none'}],
-    'quotes': [ERROR, 'single', 'avoid-escape'],
-    'space-after-keywords': ERROR,
+    'quotes': [ERROR, 'single', {avoidEscape: true, allowTemplateLiterals: true }],
     'space-before-blocks': ERROR,
     'space-before-function-paren': [ERROR, {anonymous: 'never', named: 'never'}],
-    'space-before-keywords': ERROR,
-    'strict': [ERROR, 'global'],
 
     // React & JSX
     // Our transforms set this automatically
-    'react/display-name': OFF,
     'react/jsx-boolean-value': [ERROR, 'always'],
     'react/jsx-no-undef': ERROR,
     // We don't care to do this
     'react/jsx-sort-prop-types': OFF,
-    'react/jsx-sort-props': OFF,
     'react/jsx-uses-react': ERROR,
-    'react/jsx-uses-vars': ERROR,
-    // It's easier to test some things this way
-    'react/no-did-mount-set-state': OFF,
-    'react/no-did-update-set-state': OFF,
-    // We define multiple components in test files
-    'react/no-multi-comp': OFF,
-    'react/no-unknown-property': OFF,
+    'react/no-is-mounted': OFF,
     // This isn't useful in our test code
-    'react/prop-types': OFF,
     'react/react-in-jsx-scope': ERROR,
     'react/self-closing-comp': ERROR,
     // We don't care to do this
-    'react/sort-comp': OFF,
-    'react/wrap-multilines': [ERROR, {declaration: false, assignment: false}],
+    'react/jsx-wrap-multilines': [ERROR, {declaration: false, assignment: false}],
 
     // CUSTOM RULES
     // the second argument of warning/invariant should be a literal string

.travis.yml

@@ -3,13 +3,14 @@ sudo: required
 dist: trusty
 language: node_js
 node_js:
-- 4
+- 6
 rvm:
 - 2.2.3
 cache:
   directories:
   - docs/vendor/bundle
   - node_modules
+  - $HOME/.yarn-cache
 before_install:
 - |
     if [ "$TEST_TYPE" != build_website ] && \
@@ -18,8 +19,15 @@ before_install:
       echo "Only docs were updated, stopping build process."
       exit
     fi
-    npm install -g npm@latest-2
-    npm --version
+
+    sudo apt-key adv --fetch-keys http://dl.yarnpkg.com/debian/pubkey.gpg
+    echo "deb http://dl.yarnpkg.com/debian/ stable main" | sudo tee /etc/apt/sources.list.d/yarn.list
+    sudo apt-get update -qq
+    sudo apt-get install -y -qq yarn
+    yarn --version
+install:
+- |
+    yarn install
 script:
 - |
     if [ "$TEST_TYPE" = build_website ]; then

package.json

@@ -8,7 +8,7 @@
     "async": "^1.5.0",
     "babel-cli": "^6.6.5",
     "babel-core": "^6.0.0",
-    "babel-eslint": "^5.0.0",
+    "babel-eslint": "^7.1.0",
     "babel-plugin-check-es2015-constants": "^6.5.0",
     "babel-plugin-syntax-trailing-function-commas": "^6.5.0",
     "babel-plugin-transform-class-properties": "^6.11.5",
@@ -40,8 +40,11 @@
     "coveralls": "^2.11.6",
     "del": "^2.0.2",
     "derequire": "^2.0.3",
-    "eslint": "1.10.3",
-    "eslint-plugin-react": "4.1.0",
+    "eslint": "^3.10.2",
+    "eslint-config-fbjs": "^1.1.1",
+    "eslint-plugin-babel": "^3.3.0",
+    "eslint-plugin-flowtype": "^2.25.0",
+    "eslint-plugin-react": "^6.7.1",
     "eslint-plugin-react-internal": "file:eslint-rules",
     "fbjs": "^0.8.5",
     "fbjs-scripts": "^0.6.0",

scripts/facts-tracker/index.js

@@ -160,7 +160,7 @@ for (var i = 2; i < process.argv.length; i += 2) {
     fs.appendFileSync(
       path.resolve(cwd, filename),
       currentCommitHash + '\t' + currentTimestamp + '\t' + value + '\n'
-    );    
+    );
   }
 
   console.log(name);

src/addons/__tests__/renderSubtreeIntoContainer-test.js

@@ -70,6 +70,9 @@ describe('renderSubtreeIntoContainer', () => {
       }
     }
 
+    // ESLint is confused here and thinks Parent is unused, presumably because
+    // it is only used inside of the class body?
+    // eslint-disable-next-line no-unused-vars
     class Parent extends React.Component {
       static childContextTypes = {
         foo: React.PropTypes.string.isRequired,

src/isomorphic/children/__tests__/ReactChildren-test.js

@@ -399,16 +399,14 @@ describe('ReactChildren', () => {
     // 2. If grouped in an Array, the `key` prop, falling back to array index
 
     var instance = (
-      <div>{
-        [
-          ReactFragment.create({
-            firstHalfKey: [zero, one, two],
-            secondHalfKey: [three, four],
-            keyFive: five,
-          }),
-          null,
-        ]
-      }</div>
+      <div>{[
+        ReactFragment.create({
+          firstHalfKey: [zero, one, two],
+          secondHalfKey: [three, four],
+          keyFive: five,
+        }),
+        null,
+      ]}</div>
     );
     var numberOfChildren = ReactChildren.count(instance.props.children);
     expect(numberOfChildren).toBe(5);

src/isomorphic/classic/class/__tests__/ReactClassMixin-test.js

@@ -124,7 +124,7 @@ describe('ReactClass-mixin', () => {
   it('should support chaining delegate functions', () => {
     var listener = jest.fn();
     var instance = <TestComponent listener={listener} />;
-    instance = ReactTestUtils.renderIntoDocument(instance);
+    ReactTestUtils.renderIntoDocument(instance);
 
     expect(listener.mock.calls).toEqual([
       ['MixinA didMount'],
@@ -137,7 +137,7 @@ describe('ReactClass-mixin', () => {
   it('should chain functions regardless of spec property order', () => {
     var listener = jest.fn();
     var instance = <TestComponentWithReverseSpec listener={listener} />;
-    instance = ReactTestUtils.renderIntoDocument(instance);
+    ReactTestUtils.renderIntoDocument(instance);
 
     expect(listener.mock.calls).toEqual([
       ['MixinA didMount'],
@@ -181,8 +181,7 @@ describe('ReactClass-mixin', () => {
         return <span />;
       },
     });
-    var instance = <Component />;
-    instance = ReactTestUtils.renderIntoDocument(instance);
+    var instance = ReactTestUtils.renderIntoDocument(<Component />);
     expect(instance.state.component).toBe(true);
     expect(instance.state.mixin).toBe(true);
   });
@@ -202,9 +201,8 @@ describe('ReactClass-mixin', () => {
         return <span />;
       },
     });
-    var instance = <Component />;
     expect(function() {
-      instance = ReactTestUtils.renderIntoDocument(instance);
+      ReactTestUtils.renderIntoDocument(<Component />);
     }).toThrowError(
       'mergeIntoWithNoDuplicateKeys(): Tried to merge two objects with the ' +
       'same key: `x`. This conflict may be due to a mixin; in particular, ' +
@@ -450,8 +448,7 @@ describe('ReactClass-mixin', () => {
         return <span />;
       },
     });
-    var instance = <Component />;
-    instance = ReactTestUtils.renderIntoDocument(instance);
+    ReactTestUtils.renderIntoDocument(<Component />);
   });
 
   it('should include the mixin keys in even if their values are falsy', () => {
@@ -470,8 +467,7 @@ describe('ReactClass-mixin', () => {
         return <span />;
       },
     });
-    var instance = <Component />;
-    instance = ReactTestUtils.renderIntoDocument(instance);
+    ReactTestUtils.renderIntoDocument(<Component />);
   });
 
   it('should work with a null getInitialState return value and a mixin', () => {
@@ -496,8 +492,7 @@ describe('ReactClass-mixin', () => {
       () => ReactTestUtils.renderIntoDocument(<Component />)
     ).not.toThrow();
 
-    instance = <Component />;
-    instance = ReactTestUtils.renderIntoDocument(instance);
+    instance = ReactTestUtils.renderIntoDocument(<Component />);
     expect(instance.state).toEqual({foo: 'bar'});
 
     // Also the other way round should work
@@ -519,8 +514,7 @@ describe('ReactClass-mixin', () => {
       () => ReactTestUtils.renderIntoDocument(<Component />)
     ).not.toThrow();
 
-    instance = <Component />;
-    instance = ReactTestUtils.renderIntoDocument(instance);
+    instance = ReactTestUtils.renderIntoDocument(<Component />);
     expect(instance.state).toEqual({foo: 'bar'});
 
     // Multiple mixins should be fine too
@@ -537,8 +531,7 @@ describe('ReactClass-mixin', () => {
       () => ReactTestUtils.renderIntoDocument(<Component />)
     ).not.toThrow();
 
-    instance = <Component />;
-    instance = ReactTestUtils.renderIntoDocument(instance);
+    instance = ReactTestUtils.renderIntoDocument(<Component />);
     expect(instance.state).toEqual({foo: 'bar', x: true});
   });
 

src/isomorphic/classic/types/ReactPropTypes.js

@@ -68,9 +68,11 @@ var warning = require('warning');
 
 var ANONYMOUS = '<<anonymous>>';
 
+var ReactPropTypes;
+
 if (__DEV__) {
   // Keep in sync with production version below
-  var ReactPropTypes = {
+  ReactPropTypes = {
     array: createPrimitiveTypeChecker('array'),
     bool: createPrimitiveTypeChecker('boolean'),
     func: createPrimitiveTypeChecker('function'),
@@ -99,7 +101,7 @@ if (__DEV__) {
   productionTypeChecker.isRequired = productionTypeChecker;
   var getProductionTypeChecker = () => productionTypeChecker;
   // Keep in sync with development version above
-  var ReactPropTypes = {
+  ReactPropTypes = {
     array: productionTypeChecker,
     bool: productionTypeChecker,
     func: productionTypeChecker,

src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js

@@ -308,7 +308,7 @@ describe('ReactPropTypes', () => {
       spyOn(console, 'error');
 
       var instance = <Component label={<div />} />;
-      instance = ReactTestUtils.renderIntoDocument(instance);
+      ReactTestUtils.renderIntoDocument(instance);
 
       expectDev(console.error.calls.count()).toBe(0);
     });
@@ -317,7 +317,7 @@ describe('ReactPropTypes', () => {
       spyOn(console, 'error');
 
       var instance = <Component />;
-      instance = ReactTestUtils.renderIntoDocument(instance);
+      ReactTestUtils.renderIntoDocument(instance);
 
       expectDev(console.error.calls.count()).toBe(1);
     });
@@ -851,7 +851,7 @@ describe('ReactPropTypes', () => {
       };
 
       var instance = <Component num={5} />;
-      instance = ReactTestUtils.renderIntoDocument(instance);
+      ReactTestUtils.renderIntoDocument(instance);
 
       expect(spy.calls.count()).toBe(1);
       expect(spy.calls.argsFor(0)[1]).toBe('num');
@@ -868,7 +868,7 @@ describe('ReactPropTypes', () => {
       };
 
       var instance = <Component bla={5} />;
-      instance = ReactTestUtils.renderIntoDocument(instance);
+      ReactTestUtils.renderIntoDocument(instance);
 
       expect(spy.calls.count()).toBe(1);
       expect(spy.calls.argsFor(0)[1]).toBe('num');
@@ -892,7 +892,7 @@ describe('ReactPropTypes', () => {
       };
 
       var instance = <Component num={6} />;
-      instance = ReactTestUtils.renderIntoDocument(instance);
+      ReactTestUtils.renderIntoDocument(instance);
       expectDev(console.error.calls.count()).toBe(1);
       expect(
         console.error.calls.argsFor(0)[0].replace(/\(at .+?:\d+\)/g, '(at **)')
@@ -919,7 +919,7 @@ describe('ReactPropTypes', () => {
         };
 
         var instance = <Component num={5} />;
-        instance = ReactTestUtils.renderIntoDocument(instance);
+        ReactTestUtils.renderIntoDocument(instance);
         expectDev(console.error.calls.count()).toBe(0);
       }
     );

src/isomorphic/classic/types/__tests__/ReactPropTypesProduction-test.js

@@ -212,7 +212,7 @@ describe('ReactPropTypesProduction', function() {
       });
 
       var instance = <Component num={5} />;
-      instance = ReactTestUtils.renderIntoDocument(instance);
+      ReactTestUtils.renderIntoDocument(instance);
 
       expect(spy).not.toBeCalled();
     });