[ALOY-1265] Update builtin moment.js library to 2.10.6 (or latest)
GitHub Issue | n/a |
---|---|
Type | Sub-task |
Priority | Critical |
Status | Closed |
Resolution | Fixed |
Resolution Date | 2015-12-17T18:43:03.000+0000 |
Affected Version/s | alloy 1.7.29 |
Fix Version/s | Release 5.2.0, alloy 1.7.30 |
Components | Builtins |
Labels | Alloy, moment.js |
Reporter | Shannon Hicks |
Assignee | Feon Sua Xin Miao |
Created | 2015-05-12T19:28:41.000+0000 |
Updated | 2016-02-02T23:08:45.000+0000 |
Description
Moment.js has changed how it handles time zones/UTC. Took me forever to figure out why my calls to
moment().utcOffset();
didn't work.
Attachments
File | Date | Size |
---|---|---|
Simulator Screen Shot 17 Dec 2015 10.03.24.png | 2015-12-17T09:09:31.000+0000 | 125059 |
Since the new versions of Moment use
require()
in *all* of the [locale files](https://github.com/moment/moment/blob/develop/locale/af.js#L6) it might be worth considering using the [version that has all the locales included](http://momentjs.com/downloads/moment-with-locales.js) so we don't need to patch it at all.PR on master: https://github.com/appcelerator/alloy/pull/687
The version also has a bigger file size than the one without locales included. moment + locales : 361kb moment : 85kb
PR: https://github.com/appcelerator/alloy/pull/747
Verified with:
!Simulator Screen Shot 17 Dec 2015 10.03.24.png|thumbnail!
A few notes on the PR: * We should leave the default locale to
en
as that is the only built-in locale moments has. So https://github.com/feons/alloy/blob/ALOY-1265-update/Alloy/builtins/moment.js#L2799 has to remain like https://github.com/moment/moment/blob/develop/moment.js#L2799 * I actually wonder if the change toloadLocale
(https://github.com/feons/alloy/blob/ALOY-1265-update/Alloy/builtins/moment.js#L269) makes sense. Since we require users to require any languages they want to support trying to load any other language will never succeed. So we might as well makeloadLocale
a NOOP or leave it as is will (which has same effect). We also need to update https://appcelerator.github.io/appc-docs/platform/latest/#!/api/Alloy.builtins.moment to say: * Thatlang()
is now deprecated and should belocale()
* That any languages you want to support at run-time need to be required, hard-coded (no vars) so that Alloy includes them (must be very clear) * That the last required language will become the global locale. * That changes to the global locale will not effect already created instances. * That you can change the locale of a instance without affecting the global locale.1. leave default to locale to
en
, totally agree 2. the change toloadLocale
is needed, because users only have to explicitly do require when the language is not specified in the project's "i18n" folder.OK, with that change I approve ;)
O wait, you probably want to include the API reference change in the PR as well?
Thank you for your approval! :D Yes, will do.
Verified fixed, using: MacOS 10.11.3 (15D21) Studio 4.5.0.201601262138 Ti SDK 5.2.0.v20160202103508 Appc NPM 4.2.3-1 Appc CLI 5.2.0-239 Alloy 1.7.33 Xcode 7.2 (7C68) moment.js library is now using version 2.10.6 and time zones are working as expected. Tested with the provided samples above and simple calls to moment().utcOffset() in a modified sample Alloy app.