nestjs / schedule

Schedule module for Nest framework (node.js) ⏰
https://nestjs.com
MIT License
358 stars 80 forks source link

Dynamic cron job via `nestjs/schedule` pkg requires `cron` pkg resulting with duplicate `cron` pkgs #1810

Closed shanike closed 1 week ago

shanike commented 1 week ago

Is there an existing issue for this?

Current behavior

According to docs, a manual cron job is created like so:

const job = new CronJob(`${seconds} * * * * *`, () => {
  this.logger.warn(`time (${seconds}) for job ${name} to run!`);
});

this.schedulerRegistry.addCronJob(name, job);
job.start();

Notice the CronJob class is from the cron package (and not from the @nestjs/schedule package), as mentioned in the HINT in the docs.

Because cron is a dependency (and NOT a peer-dependency) of @nestjs/schedule, if our server also depends on cron we end up with 2 cron dependencies which is not ideal, and worse: they may be of different versions(!).

This results with the following TypeScript issue: image In text: Argument of type 'import("/home/path/to/project/node_modules/.pnpm/cron@3.1.3/node_modules/cron/dist/job").CronJob<null, null>' is not assignable to parameter of type 'import("/home/path/to/project/node_modules/.pnpm/cron@3.1.7/node_modules/cron/dist/job").CronJob<null, null>'. The types of 'cronTime.source' are incompatible between these types. Type 'string | import("/home/path/to/project/node_modules/.pnpm/@types+luxon@3.3.8/node_modules/@types/luxon/src/datetime").DateTime' is not assignable to type 'string | luxon.DateTime'. Type 'DateTime' is not assignable to type 'string | DateTime'. Type 'DateTime' is missing the following properties from type 'DateTime': getPossibleOffsets, isWeekend, localWeekday, localWeekNumber, and 2 more.ts(2345)

Minimum reproduction code

https://github.com/shanike/nestjs-schedule-bug-repro

Steps to reproduce

  1. pnpm start

Expected behavior

If nestjs/schedule requires me to install the cron package -> Then nestjs/schedule should have cron as a peer-dependency.

Otherwise, nestjs/schedule can export any objects/types of the cron package once might need when using nestjs/schedule and not require us to install the cron package ourselves.

Package version

4.0.0

NestJS version

10.2.10

Node.js version

20.10.0

In which operating systems have you tested?

Other

We use pnpm as our package manager.

micalevisk commented 1 week ago

as the version of cron dependency is pinned, you'll need to use the exact same version of cron in your side as well.

see that version at npm view @nestjs/schedule dependencies

shanike commented 1 week ago

Thanks for the quick reply.

I understand using the same version of cron will solve the issue, but I still think the better approach is for nestjs/schedule to set cron as peer-dependency, no? Or to export the CronJob class?

micalevisk commented 1 week ago

it can't be a peer dependency because this package is not a plugin for cron