worker icon indicating copy to clipboard operation
worker copied to clipboard

When specifying `CronItemOptions`, `backfillPeriod` is not optional in TypeScript

Open danieldiekmeier opened this issue 1 year ago • 1 comments

Summary

I'm defining my cronList as an array of CronItems, so that I can later pass them through parseCronItems. This has worked great! (Thank you very much for the library!)

Today, I wanted to add maxAttempts: 1 to one of my CronItems, and created an options object for that. TypeScript started complaining that the options must have a backfillPeriod key.

Steps to reproduce

Use this code:

import { type CronItem } from 'graphile-worker'

const cronList: CronItem[] = [
  {
    task: 'example',
    match: '* * * * *,
    identifier: 'example',
    options: {
      maxAttempts: 1,
      // TypeScript: Property 'backfillPeriod' is missing in type '{ maxAttempts: number; }' 
      //             but required in type 'CronItemOptions'. ts(2741)
    },
  },
}

Expected results

I'd expect the key to be optional, like the other options in CronItemOptions.

Actual results

At runtime, this seems to work correctly and I don't see any issues. The problem is only the type error.

Additional context

  • Node.js 20.17.0
  • TypeScript v5.6.2

Possible Solution

I think the problem is that Graphile Worker uses the same interface internally and externally. If I understand correctly, the backfillPeriod is defaulted to 0 here:

https://github.com/graphile/worker/blob/436e29968d28c48d5f46a012b9c83b0747e2eff1/src/crontab.ts#L151-L153

So it makes sense that internally, backfillPeriod is always defined. But since this runtime default exists, I think there should be a slightly different interface to the outside.

danieldiekmeier avatar Oct 06 '24 11:10 danieldiekmeier

Having a different interface for internal types makes sense (leaving the exposed interfaces with the same names). I think I already knew about this but it seemed so minor I didn't bother resolving it. Feel free to send through a PR :+1:

benjie avatar Oct 07 '24 16:10 benjie