rbrahul / deno_cron

A cron Job scheduler for Deno that allows you to write human readable cron syntax with tons of flexibility
MIT License
100 stars 2 forks source link

Timezone #1

Closed albertosantini closed 4 years ago

albertosantini commented 4 years ago

Thanks for your module.

What about adding timezone? Inspired by https://github.com/kelektiv/node-cron.

Timezone is important because you can execute jobs on machines with a timezone different with respect to the job.

RFC for cron API: cron(schedule, job, timezone):

// Run Job at 9a.m. NY time 
cron('0 0 9 * * *', () => {
    checkStock();
}, "America/New_York"});
export const cron = (schedule: string = '', job: JobType, timezone: string) => {
// I suppose we need to change `schedules` Map, adding timezone info
...

Then in executeJobs the validation is done for the following date:

 schedules.forEach((jobs, schedule, timezone) => {
   const date = new Date().toLocaleString("en-US", {timeZone: timezone});
...

What do you think?

I noticed toLocaleString doesn't work. See https://github.com/denoland/deno/issues/1636 Compare the following snippet in deno and in node:

var asiaTime = new Date().toLocaleString("en-US", {timeZone: "Asia/Shanghai"});
console.log('Asia time: '+ (new Date(asiaTime)).toISOString());
rbrahul commented 4 years ago

@albertosantini Thanks for creating this Issue, I will expose API to pass time zone. As you also referred Deno initially doesn't support TimeZone to toLocaleString but I have noticed the created a bug ticket and they are working on it in the V8 level. So I am assuming the issue will be fixed soon. Thanks

rbrahul commented 4 years ago

I have created a separate branch where I implemented TimeZone. Since Deno does not has support yet for the toLocalString() this feature not working as expected in Deno but works well with Nodejs. But Deno is working on it. Hopefully, it will be fixed soon in their upcoming update.

albertosantini commented 3 years ago

Hey. I hope you are fine.

Since Deno 1.8.0 ICU support has landed and now the test snippet above works correctly. See https://github.com/denoland/deno/issues/1968

Great!