Fix hooks.updateOffset to return an immutable Moment

diff --git a/src/lib/create/from-anything.js b/src/lib/create/from-anything.js
index 72aad72bf148644fa9e37cdc4dd255f7e0eab3ef..c94394539061ce9b363d4ed4975b32d3cb1d75f2 100644 (file)
--- a/src/lib/create/from-anything.js
+++ b/src/lib/create/from-anything.js
@@ -18,8 +18,18 @@ import { configFromString }          from './from-string';
 import { configFromArray }           from './from-array';
 import { configFromObject }          from './from-object';
 
+var updateInProgress = false;
+
 function createFromConfig (config) {
     var res = new Moment(checkOverflow(prepareConfig(config)));
+
+    // Prevent infinite loop in case updateOffset creates new moment objects.
+    if (updateInProgress === false) {
+        updateInProgress = true;
+        res = hooks.updateOffset(res);
+        updateInProgress = false;
+    }
+
     if (res._nextDay) {
         // Adding is smart enough around DST
         res = res.add(1, 'd');

diff --git a/src/lib/moment/add-subtract.js b/src/lib/moment/add-subtract.js
index b9e86a69d2a86c3e4e041c1c98874e99e50d9108..6f67e4168e493e701f7852a6216a49d8ebffb4c1 100644 (file)
--- a/src/lib/moment/add-subtract.js
+++ b/src/lib/moment/add-subtract.js
@@ -19,8 +19,7 @@ function createAdder(direction, name) {
 
         val = typeof val === 'string' ? +val : val;
         dur = createDuration(val, period);
-        addSubtract(this, dur, direction);
-        return this;
+        return addSubtract(this, dur, direction);
     };
 }
 
@@ -31,7 +30,7 @@ export function addSubtract (mom, duration, isAdding, updateOffset) {
 
     if (!mom.isValid()) {
         // No op
-        return;
+        return mom;
     }
 
     updateOffset = updateOffset == null ? true : updateOffset;
@@ -46,8 +45,9 @@ export function addSubtract (mom, duration, isAdding, updateOffset) {
         setMonth(mom, get(mom, 'Month') + months * isAdding);
     }
     if (updateOffset) {
-        hooks.updateOffset(mom, days || months);
+        mom = hooks.updateOffset(mom, days || months);
     }
+    return mom;
 }
 
 export var add      = createAdder(1, 'add');

diff --git a/src/lib/moment/constructor.js b/src/lib/moment/constructor.js
index bc53f814fcda507cb7a59e6ff1bd832822fb8934..c28cb3c7ac8f29a29c00d93bdb861efa1226f59d 100644 (file)
--- a/src/lib/moment/constructor.js
+++ b/src/lib/moment/constructor.js
@@ -54,22 +54,18 @@ export function copyConfig(to, from) {
     return to;
 }
 
-var updateInProgress = false;
-
-// Moment prototype object
+// Moment prototype object.
+//
+// Run hooks.updateOffset(new Moment(config)) if you're constructing from user
+// input, but be careful to detect and break out of loops in case the user's
+// version of hooks.updateOffset() decides to trigger the same code path.
+// (Or just use from-anything's createFromConfig(), which handles this for you.)
 export function Moment(config) {
     copyConfig(this, config);
     this._d = new Date(config._d != null ? config._d.getTime() : NaN);
     if (!this.isValid()) {
         this._d = new Date(NaN);
     }
-    // Prevent infinite loop in case updateOffset creates new moment
-    // objects.
-    if (updateInProgress === false) {
-        updateInProgress = true;
-        hooks.updateOffset(this);
-        updateInProgress = false;
-    }
 }
 
 export function isMoment (obj) {

diff --git a/src/lib/moment/get-set.js b/src/lib/moment/get-set.js
index abdb8f4c01aa539dfcedfd5e448b7a851b914882..e23c401c04481625e909ee3212e5124da3d3d865 100644 (file)
--- a/src/lib/moment/get-set.js
+++ b/src/lib/moment/get-set.js
@@ -8,8 +8,7 @@ export function makeGetSet (unit, keepTime) {
     return function (value) {
         if (value != null) {
             set(this, unit, value);
-            hooks.updateOffset(this, keepTime);
-            return this;
+            return hooks.updateOffset(this, keepTime);
         } else {
             return get(this, unit);
         }

diff --git a/src/lib/units/month.js b/src/lib/units/month.js
index a09ffca7962a09dde81cf064ef3574fe60c74502..e0fd9e46d7d78ad033f8e82004e338cdf6839a2d 100644 (file)
--- a/src/lib/units/month.js
+++ b/src/lib/units/month.js
@@ -193,8 +193,7 @@ export function setMonth (mom, value) {
 export function getSetMonth (value) {
     if (value != null) {
         setMonth(this, value);
-        hooks.updateOffset(this, true);
-        return this;
+        return hooks.updateOffset(this, true);
     } else {
         return get(this, 'Month');
     }

diff --git a/src/lib/units/offset.js b/src/lib/units/offset.js
index b24fed2070a52d24f2df835caf46f9e1d6ef8fd7..3b6b543df8c2c87e5d6088737ba300f53e552a04 100644 (file)
--- a/src/lib/units/offset.js
+++ b/src/lib/units/offset.js
@@ -71,8 +71,7 @@ export function cloneWithOffset(input, model) {
         diff = (isMoment(input) || isDate(input) ? input.valueOf() : createLocal(input).valueOf()) - res.valueOf();
         // Use low-level api, because this fn is low-level api.
         res._d.setTime(res._d.valueOf() + diff);
-        hooks.updateOffset(res, false);
-        return res;
+        return hooks.updateOffset(res, false);
     } else {
         return createLocal(input).local();
     }
@@ -88,7 +87,13 @@ function getDateOffset (m) {
 
 // This function will be called whenever a moment is mutated.
 // It is intended to keep the offset in sync with the timezone.
-hooks.updateOffset = function () {};
+//
+// Because Moment's external API is immutable and this hook will be defined
+// externally (e.g. by Moment Timezone), this hook must return a (possibly new)
+// Moment instance that reflects the correctly-updated offset.
+hooks.updateOffset = function (m) {
+    return m;
+};
 
 // MOMENTS
 
@@ -109,6 +114,7 @@ export function getSetOffset (input, keepLocalTime, keepMinutes) {
         return input != null ? this : NaN;
     }
     if (input != null) {
+        var ret = this;
         if (typeof input === 'string') {
             input = offsetFromString(matchShortOffset, input);
             if (input === null) {
@@ -117,24 +123,24 @@ export function getSetOffset (input, keepLocalTime, keepMinutes) {
         } else if (Math.abs(input) < 16 && !keepMinutes) {
             input = input * 60;
         }
-        if (!this._isUTC && keepLocalTime) {
-            localAdjust = getDateOffset(this);
+        if (!ret._isUTC && keepLocalTime) {
+            localAdjust = getDateOffset(ret);
         }
-        this._offset = input;
-        this._isUTC = true;
+        ret._offset = input;
+        ret._isUTC = true;
         if (localAdjust != null) {
-            this.add(localAdjust, 'm');
+            ret = ret.add(localAdjust, 'm');
         }
         if (offset !== input) {
-            if (!keepLocalTime || this._changeInProgress) {
-                addSubtract(this, createDuration(input - offset, 'm'), 1, false);
-            } else if (!this._changeInProgress) {
-                this._changeInProgress = true;
-                hooks.updateOffset(this, true);
-                this._changeInProgress = null;
+            if (!keepLocalTime || ret._changeInProgress) {
+                ret = addSubtract(ret, createDuration(input - offset, 'm'), 1, false);
+            } else if (!ret._changeInProgress) {
+                ret._changeInProgress = true;
+                ret = hooks.updateOffset(ret, true);
+                ret._changeInProgress = null;
             }
         }
-        return this;
+        return ret;
     } else {
         return this._isUTC ? offset : getDateOffset(this);
     }

diff --git a/src/test/moment/getters_setters.js b/src/test/moment/getters_setters.js
index 0f9627c003a8d026ae68f9f972ba88873869faf2..063a45def1331ceb3eab6bd453f313ec0a2c5f25 100644 (file)
--- a/src/test/moment/getters_setters.js
+++ b/src/test/moment/getters_setters.js
@@ -282,10 +282,9 @@ test('setters across DST +1', function (assert) {
 
     moment.updateOffset = function (mom, keepTime) {
         if (mom.isBefore(dstAt)) {
-            mom.utcOffset(-8, keepTime);
-        } else {
-            mom.utcOffset(-7, keepTime);
+            return mom.utcOffset(-8, keepTime);
         }
+        return mom.utcOffset(-7, keepTime);
     };
 
     m = moment('2014-03-15T00:00:00-07:00').parseZone();
@@ -315,10 +314,11 @@ test('setters across DST -1', function (assert) {
 
     moment.updateOffset = function (mom, keepTime) {
         if (mom.isBefore(dstAt)) {
-            mom.utcOffset(-7, keepTime);
+            mom = mom.utcOffset(-7, keepTime);
         } else {
-            mom.utcOffset(-8, keepTime);
+            mom = mom.utcOffset(-8, keepTime);
         }
+        return mom;
     };
 
     m = moment('2014-11-15T00:00:00-08:00').parseZone();

diff --git a/src/test/moment/start_end_of.js b/src/test/moment/start_end_of.js
index ba055d5f29b99b0d22f59335abba766b12ae2fa0..723d66030af422fc72bc7a91b3c948a4ed633a5e 100644 (file)
--- a/src/test/moment/start_end_of.js
+++ b/src/test/moment/start_end_of.js
@@ -316,10 +316,11 @@ test('startOf across DST +1', function (assert) {
 
     moment.updateOffset = function (mom, keepTime) {
         if (mom.isBefore(dstAt)) {
-            mom.utcOffset(-8, keepTime);
+            mom = mom.utcOffset(-8, keepTime);
         } else {
-            mom.utcOffset(-7, keepTime);
+            mom = mom.utcOffset(-7, keepTime);
         }
+        return mom;
     };
 
     m = moment('2014-03-15T00:00:00-07:00').parseZone();
@@ -355,10 +356,11 @@ test('startOf across DST -1', function (assert) {
 
     moment.updateOffset = function (mom, keepTime) {
         if (mom.isBefore(dstAt)) {
-            mom.utcOffset(-7, keepTime);
+            mom = mom.utcOffset(-7, keepTime);
         } else {
-            mom.utcOffset(-8, keepTime);
+            mom = mom.utcOffset(-8, keepTime);
         }
+        return mom;
     };
 
     m = moment('2014-11-15T00:00:00-08:00').parseZone();

diff --git a/src/test/moment/utc_offset.js b/src/test/moment/utc_offset.js
index 717a78824cf7ae962a19e5edb39f08ddca528de9..64f4903136a849dbb4c2264f30665e4d136e0cd7 100644 (file)
--- a/src/test/moment/utc_offset.js
+++ b/src/test/moment/utc_offset.js
@@ -121,23 +121,25 @@ test('distance from the unix epoch', function (assert) {
 
 test('update offset after changing any values', function (assert) {
     var oldOffset = moment.updateOffset,
-        m = moment.utc([2000, 6, 1]);
+        m = moment.utc([2000, 6, 1]),
+        doChange = false;
 
     moment.updateOffset = function (mom, keepTime) {
-        if (mom.__doChange) {
+        if (doChange) {
             if (+mom > 962409600000) {
-                mom.utcOffset(-120, keepTime);
+                mom = mom.utcOffset(-120, keepTime);
             } else {
-                mom.utcOffset(-60, keepTime);
+                mom = mom.utcOffset(-60, keepTime);
             }
         }
+        return mom;
     };
 
     assert.equal(m.format('ZZ'), '+0000', 'should be at +0000');
     assert.equal(m.format('HH:mm'), '00:00', 'should start 12AM at +0000 timezone');
 
-    m.__doChange = true;
-    m.add(1, 'h');
+    doChange = true;
+    m = m.add(1, 'h');
 
     assert.equal(m.format('ZZ'), '-0200', 'should be at -0200');
     assert.equal(m.format('HH:mm'), '23:00', '1AM at +0000 should be 11PM at -0200 timezone');
@@ -314,10 +316,11 @@ test('add / subtract over dst', function (assert) {
 
     moment.updateOffset = function (mom, keepTime) {
         if (mom.utc().month() > 2) {
-            mom.utcOffset(60, keepTime);
+            mom = mom.utcOffset(60, keepTime);
         } else {
-            mom.utcOffset(0, keepTime);
+            mom = mom.utcOffset(0, keepTime);
         }
+        return mom;
     };
 
     assert.equal(m.hour(), 3, 'should start at 00:00');
@@ -348,10 +351,11 @@ test('isDST', function (assert) {
 
     moment.updateOffset = function (mom, keepTime) {
         if (mom.month() > 2 && mom.month() < 9) {
-            mom.utcOffset(60, keepTime);
+            mom = mom.utcOffset(60, keepTime);
         } else {
-            mom.utcOffset(0, keepTime);
+            mom = mom.utcOffset(0, keepTime);
         }
+        return mom;
     };
 
     assert.ok(!moment().month(0).isDST(),  'Jan should not be summer dst');
@@ -360,10 +364,11 @@ test('isDST', function (assert) {
 
     moment.updateOffset = function (mom) {
         if (mom.month() > 2 && mom.month() < 9) {
-            mom.utcOffset(0);
+            mom = mom.utcOffset(0);
         } else {
-            mom.utcOffset(60);
+            mom = mom.utcOffset(60);
         }
+        return mom;
     };
 
     assert.ok(moment().month(0).isDST(),  'Jan should be winter dst');

diff --git a/src/test/moment/zones.js b/src/test/moment/zones.js
index b8df23411e615c459fca494acda92440c7a2914e..3226a024806ab3af8037ebdb4c83836628a593f4 100644 (file)
--- a/src/test/moment/zones.js
+++ b/src/test/moment/zones.js
@@ -112,23 +112,25 @@ test('distance from the unix epoch', function (assert) {
 
 test('update offset after changing any values', function (assert) {
     var oldOffset = moment.updateOffset,
-        m = moment.utc([2000, 6, 1]);
+        m = moment.utc([2000, 6, 1]),
+        doChange = false;
 
     moment.updateOffset = function (mom, keepTime) {
-        if (mom.__doChange) {
+        if (doChange) {
             if (+mom > 962409600000) {
-                mom.zone(120, keepTime);
+                mom = mom.zone(120, keepTime);
             } else {
-                mom.zone(60, keepTime);
+                mom = mom.zone(60, keepTime);
             }
         }
+        return mom;
     };
 
     assert.equal(m.format('ZZ'), '+0000', 'should be at +0000');
     assert.equal(m.format('HH:mm'), '00:00', 'should start 12AM at +0000 timezone');
 
-    m.__doChange = true;
-    m.add(1, 'h');
+    doChange = true;
+    m = m.add(1, 'h');
 
     assert.equal(m.format('ZZ'), '-0200', 'should be at -0200');
     assert.equal(m.format('HH:mm'), '23:00', '1AM at +0000 should be 11PM at -0200 timezone');
@@ -296,10 +298,11 @@ test('add / subtract over dst', function (assert) {
 
     moment.updateOffset = function (mom, keepTime) {
         if (mom.utc().month() > 2) {
-            mom.zone(-60, keepTime);
+            mom = mom.zone(-60, keepTime);
         } else {
-            mom.zone(0, keepTime);
+            mom = mom.zone(0, keepTime);
         }
+        return mom;
     };
 
     assert.equal(m.hour(), 3, 'should start at 00:00');
@@ -330,10 +333,11 @@ test('isDST', function (assert) {
 
     moment.updateOffset = function (mom, keepTime) {
         if (mom.month() > 2 && mom.month() < 9) {
-            mom.zone(-60, keepTime);
+            mom = mom.zone(-60, keepTime);
         } else {
-            mom.zone(0, keepTime);
+            mom = mom.zone(0, keepTime);
         }
+        return mom;
     };
 
     assert.ok(!moment().month(0).isDST(),  'Jan should not be summer dst');
@@ -342,10 +346,11 @@ test('isDST', function (assert) {
 
     moment.updateOffset = function (mom) {
         if (mom.month() > 2 && mom.month() < 9) {
-            mom.zone(0);
+            mom = mom.zone(0);
         } else {
-            mom.zone(-60);
+            mom = mom.zone(-60);
         }
+        return mom;
     };
 
     assert.ok(moment().month(0).isDST(),  'Jan should be winter dst');