Call updateOffset when creating moment #2054
Conversation
|
OK, that looks pretty good, I'll merge it in next release, we'll sync with moment-timezone. |
|
Merged in e2e4f4f |
|
Thanks! :) Can moment-timezone#15 be closed? |
|
This creates infinite recursion when the updateOffset function has to create or clone a moment object. I hit this in tests, so we should think about something else. It might not go out in 2.9 (unless you figure a decent fix very fast). I will think about it after 2.9. Reverted in 6d3bc5f |
|
Well that's unfortunate. Do you have a reproducible test case? I haven't had a chance to look at the code yet (and in any case it will be easier with a test case), but: Can we fix the infinite recursion by introducing a state variable that a moment is being created and avoid calling updateOffset if it's set? |
Call updateOffset when creating moment
Call updateOffset when creating moment
|
I made sure (with a global boolean variable) that there would be no infinite loop. The way moment-timezone and moment are tied right now (with updateOffset) -- its too loose to assume that updateOffset sets What we really need is the notion of a zone (something with DST, a collection of utc offsets) in moment, and something (external) to tell it what is the offset in a zone of a particular UTC time. Then moment should handle the actual zone switching not some random function. We already to tricks in addOrSubtract (https://github.com/moment/moment/blob/ef27605/moment.js#L2492) because we can get recursive calls just from normal operation. I'm actually working on https://github.com/ichernev/redate which would handle just that (proper zone switching). I'll see how it will play with moment (if at all) in the future. Merged (with fixes) in ef27605 |
|
Thanks for taking the time to fix it! The In any case, I'm very happy to hear the issue has been addressed and am looking forward to 2.9. :) |
0.3.1 * tag '0.3.1': (31 commits) Version 0.3.1 time zone data: 2015a Remove references to tests/moment-timezone-utils/*. #174 [Kashif] In meta data, filter coutries which don't have any timezones for them. eg: 'Bovet island' Add links to the changelog. Version 0.3.0. Add a hint when using an older version of moment with setDefault() Renable default zone tests now that moment@2.9.0 is released. Don't add an offset to timezone moments that were created from cloned moments. Remove needsOffset tests for cloned moments. Commenting out default tests until moment/moment#2054 lands Failing tests for #135 Add temporary wrapper for moment#utcOffset until it lands in moment core Add tests for isSame() #127 Fix for error "stderr: maxBuffer exceeded", while running `data-zdump` grunt task. Rebuild metadata for all 2014 releases according to the new schema Updates to the grunt task `data:meta`: 1. Pick "zone1970.tab" as its input source if available, defaults to zone.tab. (until 'tzdata2014f', only zone.tab was available, but now it's the deprecated version) 2. Output JSON of this task now has two hashes: "countries" and "zones" to facilitate consumers to deal much better with showing timezone choices. 3. Updated contribution docs to add info about "data-meta" grunt task. Update tests to use utcOffset instead of zone Prefer moment.fn.utcOffset to moment.fn.zone Version 0.2.5 ... Conflicts: package.json
Goes along with moment-timezone/pull/152.