kelektiv / node-cron

Cron for NodeJS.
MIT License
8.3k stars 617 forks source link

type issue in CronJob #869

Closed pratikkajare closed 2 months ago

pratikkajare commented 3 months ago

Description

There is an issue with this line of code: // here is package issue with Argument of type 'CronJob<any, any> from 'cron'. This is causing an error and needs to be fixed. This can be resolved by modifying the code to ensure the correct argument type is passed.
-- this.schedulerRegistry.addCronJob(name, job);

image

Expected Behavior

.

Actual Behavior

.

Possible Fix

image

By changing the return type of the nextDates() function to DateTime | DateTime[], we can enhance the flexibility and efficiency of the code. Therefore, I recommend making this change. Thank you. like... public nextDates(): DateTime | DateTime[];

Steps to Reproduce

just call this function using ts.

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

this.schedulerRegistry.addCronJob(name, job); // here will get the type error job.start();

this.logger.warn( job ${name} added for each minute at ${seconds} seconds!, ); }

Context

giving type error in typescript code

Your Environment

sheerlox commented 2 months ago

Hey @pratikkajare, sorry for the delay. This error is usually because of a mismatch between the cron version you're using and the one your @nestjs/schedule version depends on. Could you please check if these are effectively the same?

Given the error message, I'd say you're using v3 while using an older @nestjs/schedule version which still depends on v2.

sheerlox commented 2 months ago

Regarding the typing of nextDates in v2.4.3, even though it was unclear it nonetheless was correct: nextDates returned a DateTime when called without arguments, and an array when called with a number.

This behavior was changed in v3.0.0 by PR https://github.com/kelektiv/node-cron/pull/519.

freben commented 2 months ago

We had to work around this too.

https://github.com/backstage/backstage/pull/24061/files

The reason is that you're depending on "@types/luxon": "~3.3.0" and we have a ^ dependency. The DateTime that you return is missing some features that were added in the 3.4 range of luxon.

You may want to release a version that has an updated type dependency range.

sheerlox commented 2 months ago

We had to work around this too.

https://github.com/backstage/backstage/pull/24061/files

The reason is that you're depending on "@types/luxon": "~3.3.0" and we have a ^ dependency. The DateTime that you return is missing some features that were added in the 3.4 range of luxon.

You may want to release a version that has an updated type dependency range.

Thanks for the heads up, will look into that tonight!

sheerlox commented 2 months ago

@freben just merged https://github.com/kelektiv/node-cron/pull/831 and released v3.1.7, that should solve your issue :smiley:

@pratikkajare I still think your issue is related to a version mismatch, so probably stick to my first comment in the first time :wink:

Let me know if there's anything more I can do!

freben commented 2 months ago

Nice, thanks!

pratikkajare commented 2 months ago

thank you so much