Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

State of the library & moving forward #674

Open
23 of 34 tasks
sheerlox opened this issue Jun 26, 2023 · 5 comments
Open
23 of 34 tasks

State of the library & moving forward #674

sheerlox opened this issue Jun 26, 2023 · 5 comments
Labels
help wanted Extra attention is needed

Comments

@sheerlox
Copy link
Collaborator

sheerlox commented Jun 26, 2023

Roadmap

Based on the bellow observations (see Context), here's what I think would be a good way going forward:

V2 features and bugfixes

V3 release (breaking changes)

post V3 release (ordered by priority)

Misc/Rehabilitation

Candidates for next major release

  • swap C & OC generic type parameters on every type (QoL)
  • add runOnce as a CronJob option
  • 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 #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 #467 (comment)).

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: feat: support browser environment #629
    

has more predictive behaviour and less bugs. (not affected by Scheduled jobs execute multiple times #454 as an example)

uses the same pattern as vixie cron (see EVERY_YEAR Cronexpression is invalid #1159)

has no dependencies (see Vulnerability found in dependency #1147)

  • I personally don't think having luxon as a dependency is an issue, it's even a feature since it allows us to have support for the DateTime object. 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 in croner is actually providing a way to handle errors that might occur in the tick function, see Hexagon/croner/EXAMPLES.md#error-handling)


This issue was initially discussed on Discord.

@mryellow
Copy link

mryellow commented Jul 12, 2023

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.

@sheerlox
Copy link
Collaborator Author

@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.

@mryellow

This comment was marked as resolved.

@intcreator

This comment was marked as outdated.

@mryellow

This comment was marked as outdated.

@sheerlox sheerlox added type:feature New feature or feature improvement & requests help wanted Extra attention is needed question labels Aug 15, 2023
@sheerlox sheerlox pinned this issue Aug 15, 2023
@sheerlox sheerlox self-assigned this Aug 23, 2023
@sheerlox sheerlox removed type:feature New feature or feature improvement & requests question labels Sep 30, 2023
@sheerlox sheerlox removed their assignment Oct 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants