Skip to content

Commit

Permalink
Disable for...of by default, rewrite cases where it matters (facebook…
Browse files Browse the repository at this point in the history
…#12198)

* Add no-for-of lint rule

* Ignore legit use cases of for..of

* Rewrite for..of in source code
  • Loading branch information
gaearon authored and rhagigi committed Apr 19, 2018
1 parent 15544f0 commit c8a049f
Show file tree
Hide file tree
Showing 13 changed files with 30 additions and 5 deletions.
5 changes: 5 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ module.exports = {

plugins: [
'jest',
'no-for-of-loops',
'react',
'react-internal',
],
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion dangerfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down
2 changes: 2 additions & 0 deletions packages/react-native-renderer/src/__mocks__/UIManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down
3 changes: 3 additions & 0 deletions packages/react-noop-renderer/src/ReactNoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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();
Expand Down
6 changes: 3 additions & 3 deletions packages/react-test-renderer/src/ReactTestRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
4 changes: 3 additions & 1 deletion packages/react/src/ReactElementValidator.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions scripts/error-codes/invertObject.js
Original file line number Diff line number Diff line change
Expand Up @@ -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];

Expand Down
1 change: 1 addition & 0 deletions scripts/rollup/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 4 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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 ""
Expand Down

0 comments on commit c8a049f

Please sign in to comment.