From 31cb269e763fb8268b7238f6833a828d60df4deb Mon Sep 17 00:00:00 2001 From: Greg Hurrell Date: Fri, 25 Aug 2017 21:24:33 -0700 Subject: [PATCH 1/2] Add linter error preventing use of `async` outside the test suite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We want to keep the runtime dependency footprint small, so that means avoiding use of `async` at runtime (which creates a dependency on the Regenerator runtime). There is no built-in eslint rule for this, so we make a custom one. Test plan: Add an `async` function to a file, run `yarn run lint` and see: graphql-js/src/subscription/subscribe.js 288:1 error async functions are not allowed outside of the test suite no-async ✖ 1 problem (1 error, 0 warnings) Note that no errors are issued for the `async` functions in the test suite. Closes: https://github.com/graphql/graphql-js/issues/1008 --- .eslintrc | 4 +++- package.json | 2 +- resources/lint/no-async.js | 25 +++++++++++++++++++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 resources/lint/no-async.js diff --git a/.eslintrc b/.eslintrc index 73520c1f2b..9d0b76ea4a 100644 --- a/.eslintrc +++ b/.eslintrc @@ -219,6 +219,8 @@ "vars-on-top": 0, "wrap-iife": 2, "wrap-regex": 0, - "yoda": [2, "never", {"exceptRange": true}] + "yoda": [2, "never", {"exceptRange": true}], + + "no-async": 2 } } diff --git a/package.json b/package.json index 9e42ecb533..b813ae7d2b 100644 --- a/package.json +++ b/package.json @@ -24,7 +24,7 @@ "test": "npm run lint && npm run check && npm run testonly", "testonly": "babel-node ./node_modules/.bin/_mocha $npm_package_options_mocha", "t": "babel-node ./node_modules/.bin/_mocha --require ./resources/mocha-bootload", - "lint": "eslint src || (printf '\\033[33mTry: \\033[7m npm run lint -- --fix \\033[0m\\n' && exit 1)", + "lint": "eslint --rulesdir ./resources/lint src || (printf '\\033[33mTry: \\033[7m npm run lint -- --fix \\033[0m\\n' && exit 1)", "check": "flow check", "check-cover": "for file in {src/*.js,src/**/*.js}; do echo $file; flow coverage $file; done", "build": "babel src --optional runtime --ignore __tests__ --out-dir dist/ && cp package.json dist/ && npm run build-dot-flow", diff --git a/resources/lint/no-async.js b/resources/lint/no-async.js new file mode 100644 index 0000000000..a6243cc682 --- /dev/null +++ b/resources/lint/no-async.js @@ -0,0 +1,25 @@ +/** + * Copyright (c) 2017, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +module.exports = function(context) { + if (context.getFilename().match(/\b__tests__\b/)) { + return {}; + } else { + return { + FunctionDeclaration: function(node) { + if (node.async) { + context.report( + node, + 'async functions are not allowed outside of the test suite' + ); + } + }, + }; + } +}; From 84e0ad88d0885ff442fa0991bc0586684cda4af4 Mon Sep 17 00:00:00 2001 From: Greg Hurrell Date: Fri, 25 Aug 2017 22:34:40 -0700 Subject: [PATCH 2/2] Provide more context in lint error message --- resources/lint/no-async.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/resources/lint/no-async.js b/resources/lint/no-async.js index a6243cc682..c6bdd180b6 100644 --- a/resources/lint/no-async.js +++ b/resources/lint/no-async.js @@ -16,7 +16,10 @@ module.exports = function(context) { if (node.async) { context.report( node, - 'async functions are not allowed outside of the test suite' + 'async functions are not allowed outside of the test suite ' + + 'because older versions of NodeJS do not support them ' + + 'without additional runtime dependencies. Instead, use explicit ' + + 'Promises.' ); } },