New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Minor: avoiding purely sync async functions #133

Open

jasnell opened this issue Apr 7, 2021 · 0 comments

Contributor

jasnell commented Apr 7, 2021

As a general best practice, to avoid unnecessary performance cost, purely sync async functions should be avoided.

For instance, from https://github.com/covidgreen/covid-green-backend-api/blob/current/lib/server.js:

server.addHook('onSend', async (req, res) => {
    res.header('Cache-Control', 'no-store')
    res.header('Pragma', 'no-cache')
    res.header(
      'Strict-Transport-Security',
      `max-age=${config.security.hstsMaxAge}; includeSubDomains`
    )
  })

The hook function is not doing anything asynchronous and therefore should not be an async function:

server.addHook('onSend', (req, res, done) => {
    res.header('Cache-Control', 'no-store')
    res.header('Pragma', 'no-cache')
    res.header(
      'Strict-Transport-Security',
      `max-age=${config.security.hstsMaxAge}; includeSubDomains`
    )
    done()
  })

The anti-pattern of using async functions with sync code is seen several places throughout the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment