-
-
Notifications
You must be signed in to change notification settings - Fork 513
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
Add isFinite, first & last to rrule & rruleset #422
base: master
Are you sure you want to change the base?
Conversation
This would be a lovely PR to add if it is still valid. |
Was just looking to add this functionality to my project, specifically checking for finiteness and finding the last date. |
return this.before(this.options.until, true) | ||
} | ||
|
||
const allDates = this.all() |
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.
Suggestion to use this.before() instead of this.all() here with a max date (new Date(9999, 1, 1), or some other date that is large enough to be agreed upon) so that the values from .all() are not all stored into memory while computing the result.
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.
Date has a max value which seems to be new Date(8640000000000000);
, anything after that returns "Invalid Date". We could use that if it's not more expensive (which I think it isn't)
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.
Yea, that or a similar value works.
Technically the Iter loop won't works/exits properly with that as the MaxDate in IterResult, since technically tooLate will never be set to true, as new Date(8640000000000001) > new Date(8640000000000000)
is false.
Would just round the numbers and go with new Date(8000000000000000);
to avoid that issue. Kind of doubt people are using dates that are in the year 255000+.
That said, the Javascript VM is probably going to long before it gets there if for whatever reason the iteration is that large. Would leave it up to code outside of the library to catch malicious/bad input that would let a massive amount of iterations to occur.
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.
Ah yes you're right. But I think it could be a good occasion to fix the exit condition to take NaN dates into account! :)
This implements feature request #215
isFinite
returns true if the rrule/rruleset has a determinable final occurrencelast
returns the final occurrence if it exists and is determinablefirst
returns the first occurrence if it existsThis PR also implements the following revertible changes:
count
on an infinite rrule/rruleset now returnsNumber.POSITIVE_INFINITY
instead of returning all possible dates until the year 9999 - could be considered breaking.all
on an infinite rrule/rruleset now throws instead of returning all dates until the year 9999 (unless an iterator has been provided as the iterator can stop the loop) - could be considered breaking.options.count
throws if it received something other than a safe integer,null
orNumber.POSITIVE_INFINITY
.Number.POSITIVE_INFINITY
normalizes tonull
.Thanks for contributing to
rrule
!To submit a pull request, please verify that you have done the following:
master
commitaddressing