adonisjs / transmit

A Server-Sent-Event module for AdonisJS
MIT License
65 stars 4 forks source link

`authorizeChannel` must return a promise #13

Closed Barbapapazes closed 5 months ago

Barbapapazes commented 6 months ago

Package version

0.2.3

Describe the bug

In the documentation, https://github.com/adonisjs/transmit#channel-authorization, the example return a boolean. This throw a TS error because the returned type of this function is Promise<boolean>. You need to add async in the example or to allow the function to return boolean.

https://github.com/adonisjs/transmit/blob/main/src/transmit.ts#L142

Reproduction repo

No response

Barbapapazes commented 6 months ago

Maybe a more realistic example like this one could be useful:

transmit.authorizeChannel<{ questionId: string }>(
  ':questionId/events',
  async ({ auth }: HttpContext, { questionId }) => {
    const user = await auth.authenticateUsing(['api'])

    const question = await user
      .related('questions')
      .query()
      .where('questions.id', questionId)
      .firstOrFail()

    return question.id === questionId
  }
)
RomainLanz commented 5 months ago

You need to add async in the example or to allow the function to return boolean.

We should change the return type to have Promise<boolean> | boolean. Can you create a PR for it?

Maybe a more realistic example like this one could be useful:

Yes, we can probably improve the example, it will be done in the real documentation (website),.

Barbapapazes commented 5 months ago

I'll make a PR later today!

If you plan to move documentation into the Adonis docs, I will not touch the readme and wait for the release!