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

Add support for dtend and floating dates #1

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
Draft

Conversation

simonratner
Copy link

PR for local review only; will issue PR to origin repo after.

Copy link

@RogWilco RogWilco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM once the duration/DST issue is sorted.

src/parsestring.ts Outdated Show resolved Hide resolved
test/rrule.test.ts Show resolved Hide resolved
@simonratner
Copy link
Author

once the duration/DST issue is sorted

Actually i belive it is correct; added a test to prove it. Based on this, rrule treats all js dates as zero-offset dates (it ignores the timezone offset and expects all input to be zoneless-UTC). Output is also zoneless-UTC that should be assumed ot be in the local timezone, and must be converted using luxon or similar if you want the output in a particular timezone (or actual UTC).

The end effect of all this is that DST has no effect. The duration of the event is calculated on UTC dates which have no timezone and hence no DST, and they do not end up affecting the output either because the output is returned as local time.

@simonratner
Copy link
Author

Btw, reading through jkbrzt#322 and the issue linked from there, there seems to be little chance of getting this merged upstream. DTEND is considered "out of scope" for RRULE.

@simonratner simonratner changed the title Add support for dtend and duration Add support for dtend and floating dates Oct 20, 2019
@simonratner
Copy link
Author

simonratner commented Oct 20, 2019

🛑 This PR should not be merged, see instead add-dtend-xxxx pre-built branches based off this.

@andjiev
Copy link

andjiev commented Sep 25, 2020

@simonratner what happened with this PR? As I can see from the rrule opened issue, it says feel free to send a pull request for this to get it included.

@simonratner
Copy link
Author

This code is in active use. I'll send a PR.

@simonratner
Copy link
Author

See jkbrzt#421.

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

Successfully merging this pull request may close these issues.

3 participants