refactor: enable and fix fp/no-loops #2852

tinfoil-knight · 2021-07-06T11:23:08Z

- Summary

See #1891

Enables the fp/no-loops for eslint-plugin-fp.

for loops were changed to use forEach or other array methods like map, some or every wherever appropriate.
Retry logic was changed from using loop to being recursive in nature.
4 instances of for loop were ignored due to any of the following reasons:
- async/await doesn't work with forEach
- recursive solutions would be functional but hard to read/understand
- Promise.allSettled is not supported below 12.9.0 (Can replace making await() requests in for loops)

- Test plan

npx ava --serial --verbose ran without any errors.

tinfoil-knight · 2021-07-06T18:48:55Z

All failing tests have been fixed.

tinfoil-knight · 2021-07-06T18:51:05Z

It seems that the ?. operator isn't supported yet for node in the CI. Will need to change it.

@erezrokah What version of node is the CI supporting for tests?

Also, I received a ESLint warning when using Promise.allSettled which is incompatible with current node engine specified but didn't get one for using the optional chaining operator which is only supported for >=14.0.0.

tinfoil-knight · 2021-07-07T06:35:22Z

@erezrokah All tests are passing for the CI except on ubuntu-latest, bash, 10.x.

The error seems to be in the detect-server.js file. I can't reproduce the issue but I'll try pushing a fix.

erezrokah · 2021-07-07T13:04:31Z

@erezrokah What version of node is the CI supporting for tests?

The CLI supports Node.js >= 10.18.0, The optional chaining operator ?. is supported for Node.js >= 14

erezrokah · 2021-07-07T13:05:14Z

The error seems to be in the detect-server.js file. I can't reproduce the issue but I'll try pushing a fix.

You can use nvm to install and switch to Node.js 10 locally.

erezrokah

I've added a few more comments.

In general we should treat fp/no-loops as a guideline and not a strict rule, as sometimes it's better to have a for loop.

src/detectors/utils/jsdetect.js

src/utils/detect-server.js

src/utils/get-global-config.js

tests/utils/dev-server.js

tests/utils/site-builder.js

src/utils/headers.js

src/utils/gitignore.js

src/utils/state-config.js

src/commands/env/list.js

src/commands/env/import.js

tinfoil-knight · 2021-07-07T16:00:02Z

@erezrokah I've made all the requested changes and tested on Node.js 10 w/ npx ava -v -s

…rEach loops with return

src/lib/functions/synchronous.js

erezrokah

🚀

tinfoil-knight · 2021-07-12T17:31:39Z

Thank you @erezrokah for merging my first PR in this repository. More PRs upcoming!

erezrokah · 2021-07-12T17:39:29Z

Great work @tinfoil-knight, looking forward 👍

tinfoil-knight requested a review from a team as a code owner July 6, 2021 11:23

erezrokah added the type: chore label Jul 6, 2021

tinfoil-knight force-pushed the 1891-no-loops branch from 5c26ce3 to 7f40631 Compare July 6, 2021 18:44

tinfoil-knight force-pushed the 1891-no-loops branch from 7f40631 to 9345c09 Compare July 6, 2021 18:55

erezrokah suggested changes Jul 7, 2021

View reviewed changes

tinfoil-knight force-pushed the 1891-no-loops branch 2 times, most recently from 1babb95 to 2a2daa1 Compare July 7, 2021 19:32

tinfoil-knight requested a review from erezrokah July 9, 2021 18:14

tinfoil-knight added 6 commits July 11, 2021 14:15

refactor: enable and fix fp/no-loops

Verified

This commit was signed with the committer’s verified signature.

erezrokah Erez Rokah

GPG key ID: B51CA2E23622ABB2

Verified
Learn about vigilant mode

7d96af7

fix: check before accessing forEach method

Verified

This commit was signed with the committer’s verified signature.

erezrokah Erez Rokah

GPG key ID: B51CA2E23622ABB2

Verified
Learn about vigilant mode

dc2dda0

refactor: use map,filter instead of push, make hasRequiredDeps simpler

Verified

This commit was signed with the committer’s verified signature.

erezrokah Erez Rokah

GPG key ID: B51CA2E23622ABB2

Verified
Learn about vigilant mode

6c70d84

refactor: undo recursive retry in favor of loops

Verified

This commit was signed with the committer’s verified signature.

erezrokah Erez Rokah

GPG key ID: B51CA2E23622ABB2

Verified
Learn about vigilant mode

9c05024

refactor: use addRowMatrix for bulk row addition instead of addRow

Verified

This commit was signed with the committer’s verified signature.

erezrokah Erez Rokah

GPG key ID: B51CA2E23622ABB2

Verified
Learn about vigilant mode

332c088

refactor: remove unused functions from utils/gitignore, stop using fo…

Verified

This commit was signed with the committer’s verified signature.

erezrokah Erez Rokah

GPG key ID: B51CA2E23622ABB2

Verified
Learn about vigilant mode

f760314

…rEach loops with return

erezrokah force-pushed the 1891-no-loops branch from ad6f5a1 to f760314 Compare July 11, 2021 12:15

erezrokah reviewed Jul 11, 2021

View reviewed changes

src/lib/functions/synchronous.js Show resolved Hide resolved

erezrokah mentioned this pull request Jul 11, 2021

fix(command-dev): pass full lambda-local result functions server #2901

Merged

erezrokah added 2 commits July 11, 2021 16:22

Merge branch 'main' into 1891-no-loops

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

GPG key ID: 4AEE18F83AFDEB23
Expired

Verified
Learn about vigilant mode

096b1c3

Merge branch 'main' into 1891-no-loops

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

GPG key ID: 4AEE18F83AFDEB23
Expired

Verified
Learn about vigilant mode

ca87f52

erezrokah approved these changes Jul 12, 2021

View reviewed changes

erezrokah enabled auto-merge (squash) July 12, 2021 15:54

Merge branch 'main' into 1891-no-loops

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

GPG key ID: 4AEE18F83AFDEB23
Expired

Verified
Learn about vigilant mode

aa9700d

erezrokah disabled auto-merge July 12, 2021 16:25

Merge branch 'main' into 1891-no-loops

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

GPG key ID: 4AEE18F83AFDEB23
Expired

Verified
Learn about vigilant mode

c74b6f2

erezrokah enabled auto-merge (squash) July 12, 2021 16:25

erezrokah disabled auto-merge July 12, 2021 16:53

erezrokah merged commit aa6c8fb into netlify:main Jul 12, 2021

tinfoil-knight deleted the 1891-no-loops branch September 24, 2021 15:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: enable and fix fp/no-loops #2852

refactor: enable and fix fp/no-loops #2852

tinfoil-knight commented Jul 6, 2021 •

edited

Loading

tinfoil-knight commented Jul 6, 2021

tinfoil-knight commented Jul 6, 2021 •

edited

Loading

tinfoil-knight commented Jul 7, 2021 •

edited

Loading

erezrokah commented Jul 7, 2021

erezrokah commented Jul 7, 2021

erezrokah left a comment

tinfoil-knight commented Jul 7, 2021

erezrokah left a comment

tinfoil-knight commented Jul 12, 2021

erezrokah commented Jul 12, 2021

refactor: enable and fix fp/no-loops #2852

refactor: enable and fix fp/no-loops #2852

Conversation

tinfoil-knight commented Jul 6, 2021 • edited Loading

tinfoil-knight commented Jul 6, 2021

tinfoil-knight commented Jul 6, 2021 • edited Loading

tinfoil-knight commented Jul 7, 2021 • edited Loading

erezrokah commented Jul 7, 2021

erezrokah commented Jul 7, 2021

erezrokah left a comment

Choose a reason for hiding this comment

tinfoil-knight commented Jul 7, 2021

erezrokah left a comment

Choose a reason for hiding this comment

tinfoil-knight commented Jul 12, 2021

erezrokah commented Jul 12, 2021

tinfoil-knight commented Jul 6, 2021 •

edited

Loading

tinfoil-knight commented Jul 6, 2021 •

edited

Loading

tinfoil-knight commented Jul 7, 2021 •

edited

Loading