breejs / later

*Maintained fork of Later.* A javascript library for defining recurring schedules and calculating future (or past) occurrences for them. Includes support for using English phrases and Cron schedules. Works in Node and in the browser.
https://breejs.github.io/later/
MIT License
134 stars 13 forks source link

[fix] Order is significant in composite/complex schedules where only some entries specify a year #22

Open shaunhurley opened 2 years ago

shaunhurley commented 2 years ago

Describe the bug

Not sure if this is truly a bug, or an expected behavior. In a composite schedule where some entries specify a year, and others do not, using the .next() call with the schedule throws an error depending on the order that the crontabs are presented / processed. If schedules that include a year are presented last, then the result is as expected. If schedules that include a year are presented before any open/generic schedules it throws an error.

Actual behavior

\test-later\node_modules\@breejs\later\lib\index.js:926
      return !b || a.getTime() > b.getTime();
                     ^

TypeError: a.getTime is not a function
    at \test-later\node_modules\←[4m@breejs←[24m\later\lib\index.js:926:22
    at findNext (\test-later\node_modules\←[4m@breejs←[24m\later\lib\index.js:936:26)
    at getInstances (\test-later\node_modules\←[4m@breejs←[24m\later\lib\index.js:745:50)
    at Object.next (\test-later\node_modules\←[4m@breejs←[24m\later\lib\index.js:949:14)
    at Object.<anonymous> (\test-later\bad.js:25:52)
←[90m    at Module._compile (node:internal/modules/cjs/loader:1101:14)←[39m
←[90m    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)←[39m
←[90m    at Module.load (node:internal/modules/cjs/loader:981:32)←[39m
←[90m    at Function.Module._load (node:internal/modules/cjs/loader:822:12)←[39m
←[90m    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)←[39m

Expected behavior

[
  2023-01-01T23:30:00.000Z,
  2024-01-01T23:30:00.000Z,
  2025-01-01T23:30:00.000Z,
  2026-01-01T23:30:00.000Z,
  2027-01-01T23:30:00.000Z
]

Code to reproduce

const later = require('@breejs/later');
later.date.localTime();

let good_composite_schedule = {
  "schedules": [
    { "s": [ 0 ], "m": [ 30 ], "h": [ 18 ], "D": [ 1 ], "M": [ 1 ] },
    { "s": [ 0 ], "m": [ 30 ], "h": [ 17 ], "D": [ 2 ], "M": [ 2 ],"Y": [ 2021 ] }
  ],
  "exceptions": []
};
console.log(later.schedule(good_composite_schedule).next(5));

let bad_composite_schedule = {
  "schedules": [
    { "s": [ 0 ], "m": [ 30 ], "h": [ 17 ], "D": [ 2 ], "M": [ 2 ],"Y": [ 2021 ] },
    { "s": [ 0 ], "m": [ 30 ], "h": [ 18 ], "D": [ 1 ], "M": [ 1 ] }
  ],
  "exceptions": []
};
console.log(later.schedule(bad_composite_schedule).next(5));

Checklist

titanism commented 9 months ago

I had to comment this out since it was breaking of course, but I at least merged it. If you find the solution and submit a PR please uncomment the tests. Thank you