node-cron
node-cron copied to clipboard
State of the library & moving forward
Roadmap
Based on the bellow observations (see Context), here's what I think would be a good way going forward:
V2 features and bugfixes
- [X] https://github.com/kelektiv/node-cron/pull/669
- [X] https://github.com/kelektiv/node-cron/pull/686
- [X] https://github.com/kelektiv/node-cron/pull/687
- [X] https://github.com/kelektiv/node-cron/pull/689
- [x] https://github.com/kelektiv/node-cron/pull/685
V3 release (breaking changes)
- [X] https://github.com/kelektiv/node-cron/pull/667 (available on
betachannel) - [x] https://github.com/kelektiv/node-cron/pull/519 (available on
betachannel) - [x] https://github.com/kelektiv/node-cron/pull/694
- [x] https://github.com/kelektiv/node-cron/pull/699
- [x] support both ESM & CJS (NodeJS/Deno/Bun) (see https://github.com/kelektiv/node-cron/issues/700)
post V3 release (ordered by priority)
- [X] https://github.com/kelektiv/node-cron/issues/467
- [X] https://github.com/kelektiv/node-cron/issues/654
- [x] https://github.com/kelektiv/node-cron/issues/441
- [ ] https://github.com/kelektiv/node-cron/issues/426
- [ ] https://github.com/kelektiv/node-cron/issues/556
- [ ] https://github.com/kelektiv/node-cron/issues/713
- [ ] https://github.com/kelektiv/node-cron/issues/435
- [ ] https://github.com/kelektiv/node-cron/issues/315
- [x] https://github.com/kelektiv/node-cron/issues/489
- [x] https://github.com/kelektiv/node-cron/issues/423
- [ ] https://github.com/kelektiv/node-cron/issues/671
- [ ] support using both day-of-month and day-of-week in a cron expression
- [ ] browser support (partial work done in https://github.com/kelektiv/node-cron/pull/629)
- [ ] performance analysis & improvements
Misc/Rehabilitation
- [X] automated release process & enforced conventional commits (https://github.com/kelektiv/node-cron/pull/673)
- [X] thorough inspection of every currently opened issue, push forward on fixing critical ones, and close outdated/irrelevant ones
- [X] take over relevant staled opened PR and push them through, and close outdated/irrelevant ones
- [x] add dependencies bot & unify dependencies version pinning (GitHub app needs to be installed, see https://github.com/kelektiv/node-cron/pull/683)
- [x] #696
Candidates for next major release
- [ ] swap
C&OCgeneric type parameters on every type (QoL) - [ ] add
runOnceas aCronJoboption - [ ] revamp all class properties (make some private & potentially add getters/setters)
Less urgent
- [ ] re-enable CodeCov
- [ ] automate checking if the mandatory "Related Issue" field in PR is filled
Context
Click to expand
Motivation
In my opinion, the OpenSource spirit is more about trying to maintain/improve existing projects (especially when they are "industry leaders") than trying to create a new project overseeding the existing ones.
On February 12, this issue was created to replace cron with croner (by the creator of the later) in the @nestjs/schedule package. Work is being done by the same individual in this PR to meet that goal.
In this @nestjs/nest issue (one year ago), OP mentioned this issue from our repository, that is closed but according to some comments, it might still be occurring (see issue https://github.com/kelektiv/node-cron/issues/467 for follow-up on this), although we cannot be sure users are using the latest version. We should implement test cases that somehow limit/overload cpi & memory resources to test for this kind of problem (this has been successfully implemented, see https://github.com/kelektiv/node-cron/issues/467#issuecomment-1580693046).
Here are the arguments to migrate to croner according to its creator (from the @nestjs/schedule PR):
supports both ESM and CJS.
- migrating to Typescript will enable supporting both ESM and CJS
is ready for the future, supporting Node, Deno, Bun and Browser environments.
- I never tried to use this library in the browser, not sure what are the use cases but I guess we can make sure it is portable. This should be taken care of during the Typescript migration as well.
- Regarding Deno & Bun, I have no experience with those but we would need to look further into that (looks like Deno only supports ESM and that it's the only blocking point. Bun should already be working according to their docs)
- Browser-support related PR: https://github.com/kelektiv/node-cron/pull/629
has more predictive behaviour and less bugs. (not affected by Scheduled jobs execute multiple times #454 as an example)
- this issue (https://github.com/nestjs/schedule/issues/454) has been resolved for two years (not explicitly but if it was still the case we would have heard about it), but there might be a good point regarding predictability and bugs.
uses the same pattern as vixie cron (see EVERY_YEAR Cronexpression is invalid #1159)
- we could add the support for
Lfor the last day and weekday of the month in cron strings? - there is an issue opened about this: https://github.com/kelektiv/node-cron/issues/435
has no dependencies (see Vulnerability found in dependency #1147)
- I personally don't think having
luxonas a dependency is an issue, it's even a feature since it allows us to have support for theDateTimeobject. That is debatable though, and I'm welcoming any objecting point of view so we can find the solution that best fits everyone's use cases. - We definitely need to add some dependencies scanning & automated version bumps PR tool (Renovate/Dependabot), as is standard in the Javascript OpenSource ecosystem.
As another useful reference, here's a link to the features comparison from croner.
Two features I think are particularly interesting:
Over-run protection: an option to prevent the next occurrence from running if the first one is still processing (long-running jobs)Error handling: this is something I've seen multiple times in our issues' comments, if something goes wrong it seems to go unnoticed most of the time, maybe we should think about a better way of detecting and reporting potential errors to the user (EDIT: error handling incroneris actually providing a way to handle errors that might occur in thetickfunction, see Hexagon/croner/EXAMPLES.md#error-handling)
This issue was initially discussed on Discord.
Given the state of #467 and the rate at which it keeps me entertained with report after report, I think this whole thing needs to be abandon. There are other options and they work.
p.s. TypeScript won't solve anything.
@mryellow if you want to move to another library, please feel free to do so :) If, on the other hand, you'd like to start being constructive and helpful, then useful comments and PRs are welcome.
Have you considered removing the calculations for "next scheduled" and using a library instead?
cron-parser will handle the while loop in a way which doesn't fail under the same conditions.
we're looking at a rewrite of that function yes because this has been an issue for a while. if you have any ideas for a specific approach we're open to it. I kind of would prefer not to use a library if possible because it's better if this utility stays as lightweight as it can although since we're already using Luxon maybe there's something there we can use
if you have any ideas
That's the thing, at a glance the structure of _getNextDateFrom looks fairly similar to approaches used elsewhere. Any re-write would probably just involve taking a very close look at those functions elsewhere and incorporation whatever differences there are. At which point you may as well just use a proven library rather than copy/pasting it's code.