nestjs / bull

Bull module for Nest framework (node.js) :cow:
https://nestjs.com
MIT License
613 stars 103 forks source link

Better way to get `QueueEventsListener` method types #2181

Open Mnigos opened 3 months ago

Mnigos commented 3 months ago

Is there an existing issue that is already proposing this?

Is your feature request related to a problem? Please describe it

I have created a QueueEvents listener and method parameter types are not inferred correctly which causes passing wrong parameters to the listener definition.

I found a way to get this listener typed, but the solution is not the best

import {
  OnQueueEvent,
  QueueEventsHost,
  QueueEventsListener,
} from '@nestjs/bullmq'
import { QueueEventsListener as IQueueEventsListener } from 'bullmq'
import { Injectable, Logger } from '@nestjs/common'
import type { Queue } from 'bullmq'

import { HISTORY_QUEUE } from './constants'
import { InjectHistoryQueue } from './decorators'

import type { User } from '@modules/users'

type QueueEventsListeners = Partial<{
  [K in keyof IQueueEventsListener as `on${Capitalize<K>}`]: IQueueEventsListener[K]
}>

type QueueEventListenerParams<TListener extends keyof IQueueEventsListener> =
  Parameters<IQueueEventsListener[TListener]>[0]

@QueueEventsListener(HISTORY_QUEUE)
@Injectable()
export class HistoryQueueEvents
  extends QueueEventsHost
  implements QueueEventsListeners
{
  private readonly logger = new Logger(HistoryQueueEvents.name)

  constructor(
    @InjectHistoryQueue() private readonly historyQueue: Queue<User>
  ) {
    super()
  }

  @OnQueueEvent('error')
  onError(error: Error) {
    this.logger.error(error)
  }

  @OnQueueEvent('failed')
  async onFailed({ jobId, failedReason }: QueueEventListenerParams<'failed'>) {
    const job = await this.historyQueue.getJob(jobId)

    if (!job) return

    const {
      data: { profile },
    } = job

    this.logger.error(
      `History synchronization failed for user: ${profile.displayName}`
    )
    this.logger.error(failedReason)
  }

  @OnQueueEvent('active')
  async onActive({ jobId }: QueueEventListenerParams<'active'>) {
    const job = await this.historyQueue.getJob(jobId)

    if (!job) return

    const {
      data: { profile },
    } = job

    this.logger.log(
      `Processing history synchronization for user: ${profile.displayName}...`
    )
  }

  @OnQueueEvent('completed')
  async onCompleted({ jobId }: QueueEventListenerParams<'completed'>) {
    const job = await this.historyQueue.getJob(jobId)

    if (!job) return

    const {
      data: { profile },
    } = job

    this.logger.log(
      `History synchronization completed for user: ${profile.displayName}`
    )
  }

  @OnQueueEvent('stalled')
  async onStalled({ jobId }: QueueEventListenerParams<'stalled'>) {
    const job = await this.historyQueue.getJob(jobId)

    if (!job) return

    const {
      data: { profile },
    } = job

    this.logger.error(
      `History synchronization stalled for user: ${profile.displayName}`
    )
  }
}

Describe the solution you'd like

Maybe QueueEventsHost class should have this methods defined so inferring types won't be so hard.

Teachability, documentation, adoption, migration strategy

No response

What is the motivation / use case for changing the behavior?

Better type safety.

doneumark commented 3 months ago

+1

kamilmysliwiec commented 1 day ago

Would you be interested in creating a PR to add the QueueEventListenerParams utility type to the package? We could also include a usage example in the documentation to highlight the easiest approach (using that type) for other developers.