fix: set runOnce when actual date is given to setTime

[sheerlox]

Description

initially, set the internal variable runOnce correctly when passing an actual date to setTime. previously this was only done in the constructor.

since it is only used in one place and as to limit this kind of error, I then removed the runOnce property completely (we now check directly if this.cronTime is a real date). we do not consider this a breaking change since it was not referenced in the API documentation).

Related Issue

Fixes #739.

Motivation and Context

Fixes an issue when after creating a job with a cron expression and using setTime with an actual date, the job attempts to run multiple times.

How Has This Been Tested?

added test case provided by #739's OP.

Screenshots (if appropriate):

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.
• If my change introduces a breaking change, I have added a ! after the type/scope in the title (see the Conventional Commits standard).

[lpgera]

Thank you very much @sheerlox for the fix! I tested my use case with your fork and it perfectly resolves the issue.

[ncb000gt]

🎉 This PR is included in version 3.1.4 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[mirogajdos]

Hello. Using Cron together with with @nestjs/schedule and now my application won't build. Removal of property in PATCH release will ruin typechecks and builds for many people. It's definitely breaking change even if you didn't mention it in the docs as it was part of the type definitions. Now Job class is not backward compatible with previous versions. 😿

[sheerlox]

thanks for the heads up @extrememito, and sorry about the issue we caused in your build. not releasing these kinds of changes as breaking changes was probably okay for JS, but you're right that TS types actually are documentation on their own, and we should not break any public interfaces in patch or minor releases. we'll be more cautious about this in the future.

#751 is going to fix the issue once it's merged and released in 3.1.5.

as a side note, I'm curious what your setup looks like, because @nestjs/schedule uses pinned dependencies and didn't upgrade to 3.1.4 yet. would you mind expanding a bit on this?

[mirogajdos]

Thank you!! Sure. I'm running NX monorepo where some apps are using node-cron by itself and others via @nestjs/schedule. I'm using cron as described in this exmaple which requires importing CronJob class from node-cron and adding an instance into the registry. Therefore I have node-cron explicitly installed. I have also renovate bot setup to update Patch versions which bumped node-cron to 3.1.4 which resulted in:

TS2345: Argument of type 'import("/apps/node_modules/cron/dist/job").CronJob<null, null>' is not assignable to parameter of type 'import("/apps/node_modules/@nestjs/schedule/node_modules/cron/dist/job").CronJob<null, null>'.
  Property 'runOnce' is missing in type 'import("/apps/node_modules/cron/dist/job").CronJob<null, null>' but required in type 'import("/apps/node_modules/@nestjs/schedule/node_modules/cron/dist/job").CronJob<null, null>'.
    191 |       }
    192 |     })
  > 193 |     this.schedulerRegistry.addCronJob(expenseBlueprint.id, job)

[sheerlox]

@extrememito 3.1.5 is out, effectively restoring the runOnce property on CronJob! sorry again for the trouble.