fix: fix object constructor typings & make OC generic type optional (#712)

Author: sheerlox

README.md

@@ -118,7 +118,7 @@ Parameter Based
   - `constructor(cronTime, onTick, onComplete, start, timeZone, context, runOnInit, utcOffset, unrefTimeout)`
     - `cronTime` - [REQUIRED] - The time to fire off your job. This can be in the form of cron syntax or a JS [Date](https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Date) object.
     - `onTick` - [REQUIRED] - The function to fire at the specified time. If an `onComplete` callback was provided, `onTick` will receive it as an argument. `onTick` may call `onComplete` when it has finished its work.
-    - `onComplete` - [OPTIONAL] - A function that will fire when the job is stopped with `job.stop()`, and may also be called by `onTick` at the end of each run.
+    - `onComplete` - [OPTIONAL] - A function that will fire when the job is stopped with `job.stop()`, and may also be called by `onTick` at the end of each run. **Note for TS users**: This should either be an arrow function, or a regular function cast to `() => void` (bug with generic type inference).
     - `start` - [OPTIONAL] - Specifies whether to start the job just before exiting the constructor. By default this is set to false. If left at default you will need to call `job.start()` in order to start the job (assuming `job` is the variable you set the cronjob to). This does not immediately fire your `onTick` function, it just gives you more control over the behavior of your jobs.
     - `timeZone` - [OPTIONAL] - Specify the time zone for the execution. This will modify the actual time relative to your time zone. If the time zone is invalid, an error is thrown. By default (if this is omitted) the local time zone will be used. You can check the various time zones format accepted in the [Luxon documentation](https://github.com/moment/luxon/blob/master/docs/zones.md#specifying-a-zone). Note: This parameter supports minutes offsets, e.g. `UTC+5:30`. **Note**: Cannot be used together with `utcOffset`.
     - `context` - [OPTIONAL] - The context within which to execute the onTick method. This defaults to the cronjob itself allowing you to call `this.stop()`. However, if you change this you'll have access to the functions and values within your context object.

src/job.ts

@@ -11,15 +11,15 @@ import {
 	WithOnComplete
 } from './types/cron.types';
 
-export class CronJob<OC extends CronOnCompleteCommand<C> | null, C = null> {
+export class CronJob<OC extends CronOnCompleteCommand | null = null, C = null> {
 	cronTime: CronTime;
 	running = false;
 	unrefTimeout = false;
 	lastExecution: Date | null = null;
 	runOnce = false;
 	context: CronContext<C>;
 	onComplete?: WithOnComplete<OC> extends true
-		? CronOnCompleteCallback<C>
+		? CronOnCompleteCallback
 		: undefined;
 
 	private _timeout?: NodeJS.Timeout;
@@ -81,9 +81,7 @@ export class CronJob<OC extends CronOnCompleteCommand<C> | null, C = null> {
 			// casting to the correct type since we just made sure that WithOnComplete<OC> = true
 			this.onComplete = this._fnWrap(
 				onComplete
-			) as WithOnComplete<OC> extends true
-				? CronOnCompleteCallback<C>
-				: undefined;
+			) as WithOnComplete<OC> extends true ? CronOnCompleteCallback : undefined;
 		}
 
 		if (this.cronTime.realDate) {
@@ -100,7 +98,7 @@ export class CronJob<OC extends CronOnCompleteCommand<C> | null, C = null> {
 		if (start) this.start();
 	}
 
-	static from<C = null, OC extends CronOnCompleteCommand<C> | null = null>(
+	static from<OC extends CronOnCompleteCommand | null = null, C = null>(
 		params: CronJobParams<OC, C>
 	) {
 		// runtime check for JS users
@@ -196,7 +194,7 @@ export class CronJob<OC extends CronOnCompleteCommand<C> | null, C = null> {
 			callback.call(
 				this.context,
 				this.onComplete as WithOnComplete<OC> extends true
-					? CronOnCompleteCallback<C>
+					? CronOnCompleteCallback
 					: never
 			);
 		}

src/time.ts

@@ -37,19 +37,19 @@ export class CronTime {
 	private dayOfWeek: TimeUnitField<'dayOfWeek'> = {};
 
 	constructor(
-		source: CronJobParams<null>['cronTime'],
-		timeZone?: CronJobParams<null>['timeZone'],
+		source: CronJobParams['cronTime'],
+		timeZone?: CronJobParams['timeZone'],
 		utcOffset?: null
 	);
 	constructor(
-		source: CronJobParams<null>['cronTime'],
+		source: CronJobParams['cronTime'],
 		timeZone?: null,
-		utcOffset?: CronJobParams<null>['utcOffset']
+		utcOffset?: CronJobParams['utcOffset']
 	);
 	constructor(
-		source: CronJobParams<null>['cronTime'],
-		timeZone?: CronJobParams<null>['timeZone'],
-		utcOffset?: CronJobParams<null>['utcOffset']
+		source: CronJobParams['cronTime'],
+		timeZone?: CronJobParams['timeZone'],
+		utcOffset?: CronJobParams['utcOffset']
 	) {
 		// runtime check for JS users
 		if (timeZone != null && utcOffset != null) {

src/types/cron.types.ts

@@ -5,7 +5,7 @@ import { CronJob } from '../job';
 import { IntRange } from './utils';
 
 interface BaseCronJobParams<
-	OC extends CronOnCompleteCommand<C> | null,
+	OC extends CronOnCompleteCommand | null = null,
 	C = null
 > {
 	cronTime: string | Date | DateTime;
@@ -18,7 +18,7 @@ interface BaseCronJobParams<
 }
 
 export type CronJobParams<
-	OC extends CronOnCompleteCommand<C> | null,
+	OC extends CronOnCompleteCommand | null = null,
 	C = null
 > =
 	| BaseCronJobParams<OC, C> &
@@ -33,16 +33,14 @@ export type CronJobParams<
 				  }
 			);
 
-export type CronContext<C> = C extends null ? CronJob<null> : NonNullable<C>;
+export type CronContext<C> = C extends null ? CronJob : NonNullable<C>;
 
 export type CronCallback<C, WithOnCompleteBool extends boolean = false> = (
 	this: CronContext<C>,
-	onComplete: WithOnCompleteBool extends true
-		? OmitThisParameter<CronOnCompleteCallback<C>>
-		: never
+	onComplete: WithOnCompleteBool extends true ? CronOnCompleteCallback : never
 ) => void;
 
-export type CronOnCompleteCallback<C> = (this: CronContext<C>) => void;
+export type CronOnCompleteCallback = () => void;
 
 export type CronSystemCommand =
 	| string
@@ -56,9 +54,7 @@ export type CronCommand<C, WithOnCompleteBool extends boolean = false> =
 	| CronCallback<C, WithOnCompleteBool>
 	| CronSystemCommand;
 
-export type CronOnCompleteCommand<C> =
-	| OmitThisParameter<CronOnCompleteCallback<C>>
-	| CronSystemCommand;
+export type CronOnCompleteCommand = CronOnCompleteCallback | CronSystemCommand;
 
 export type WithOnComplete<OC> = OC extends null ? false : true;
 

tests/cron.test.ts

@@ -329,7 +329,7 @@ describe('cron', () => {
 						expect(t.getSeconds()).toBe(d.getSeconds());
 						onComplete();
 					},
-					function () {
+					() => {
 						callback();
 						resolve();
 					},
@@ -344,6 +344,35 @@ describe('cron', () => {
 			expect(callback).toHaveBeenCalledTimes(2);
 		});
 
+		it('should run on a specific date and call onComplete from onTick using the object constructor', async () => {
+			const d = new Date();
+			const clock = sinon.useFakeTimers(d.getTime());
+			d.setSeconds(d.getSeconds() + 1);
+			const callback = jest.fn();
+
+			await new Promise<void>(resolve => {
+				const job = CronJob.from({
+					cronTime: d,
+					onTick: onComplete => {
+						const t = new Date();
+						expect(t.getSeconds()).toBe(d.getSeconds());
+						onComplete();
+					},
+					onComplete: function () {
+						callback();
+						resolve();
+					} as () => void,
+					start: true
+				});
+				clock.tick(1000);
+				clock.restore();
+				job.stop();
+			});
+
+			// onComplete is called 2 times: once in onTick() & once when calling job.stop()
+			expect(callback).toHaveBeenCalledTimes(2);
+		});
+
 		it("should not be able to call onComplete from onTick if if wasn't provided", () => {
 			expect.assertions(4);
 			const d = new Date();