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

Incomplete reset() when prev() was called before makes next() skip a day #338

Open
njoyard opened this issue Nov 21, 2023 · 3 comments
Open

Comments

@njoyard
Copy link

njoyard commented Nov 21, 2023

I noticed that next() skips a day when prev() was called before, even if reset() has been called.

Here is a test case, tested with [email protected]. Local TZ is Europe/Paris (so +1 right now). Not sure if TZ is involved in the issue.

const cronParser = require('cron-parser')
const expr = cronParser.parseExpression('0 0 22 * * *')
const now = new Date() // 2023-11-21T21:04:39Z
expr.reset(now)
expr.next().toDate() // 2023-11-21T21:00:00Z - correct (that's T22:00:00+01:00 today)
expr.reset(now)
expr.prev().toDate() // 2023-11-20T21:00:00Z - correct (yesterday)
expr.reset(now)
expr.next().toDate() // 2023-11-22T21:00:00Z - incorrect (tomorrow)

My understanding is that the last return value should be the same as the first. My current workaround is to re-parse the expression every time.

@njoyard njoyard changed the title Incomplete reset() when last() was called before makes next() skip a day Incomplete reset() when prev() was called before makes next() skip a day Nov 21, 2023
@harrisiirak
Copy link
Owner

Hi @njoyard!

If I removed reset() calls (to be precise, the one after prev() call was messing things up), it seems to be working:

const parser = require('./');

// Initial `2023-11-21T21:04:39Z` will not be matched, so I had to move the date one hour before
const currentDate = new Date('2023-11-21T20:04:39Z');
const options = {
  currentDate,
  // You shouldn't need to set this; I just needed that to test out the timezone
  tz: 'Europe/Paris' 
};

const expr = parser.parseExpression('0 0 22 * * *', options);

console.log(expr.next().toDate()); // 2023-11-21T21:00:00.000Z
console.log(expr.prev().toDate()); // 2023-11-20T21:00:00.000Z
console.log(expr.next().toDate()); // 2023-11-21T21:00:00.000Z

The culprit seems to be reset() call itself that won't work properly after prev() call.

@markNZed
Copy link

markNZed commented Nov 25, 2023

That might be because reset is not passing a timezone into CronDate at

this._currentDate = new CronDate(newDate || this._options.currentDate);
Maybe test if it works without the timezone.

@harrisiirak
Copy link
Owner

That might be because reset is not passing a timezone into CronDate at

When I tested it, I also also briefly tested with passing the tz in reset method - didn't see any difference. Definitely requires a bit more digging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants