fix(pattern): do not save offset when immediately is provided #2756
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
roggervalf
commented
Sep 4, 2024
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.
when immediately is provided, previous time should be provided as prevMillis
There was a problem hiding this comment.
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
roggervalf
commented
Sep 4, 2024
| @@ -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.
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
roggervalf
force-pushed
the
fix-pattern-remove-offset
branch
from
b757f76 to
78553c1
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #2754