Skip to content

Plugin method types shouldn't force users to return a promise #8146

@stretchkennedy

Description

@stretchkennedy

Back in v3, most plugin methods (requestDidStart etc.) were unified to always return a promise. This means that instead of

class LoggingPlugin implements ApolloServerPlugin<MyContext> {
  requestDidStart() {
    const start = hrtime.bigint()
    return {
      willSendResponse () {
        console.log('completed in', (hrtime.bigint() - start) / BigInt(1000000), 'ms')
      }
    }
  }
}

users have to do

class LoggingPlugin implements ApolloServerPlugin<MyContext> {
  async requestDidStart() {
    const start = hrtime.bigint()
    return {
      async willSendResponse () {
        console.log('completed in', (hrtime.bigint() - start) / BigInt(1000000), 'ms')
      }
    }
  }
}

This has the following disadvantages:

  1. In a larger example, using async for code that will never await anything can be confusing. "Why does this code need to be async, is there a bug, has it changed?"
  2. It probably has a performance impact.
  3. You can write plugins without async in JavaScript and they'll work fine, which makes porting them to TypeScript confusing.
  4. If JavaScript plugins work fine with non-promise returns but the TS types don't reflect this, there's a risk of breaking existing plugins if future behaviour comes to rely on the return type being a promise.

I promise I actually wrote this feature request and it's not some LLM slop. I just finished porting some of our plugins to TS at work, and I found this API confusing. It's not a huge deal but it would make the ergonomics of the library nicer.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions