-
Notifications
You must be signed in to change notification settings - Fork 410
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
fix(pattern): do not save offset when immediately is provided #2756
Conversation
src/classes/repeat.ts
Outdated
@@ -340,7 +340,11 @@ export const getNextMillis = ( | |||
}); | |||
|
|||
try { | |||
return interval.next().getTime(); | |||
if(opts.immediately){ | |||
return interval.prev().getTime(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when immediately is provided, previous time should be provided as prevMillis
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when next getTime is returned, next delayed job won't be schedule in the first cron iteration but in the second one, so it add extra delay. In order to prevent that, this value should be less that the first millis value. Looks like returning the current time is sufficient and makes more sense as we are using immediately option, changed in last commit
@@ -65,7 +65,7 @@ export class Repeat extends QueueBase { | |||
const hasImmediately = Boolean( | |||
(every || pattern) && repeatOpts.immediately, | |||
); | |||
const offset = hasImmediately ? now - nextMillis : undefined; | |||
const offset = (hasImmediately && every) ? now - nextMillis : undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only needed when every is provided, with pattern we don't need to subtract this value as it should execute next delayed job in an specific time
b757f76
to
78553c1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
fixes #2754