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

Time Zone Support #97

Closed
wants to merge 23 commits into from
Closed

Time Zone Support #97

wants to merge 23 commits into from

Conversation

aramk
Copy link
Contributor

@aramk aramk commented May 3, 2015

This uses some of the methods in #78 and is an implementation of what was discussed in #38.

I've created a separate library at tz-date which contains a TimeZoneDate that extends Date, adheres to its API, has Mocha tests, and supports time zones through http://momentjs.com/timezone. This is in CoffeeScript so you'll need to run bower install in rrule and then grunt build in bower_components/tz-date to generate the JavaScript used by the tests I've added.

Since Date doesn't support having a time zone as the last argument and returns Invalid Date, I've
created a new method called createDate() to handle this situation. All date creation in the
library is now handled through this method. This also adds a "timezone" option which is passed to
all calls of createDate().

As for the testing, tests/index.html will still work as before and use Date. In tests/timezone.html
I add TimeZoneDate and its dependency libraries, and in timezone-tests I set TimeZoneDate as the
custom Date class to use. Here I've tests two time zones - my home city of Melbourne and
Los Angeles. TimeZoneDate has its own Mocha tests to ensure the getters and setters behave correctly.
For maximum coverage I've also ensured the existing tests are run with TimeZoneDate, so I'm fairly
convinced it's working well :)

One thing we might want to standardise is whether to use timeZone or timezone. I've been using
the "time zone" convention on the Wikipedia page, but "timezone" is fairly common elsewhere,
especially in libraries, and moment-timezone's docs use it interchangeably... Not sure which is
"correct", but standardising will reduce the confusion.

I hope this can be merged since it adds a vital feature which is missing from this library and which
likely impacts many people who are building apps and deploying them across the world. If not, this
branch is here for everyone to use.

aramk and others added 21 commits June 8, 2014 20:15
* master:
  Added check for invalid dates in init(). Added isValid and isDate methods to dateutil.
* mattcasey-master:
  add ability to use utility to supply timezoneoffset
  Fix typo
  Do not hang if interval is 0 or not a number

Conflicts:
	lib/rrule.js
Added compiled MomentDate for testing and created a separate html file for testing with existing test suite.
Added test for using MomentDate with RRule.
…etermining whether to use time zones or local dates.

Added method for getting duck-typed time zone from dates.
Added tz-date library for testing time zone support.
Removed old MomentDate.
Added tests for between and between in Melbourne.
Fixed timezone support in toString() and fromString().
Added timezone to IterResult.
Added back timezone tests and fixed issues causing infinite loops.
# By Linquize (2) and others
# Via Jakub Roztočil (4) and Jakub Roztocil (2)
* 'master' of https://github.com/jakubroztocil/rrule:
  Undo jkbrzt#91 as it was breaking the demo app
  Allow Negative and 2-Digit Values in Demo
  Bugfix
  Fix typo
  Do not hang if interval is 0 or not a number
  Updating script location in Bower, fixes jkbrzt#61.
* master:
  Renamed isValid() to isValidDate().
  Undo jkbrzt#91 as it was breaking the demo app
  Allow Negative and 2-Digit Values in Demo
  Bugfix
  Updating script location in Bower, fixes jkbrzt#61.

Conflicts:
	bower.json
	lib/rrule.js
@fampinheiro
Copy link

Any updates regarding this?

@aramk
Copy link
Contributor Author

aramk commented Jun 6, 2015

I've been using this in production and it's been working fine. I've added tests, it's just a question of whether this is eventually merged in or not.

@jamesdixon
Copy link

@aramk I'm going to give your fork a shot. I see there a specific branch that is more up-to-date than others? I see one with stable appended to the name, so was curious.

Thanks!

@aramk
Copy link
Contributor Author

aramk commented May 18, 2016

@jamesdixon Yeah the feature/timezone-stable is older and the timezone branch is based off that. I'm using feature/meteor which has a few other fixes actually.

@aramk
Copy link
Contributor Author

aramk commented May 18, 2016

Removed feature/timezone-stable to avoid confusion

@jamesdixon
Copy link

Thanks @aramk! Going to give it a shot. Is there a good place in the repo to grab an example?

@jamesdixon
Copy link

Nevermind, I see the tests. Thanks!

@aramk
Copy link
Contributor Author

aramk commented May 18, 2016

Yeah, see
https://github.com/aramk/rrule/blob/feature/meteor/tests/timezone-tests.js

On Wed, May 18, 2016 at 11:19 AM James Dixon notifications@github.com
wrote:

Nevermind, I see the tests. Thanks!


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#97 (comment)

@Sutikshan
Copy link

Sutikshan commented Jul 6, 2016

Thanks @aramk , I am trying to use your fork in production for our purpose. There are few obstacle I am facing -

  • I am not able to run tests cases, when I try $grunt test, it is failing with error
    Error: Cannot find module 'tz-date'
    Though tz-date is present inside node_modules folder. any clue?
  • When I try to use it in our project as following, it get stuck inside infinite loop of _iter function

new Rrule("FREQ=WEEKLY;BYDAY=FR;BYHOUR=15;BYMINUTE=0;TIMEZONE=Australia/Perth").after(new Date)

Is Dtstart/until are mandatory attribute, if we use your fix?

@aramk
Copy link
Contributor Author

aramk commented Jul 13, 2016

@Sutikshan I think dtstart is mandatory so we have a starting date for the recurrence, and if you don't provide an until it will run endlessly? As far as I can tell I haven't changed the behaviour or interface for the plugin. All existing tests are passing for me on this hash.

@arolson101
Copy link
Collaborator

Hi! I'm trying to clear out bugs and PRs. If this PR is still wanted/relevant, please comment and/or fix the conflicts!

@davidgoli
Copy link
Collaborator

This will be addressed in a separate PR.

@davidgoli davidgoli closed this Aug 13, 2018
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.

None yet

6 participants