Skip to content

Conversation

wincent
Copy link
Contributor

@wincent wincent commented Aug 26, 2017

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: #1008

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: #1008
"wrap-regex": 0,
"yoda": [2, "never", {"exceptRange": true}]
"yoda": [2, "never", {"exceptRange": true}],

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra line break intended here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I wanted to separate the custom rules. Would have put in a comment to make it clear but this is faux-JSON so there are no comments... (We do use a blank line as a separator higher up in the file, at least.)

if (node.async) {
context.report(
node,
'async functions are not allowed outside of the test suite'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest adding more detail here: "async functions are not allowed outside of the test suite because older versions of NodeJS do not support async functions without transpilation. Instead, use explicit Promises."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call.

@robzhu robzhu merged commit 4a378b9 into graphql:master Aug 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants