Disable for...of by default, rewrite cases where it matters (facebook…

…#12198)

* Add no-for-of lint rule

* Ignore legit use cases of for..of

* Rewrite for..of in source code

.eslintrc.js

@@ -11,6 +11,7 @@ module.exports = {
 
   plugins: [
     'jest',
+    'no-for-of-loops',
     'react',
     'react-internal',
   ],
@@ -56,6 +57,10 @@ module.exports = {
     // We don't care to do this
     'react/jsx-wrap-multilines': [ERROR, {declaration: false, assignment: false}],
 
+    // Prevent for...of loops because they require a Symbol polyfill.
+    // You can disable this rule for code that isn't shipped (e.g. build scripts and tests).
+    'no-for-of-loops/no-for-of-loops': ERROR,
+
     // CUSTOM RULES
     // the second argument of warning/invariant should be a literal string
     'react-internal/no-primitive-constructors': ERROR,

dangerfile.js

@@ -112,7 +112,7 @@ fetch(commitURL(parentOfOldestCommit)).then(async response => {
 
     // Show a hidden summary table for all diffs
 
-    // eslint-disable-next-line no-var
+    // eslint-disable-next-line no-var,no-for-of-loops/no-for-of-loops
     for (var name of new Set(packagesToShow)) {
       const thisBundleResults = results.filter(r => r.packageName === name);
       const changedFiles = thisBundleResults.filter(

package.json

@@ -56,6 +56,7 @@
     "eslint-plugin-babel": "^3.3.0",
     "eslint-plugin-flowtype": "^2.25.0",
     "eslint-plugin-jest": "^21.6.1",
+    "eslint-plugin-no-for-of-loops": "^1.0.0",
     "eslint-plugin-react": "^6.7.1",
     "eslint-plugin-react-internal": "link:./scripts/eslint-rules/",
     "fbjs": "^0.8.16",

packages/react-dom/src/__tests__/ReactMultiChildReconcile-test.js

@@ -251,6 +251,7 @@ function prepareChildrenArray(childrenArray) {
 function prepareChildrenIterable(childrenArray) {
   return {
     '@@iterator': function*() {
+      // eslint-disable-next-line no-for-of-loops/no-for-of-loops
       for (const child of childrenArray) {
         yield child;
       }

packages/react-native-renderer/src/__mocks__/FabricUIManager.js

@@ -20,14 +20,17 @@ const RCTFabricUIManager = {
       let out = '';
       out +=
         ' '.repeat(indent) + info.viewName + ' ' + JSON.stringify(info.props);
+      // eslint-disable-next-line no-for-of-loops/no-for-of-loops
       for (const child of info.children) {
         out += '\n' + dumpSubtree(child, indent + 2);
       }
       return out;
     }
     let result = [];
+    // eslint-disable-next-line no-for-of-loops/no-for-of-loops
     for (const [rootTag, childSet] of roots) {
       result.push(rootTag);
+      // eslint-disable-next-line no-for-of-loops/no-for-of-loops
       for (const child of childSet) {
         result.push(dumpSubtree(child, 1));
       }

packages/react-native-renderer/src/__mocks__/UIManager.js

@@ -66,6 +66,7 @@ const RCTUIManager = {
       let out = '';
       out +=
         ' '.repeat(indent) + info.viewName + ' ' + JSON.stringify(info.props);
+      // eslint-disable-next-line no-for-of-loops/no-for-of-loops
       for (const child of info.children) {
         out += '\n' + dumpSubtree(child, indent + 2);
       }
@@ -143,6 +144,7 @@ const RCTUIManager = {
       indicesToInsert.push([addAtIndices[i], addChildReactTags[i]]);
     });
     indicesToInsert.sort((a, b) => a[0] - b[0]);
+    // eslint-disable-next-line no-for-of-loops/no-for-of-loops
     for (const [i, tag] of indicesToInsert) {
       insertSubviewAtIndex(parentTag, tag, i);
     }

packages/react-noop-renderer/src/ReactNoop.js

@@ -400,6 +400,7 @@ const ReactNoop = {
     const n = timeout / 5 - 1;
 
     let values = [];
+    // eslint-disable-next-line no-for-of-loops/no-for-of-loops
     for (const value of flushUnitsOfWork(n)) {
       values.push(...value);
     }
@@ -418,6 +419,7 @@ const ReactNoop = {
 
   flushUnitsOfWork(n: number): Array<mixed> {
     let values = [];
+    // eslint-disable-next-line no-for-of-loops/no-for-of-loops
     for (const value of flushUnitsOfWork(n)) {
       values.push(...value);
     }
@@ -427,6 +429,7 @@ const ReactNoop = {
   flushThrough(expected: Array<mixed>): void {
     let actual = [];
     if (expected.length !== 0) {
+      // eslint-disable-next-line no-for-of-loops/no-for-of-loops
       for (const value of flushUnitsOfWork(Infinity)) {
         actual.push(...value);
         if (actual.length >= expected.length) {

packages/react-reconciler/src/__tests__/ReactIncrementalTriangle-test.internal.js

@@ -322,6 +322,7 @@ describe('ReactIncrementalTriangle', () => {
 
     function simulate(...actions) {
       const gen = simulateAndYield();
+      // eslint-disable-next-line no-for-of-loops/no-for-of-loops
       for (let action of actions) {
         gen.next(action);
       }
@@ -407,6 +408,7 @@ ${formatActions(actions)}
 
     function simulateMultipleRoots(...actions) {
       const roots = new Map();
+      // eslint-disable-next-line no-for-of-loops/no-for-of-loops
       for (let rootID of rootIDs) {
         const simulator = TriangleSimulator(rootID);
         const generator = simulator.simulateAndYield();

packages/react-test-renderer/src/ReactTestRenderer.js

@@ -553,12 +553,12 @@ function findAll(
     }
   }
 
-  for (const child of root.children) {
+  root.children.forEach(child => {
     if (typeof child === 'string') {
-      continue;
+      return;
     }
     results.push(...findAll(child, predicate, options));
-  }
+  });
 
   return results;
 }

packages/react/src/ReactElementValidator.js

@@ -260,7 +260,9 @@ function validatePropTypes(element) {
 function validateFragmentProps(fragment) {
   currentlyValidatingElement = fragment;
 
-  for (const key of Object.keys(fragment.props)) {
+  const keys = Object.keys(fragment.props);
+  for (let i = 0; i < keys.length; i++) {
+    const key = keys[i];
     if (!VALID_FRAGMENT_PROPS.has(key)) {
       warning(
         false,

scripts/error-codes/invertObject.js

@@ -20,6 +20,7 @@ function invertObject(targetObj /* : ErrorMap */) /* : ErrorMap */ {
   const result = {};
   const mapKeys = Object.keys(targetObj);
 
+  // eslint-disable-next-line no-for-of-loops/no-for-of-loops
   for (const originalKey of mapKeys) {
     const originalVal = targetObj[originalKey];
 

scripts/rollup/build.js

@@ -461,6 +461,7 @@ async function buildEverything() {
 
   // Run them serially for better console output
   // and to avoid any potential race conditions.
+  // eslint-disable-next-line no-for-of-loops/no-for-of-loops
   for (const bundle of Bundles.bundles) {
     await createBundle(bundle, UMD_DEV);
     await createBundle(bundle, UMD_PROD);

yarn.lock

@@ -1883,6 +1883,10 @@ eslint-plugin-jest@^21.6.1:
   version "21.6.1"
   resolved "https://registry.yarnpkg.com/eslint-plugin-jest/-/eslint-plugin-jest-21.6.1.tgz#adca015bbdb8d23b210438ff9e1cee1dd9ec35df"
 
+eslint-plugin-no-for-of-loops@^1.0.0:
+  version "1.0.0"
+  resolved "https://registry.yarnpkg.com/eslint-plugin-no-for-of-loops/-/eslint-plugin-no-for-of-loops-1.0.0.tgz#a13d91a8f1922f7fefedeab351dc0055994601f6"
+
 "eslint-plugin-react-internal@link:./scripts/eslint-rules":
   version "0.0.0"
   uid ""