From e5404d2af39d09e2c1dc6fe8f3dc61c7fabf368a Mon Sep 17 00:00:00 2001 From: Pascal Hartig Date: Mon, 23 Aug 2021 05:21:51 -0700 Subject: [PATCH] Add linter for sync function calls Summary: That's another thing I comment on a lot and is a mostly avoidable source of bad perf. Reviewed By: mweststrate Differential Revision: D30450577 fbshipit-source-id: bb82d8cbd34956fa790243f59cda09ff9c4e7379 --- desktop/.eslintrc.js | 1 + docs/internals/linters.mdx | 40 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/desktop/.eslintrc.js b/desktop/.eslintrc.js index c528a3ed2..5fd132590 100644 --- a/desktop/.eslintrc.js +++ b/desktop/.eslintrc.js @@ -100,6 +100,7 @@ module.exports = { 'import/no-unresolved': [2, {commonjs: true, amd: true}], 'node/no-extraneous-import': [2, {allowModules: builtInModules}], 'node/no-extraneous-require': [2, {allowModules: builtInModules}], + 'node/no-sync': [1], 'flipper/no-relative-imports-across-packages': [2], 'flipper/no-electron-remote-imports': [1], 'flipper/no-console-error-without-context': [1], diff --git a/docs/internals/linters.mdx b/docs/internals/linters.mdx index 06506bf6f..645a4deff 100644 --- a/docs/internals/linters.mdx +++ b/docs/internals/linters.mdx @@ -192,3 +192,43 @@ const portforwardingClient = getStaticPath( ), ); ``` + +### `node/no-sync` + +- **Summary**: Use asynchronous methods whereever possible. [More details.](https://github.com/mysticatea/eslint-plugin-node/blob/master/docs/rules/no-sync.md) +- **Why**: Synchronous method calls block the event loop. Even innocous calls like `fs.existsSync()` can cause + frame drops for users or even long stalls. +- **How to fix it**: We have `fs-extra` as a dependency, which provides Promise-based alternatives for all `fs` functions. + Most often, replacing a sync call with an async call and adding an `await` is all that's needed. + +*Before* +```js +import fs from 'fs'; +function ensureCertsExist() { + if ( + !( + fs.existsSync(serverKey) && + fs.existsSync(serverCert) && + fs.existsSync(caCert) + ) + ) { + return generateServerCertificate(); + } +} +``` + +*After* + +```js +import fsExtra from 'fs-extra'; +async function ensureCertsExist() { + const allExist = Promise.all([ + fsExtra.exists(serverKey), + fsExtra.exists(serverCert), + fsExtra.exists(caCert), + ]).then((exist) => exist.every(Boolean)); + if (!allExist) { + return this.generateServerCertificate(); + } +} +```