make moment mostly immutable

[ichernev]

There has been a lot of discussions about this. Here's the proposal:

The following mutable methods will become immutable in 3.0.0: utc, local, zone, add, subtract, startOf, endOf, lang, also in duration: add, subtract, and lang.

As a start, all methods will be duplicated with methodNameMute variants. We also need immutable variants named methodNameImmute. From 3.0 the plain old methods will start using the immutable option by default.

What is debatable is:

• should lang be made immutable
• should all the getters/setters (including get/set) be made immutable too
• naming of the mutable and immutable versions of the methods
• and of course -- should we do the switch, or just stop at the immutable API

The good part is we can make immutable versions of the methods today and decide later on what to do. If we switch it would also mean the 2.x branch would be around for quite some time after 3.x is out.

@icambron @timrwood @gregwebs @yang @lfnavess @soswow @langalex

[icambron]

My two cents is that we should either go for full-hog immutability or not at all. Having some methods be immutable (startOf, add) and some not (year, get) is just confusing, and developers will have to keep track of which ones are which.

[gregwebs]

I would also prefer everything immutable by default. Are the getters not immutable already?

[timrwood]

Here are the big issues I see with switching to immutability.

Pseudo-Immutability

Because of the nature of javascript, we will never be able to have true immutability.

The best we can do is create a copy and then mutate and return that copy. We can carefully wrap all the public methods to make sure we are always copying and mutating rather than just mutating, but that doesn't prevent someone from doing m._d = new Date() or even m._d.setHours(1).

I agree with @icambron, if we move to immutability, it should be a complete change. Any method that could possibly change a property on a moment would instead create a copy of the moment and make a change on the copy instead.

Api Surface Area

The unfortunate thing about switching to pseudo-immutability is that many new apis need to be created if we want to still support mutability.

Before, switching from mutating a moment to cloning and mutating a moment was as simple as adding a .clone() in the right place. Now, we would have to create mutable interfaces for all setters, which adds considerably to the api surface area.

This includes the ~20 setter methods, add/subtract, local/utc/zone/tz, startOf/endOf, lang, and any other methods used in 3rd party plugins.

Memory Concerns

Because we are now creating copies every time we want to change a value, the memory usage will increase. Of course, the new moments will be garbage collected, but the additional cost associated with this change is something to keep in mind.

We would have to take great care to make sure we are not creating tons of disposable clones with methods that use other setters.

To track what methods are being called, I used this little function wrapper.

for (var method in moment.fn) {
  moment.fn[method] = (function (fn, method) {
    return function () {
      console.log(method);
      return fn.apply(this, arguments)
    }
  })(moment.fn[method], method)
}

Now, when running a method, we can see how may other methods on the prototype are used. Not all of these methods would need to clone the moment, so I added comments on the ones that would require cloning.

moment().isSame(moment(), 'year')
isSame
clone        // clone
startOf      // clone
month        // clone
date         // clone
year         // clone
date         // clone
hours        // clone
minutes      // clone
seconds      // clone
milliseconds // clone
valueOf
local        // clone
zone         // clone
startOf      // clone
month        // clone
date         // clone
year         // clone
date         // clone
hours        // clone
minutes      // clone
seconds      // clone
milliseconds // clone
valueOf

That is 21 copies that are created and then immediately discarded. Obviously we could optimize this by using some internal methods that are mutable and only expose the immutable versions, but it will significantly add to the internal complexity trying to keep a record of which moments still need cloning and which don't.

Performance Concerns

Cloning a moment is much slower than mutating a moment. I threw together a couple jsperf tests for this.

http://jsperf.com/moment-cloning

http://jsperf.com/moment-cloning-2

I think the second test is a much better representation of the performance losses when switching to pseudo-immutability. If we multiply those results by the 21 cloning instances noted above, the execution times are that much slower.

I'm sure we could optimize the path for cloning a moment, but we would need to make it 50x faster in order to have comparable performance. I'm pretty sure this is impossible.

Summary

Switching to immutability greatly increases the complexity of the internal and external apis and has major performance and memory concerns. I don't think those costs are worth the benefits that immutability would provide.

[gregwebs]

I think the performance concerns listed here are missing the point:

Generally speaking, an initial .clone() is needed to ensure correctness before performing mutation.

We can't pretend like clone() is not needed with the current API. The main case here that is different is performing multiple sequential mutations. That case is addressed by creating a builder API such that all the mutations are performed as mutations on a single clone.

Am I missing other common use cases?

[soswow]

My original issue was about startOf and endOf methods specifically. For some reason these names for me were like "get me startOf a month" not "set this moment to the startOf a month". Methods like add and subtract are perfectly ok in the way they are in sense of semantics. It's perfectly ok to Add something To an object without creating new one.
For me personally renaming methods startOf and endOf to smth like toStartOf and toEndOf (like "move this moment to start of a month") would solve the issue. IMHO

[icambron]

@gregwebs Sorry, I meant set above.

I disagree with @soswow; I think it needs to be consistent. In fact, I think that toStartOf implies immutability even more strongly, like it's providing an alternative presentation a la toISOString. More importantly, I think we need to be able to make statements like "Moment's setters mutate moments" or "Moments setters return copies", not "well, for these methods..."

On @timrwood's concerns:

That the JS objects won't be truly immutable doesn't bother me. The point is that the API provide an immutable contract. Of course the user can cheat by fiddling with the underscored properties, and cheating is generally possible even languages where immutability is the main way of doing things.

On surface area and about perf: I think we'll need to use the mutators internally to avoid using up all that CPU and memory [1], so we'll then we'll have to support them at some level. Then we might as well expose them externally, like setYear(), etc. That adds a bunch of surface area, but it doesn't really add much complexity; for non-explicit-mutators, clone externally, mutate internally.

One way to look at that is that the user has to clone in their code, so Moment might as well be doing for them. That does present a problem with chaining in perf-sensitive places, which could be battled by either a builder interface (per Greg's idea) or by letting the user just use mutators there. The builder adds a bunch complexity [2], so I think I just favor explicit mutator alternatives. I think the reality is that most of the time, Moment is not being used in perf-sensitive situations, so those situations don't have to be the most convenient API-wise. I'd rather have a nice immutable API with a perf hatch for when I need it.

[1] The cool kids in FP land solve this with structural sharing, but that's probably impractical here.

[2] Traditionally, people make builders that are separate objects, but that would be really verbose here, since you'd have to copy the whole setter API. Just spitballing, but one alternative is that .chain() creates a clone moment that just has an isBuilding flag set on it. Then internal clones are ignored, just returning the object for mutation. Then build() unsets the flag and returns that clone. The trouble is that you need your getters to scream bloody murder if the flag is set, or people will end up using the using the chained but unbuilt Moments which are suddenly mutatorish. Then you need to differentiate externally and internally called getters. Blech. Another alternative is to internally decompose the functionality needed by the builder into a mixin and use it in both the builder and in Moment, but that's probably not workable from a code-organization perspective.

[lfnavess]

what worked to me, was adding an extra parameter to the functions, a flag (i named self) to denote mutability, by default is in inmutable (return a copy or new object), and when i detect perfomance i set the flag to true

this stand point solved a lot of conflicts,
having functions with similar name executing almost the same code,
or having to change the functionname and problably the parameters when i detect performance points
in my public methods i start the code calling the functions with a copy and following calls with the flag in true
with this i also i can chain the functions

in my code i work with arrays of arrays, (like a table, array of rows)
so i have functions to filter, union, etc, that previusly retruns a new array with the reult, and i detect that to get the final result i called several times the same function, now the first call is to create a copy and not change the initial array, and teh following calls i work with the same array droping row that i don't need

an basic example that could be here:
moment.add = function(measure, ammount, self){
}

[lfnavess]

moment.add = function (measure, ammount, self) {
var $moment = self ? this : this.clone();
// the actual code
return $moment;
}

[ichernev]

Thank you everybody for their 2 cents :)

For the protocol, I agree on @icambron's last post on all points.

There are two big questions left.

The easier one is what should the new API be, two options:
1.1 differently named methods (mutable and immutable) year/setYear, startOf/setStartOf
1.2 or builder api chain().mutators().build(), which is the non-hacky version of what @lfnavess proposed.

The builder api definitely looks sexier, but care should be taken that objects don't stay in build mode for too long, which adds another source of trouble for us and the users.

Now the hard problem -- migrating to the new version. I see two options here:
2.1 devs have to rewrite their code (crazy regex might work in 1.1 and AST-level solution for 1.2 -- given nobody uses year and month as the names of their own methods). python took this approach -- we can all see the result -- a brand new language was born!
2.2 option to turn builder api always on (same as now), and a way to deactivate it for new code. This looks more evolutionary, but the amount of confusion it would cause is probably not worth it. Every moment now has a two flags: is it mutable, and in case it is -- is it strictly mutable (no getters) or transitionally mutable (getters ok). Not to mention receiving moment objects in functions -- you should check what the mode is, make sure to maintain it ... what a mess!

---

And now a crazy idea that came to me now

Copy-on-write clone

m = moment();
funcIDontTrust(m.clone());  // doesn't actually clone

function funcIDontTrust(m) {
  m.year(2005);  // perform the clone here
  console.log(m);
}

I'm not sure how much can be shaved off with this approach, given moment instances are pretty light. Also all mutators now have to perform a check.

There are a few ways to implement with varying performance in different scenarios. The good news is that its backwards compatible and we'll save us and our users a great deal of effort. And I think this is more important than reinventing the wheel.

[timrwood]

I'm not sure what we are gaining here.

Switching to immutablilty has a ton of associated costs and maybe I'm missing it, but
I don't really see comparable benefits.

The main benefits seem to be developer preference. Is this all so that developers don't have
to think about ownership of a moment when passing it around?

I don't think switching to immutability will make bugs any less frequent, it will just
change the type of bugs. @ichernev's example even shows the exact kind of bugs that will
surface, which are just as hard to track down.

m = moment();
funcIDontTrust(m.clone());  // doesn't actually clone

function funcIDontTrust(m) {
  m.year(2005);  // perform the clone here
  // m is still in 2014
  // m.year(2005) created a clone but did not assign it to anything
  // it should be `m = m.year(2005)`
  console.log(m);
}

Here is a pros/cons list between mutability and immutability. If I missed anything,
let me know and I'll edit this comment.

Immutable	Mutable
Some devs prefer it	Some other devs prefer it
Avoids bugs when passing around moments	Avoids bugs when forgetting to assign cloned moments
With a few dozen new api methods, mutability will also be supported	With the existing .clone() method, immutability is already supported
	An order of magnitude faster
	Uses significantly less memory

I do think immutability is useful, but I don't think it is a good fit in JavaScript. I think an immutable interface may make sense for a lanugage like Elm, where immutablity is expected, but for JavaScript, I think mutability is expected.

Much of the apis for typeof a === "object" built-ins are mutable. Array#push,pop,reverse,sort,shift,splice,unshift all change the contents of an array rather than returning a new array. All 16 of the Date#setX methods mutate their instance.

I think we are seeing a lot of people complaining about moments being mutable, but if we switch, I think we will have just as many people complaining. This has already happened with the eod/sod methods two years ago.

After looking at a bunch of old issues on mutability, I guess I'm probably sounding like a broken record here. On both sides, it's the same points that have been brought up for the past few years. I just wanted to make sure an argument for keeping a mutable api was represented in the discussion.

[gregwebs]

@timrwood those are good comparisons, but it is pretty clear you haven't taken the time to understand the immutable use case. We have already discussed why the performance comparisons you posted assume a poorly implemented API and do not make sense.

The bug comparison is also invalid. Because momentjs supports a chaining API, one could expect it to be immutable.

var newM = m.year(2005) // wrong, these are both the same!

So both immutable and mutable have the same problem right now. You could avoid it with the current mutable version if you got rid of the chaining API.

So the immutable API is preferable to mutable because you can safely pass a moment between functions. With the current mutable moments if I pass a moment between function I have 2 options

1. The insane buggy way (that is probably most common): investigate all the source code to make sure there aren't unwanted mutations. Write unit tests to make sure unwanted mutations don't creep in.
2. The sane way (lets instead assume everyone is doing this), defensive programming: remember to call the clone() function before mutating in my function.

With the immutable API we don't have to remember to call clone() every time. Instead, we have to remember to call the API function that lets us avoid cloning, but this is only a performance optimization, not an issue of correctness.

[timrwood]

it is pretty clear you haven't taken the time to understand the immutable use case

That's an unfair statement. My argument is that I see the benefits, but do not think they outweigh the costs.

you can safely pass a moment between functions

we don't have to remember to call clone

Is this not the use case for immutability? If there is more to it that I haven't taken the time to understand, please let me know, but this seems to be the only argument for the past few years.

[gregwebs]

@timrwood yes, that is the entire case.

But I don't see a sign from you acknowledging that your case against immutability (horrible performance, promotes a different type of bug not present in the mutable API) is not valid.

[lfnavess]

i don't know if freeze() could help
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/freeze

[lfnavess]

i think we should stick with the ecmascript 5 point of view, and maybe add a function that deep freeze the current object, or a global flag that automatically creates frezee objects

http://blogorama.nerdworks.in/preventextensionssealandfreeze/

[lfnavess]

maybe a extra parameter in the constructor to create a freeze object, because a freeze object can't be unfreeze

[ichernev]

@lfnavess I thought about freeze before mentioning the copy on write. The problem is that ... nobody uses it / knows about it, nor does it help when it doesn't throw an exception (in non-strict mode) -- it actually creates crazy bugs for you to track.

@timrwood I don't think I made my example clear. On the line m.year(2014) // clone here I meant that internally moment would actually make a clone (allocate more memory) and m would point to that new memory, automatically. Well that basically means that clone() should also allocate a bit of shell memory (something to point to the internal date representation), I'm just not sure how much would be gained by doing so.

Creating a half-assed version of clone, that clones only the interface, and the ability to change the underlying data (from shared storage to instance-specific) -- it really depends on how expensive Date objects are. The downside is that every function needs to do this._storage._d instead of this._d, and I'm not sure if that would overcome the benefit.

I didn't get any any comments on how to deal with migrating the existing libraries/users of moment. I don't really like any of the options I listed above.

[icambron]

Reverse-compatability is, IMO, the strongest argument against this. If we do this, we just have to accept that it's a big breaking change. If we don't want to accept that, we shouldn't do it.

It's worth mentioning re:perf that there are also some huge advantages you can gain from immutability; it's not a monotonic perf hit. For example, you can cache things at the object level, because they'll never change. I also think we should be able to optimize the living crap out of clone(); AFAIK it involves cloning a date and copying like five values; I think we should just hardcode them like newThing._offset = oldThing._offset.

Edit, arg, no - plugins add fields too (e.g. here).

[gregwebs]

given the strong desire for backwards compatibility and yet to keep thing light-weight, I think the best solution is to fork the javascript sources (either in the sources of this project or start an entirely new project). There is room for more than 1 time library for the internet.

[icambron]

Also: re: @ichernev's idea on structural sharing, one possibility is to use prototype inheritance instead of wrapping a shared state object.

[butterflyhug]

We at WhoopInc have been lurking in this discussion for quite a while. Since the discussion here seems to be going in circles, I took some time this weekend to explore what an immutable version of moment with a builder API might look like. (I have no intention of submitting a PR against moment unless invited to do so, since I'm deliberately making more strident API changes than I would expect to ever see implemented in moment itself.) Here's the result: https://github.com/WhoopInc/frozen-moment

I'm just a few hours in, so everything is really rough around the edges, but based on test results I think most of the core moment functionality is working. I'll continue to track this conversation, and I'd welcome feedback specific to our fork in our repo's issues.

I'll try to publish some updated docs on that repo tonight, but basically I just split out all the setter and mutation methods into a separate builder object. So using the API might look like frozenMoment("2014-07-21").thaw().subtract(1, "day").startOf("day").freeze().format("YYYY-MM-DD"). (Although in this particular example it would be more efficient to just start the chain with a builder instead of initializing a builder from a frozenMoment, using frozenMoment.build("2014-07-21").subtract...)

[jehoshua02]

FWIW, when I started using moment I had assumed it followed FP principles and would return the same value every time I call a function:

var now = moment();
var yesterday = now.subtract(1, 'days');
var dayBeforeYesterday = now.subtract(2, 'days');

Of course, I didn't get the results I expected. This caught me off guard as a new user.

Consider this pseudocode, which demonstrates how I would have expected the code to behave:

var now = now;
var yesterday = now - 1day;
var dayBeforeYesterday = now - 2days;

But instead it ended up working like this, which feels strange to me:

var now = now;
var yesterday = now = now - 1day;
var dayBeforeYesterday = now = now - 2days;

For now, even though it's quite tedious, I just carefully .clone() everywhere.

var now = moment();
var yesterday = now.clone().subtract(1, 'days');
var dayBeforeYesterday = now.clone().subtract(2, 'days');

IMO, Javascript is prone to subtle errors and I feel that FP principles help minimize those errors.

I sympathize that this is a hard decision to make. I appreciate your work. Moment.js is amazing.

[AlexGalays]

+1 for 100% immutability.

[fingermark]

It's honestly kind of frustrating having it not be immutable.

[nikoskalogridis]

+1 for 100% immutability in version 3

[ForNeVeR]

There definitely should be an immutable API. As a user of other date libraries (notably, .NET DateTime and Joda Time / Noda Time), I intuitively expect that the add method will not mutate the date object.

[chopachom]

+1 for immutability

[hojberg]

+1 for 100% immutability

[jehoshua02]

If the decision is made for immutability, I'd be willing to give of my time to make it happen. Perhaps pairing over a video call. I'd like to contribute more to open source but need to learn the ropes.

[dsherret]

To me, immutability is preferable, but it should have been done from the start. This is a drastic breaking change. A fork of this project that focuses on immutability would be a better idea.

[jehoshua02]

@dsherret That's what semver is for. We'd just bump the major version.

[jehoshua02]

However, with some effort it could be introduced as a configuration option: "Do you want everything immutable? True or False". Default would be false.

[timrwood]

Unsupported unofficial ymmv immutable moment plugin here: https://gist.github.com/timrwood/fcd0925bb779f4374a7c

[magwo]

Haha! I was surprised to come here so much later and discover that I was one of the first proponents for a more immutable API.. :)

[magwo]

And yes I'm +1 for immutability for next major version.

[idrm]

Another +1 for immutable-by-default moment from me.
And here's my 'imoment' concoction: https://gist.github.com/idrm/a91dc7a4cfb381dca24e (use at your own risk!). Just replace your moment() calls with imoment() and that should be enough. All static moment.xyz() (e.g. moment.min(), moment.max(), etc.) function calls should remain as is.

[dan-dr]

+1million dollars

[rankida]

+1 for immutability

[idrm]

May I also add a +1 to a previous suggestion from this discussion thread to rename some of the functions so they are easier to read ("startOf" to "toStartOf", "add" to "plus", "month" to "withMonth", etc.)? That is, assuming you take the immutable-by-default route. I use Joda Time a lot, and it's a snap to figure out what, for example, "date.withDayOfMonth(1).withDayOfWeek(DateTimeConstants.MONDAY)" means.
These don't have to be in the main distro JS, either; an add-on that slaps these on top of the vanilla JS would work just as well (heck, I am strongly considering writing such a Joda Time-aligned add-on myself to combine with my "imoment" mod).

[alexbepple]

@ichernev, @icambron, have you made a decision on this? Will moments will be immutable in 3.0? If so: when do you expect 3.0 to be out? I am asking 'cause I am considering using frozen-moment or writing a small wrapper myself – and I wonder whether I should wait.

[butterflyhug]

FYI, frozen-moment has mostly been in a holding pattern recently -- I've ported a bunch of PRs from upstream Moment, but haven't really worked on any of the other refactoring that should ideally happen in that fork.

That said, frozen-moment should work great if you only need the default English locale. (Everything works for my use cases, and I've maintained Moment's high unit test coverage.) Non-English locales are broken because I didn't get them updated after porting Moment's recently refactored locale APIs.

Once we have a decision for Moment 3.0, I'll plan to work a bit more seriously on immutability work in Moment, or on the frozen-moment fork, as appropriate. My initial goal would be to fix locale support and maintain feature parity with Moment 3.0's API.

[jasonhansel]

+1 for immutability

[eliseumds]

+1 for immutability. What about the format method that also mutates the object?

[karl-gustav]

+1 for an immutable moment.js
Or maybe a fork of moment.js? immutable-moment.js? Since this will definitely be a breaking change.

[boxxxie]

💯
do the imootables!

[SystemParadox]

+1 these days immutability is something I expect in any good JS API

[abergs]

👍 Yes please, we would very much like this. Currently we sprinkle .clone() everywhere.

[manuelbernhardt]

+1 this would be a very nice improvement

[martiansnoop]

👍 immutable all the things

[adamhutch]

+1 for immutable

[bastienJS]

Make it all immutable. I am sick of cloning every moment(now) variable before I operate on now variable and then now is changed again.

[drobtravels]

+1 for immutability

I would not have expected startOf('day') to mutate (although I will admit its well documented if I had read more carefully). That caused a fun bug in my application.

[jdurand]

I definitely agree that most of moment.js operators are awkward in their mutability. mmnt.startOf('day')is super counter intuitive in that is mutates mmnt.

I use moment.js for calendar type use cases with a lot of loops and date comparisons. I have hit the performance issues with clone() and they are atrocious. Having some control over what clones and what mutates is essential to me and probably others.
Even though having clone()everywhere seems awkward at first, what it does is crystal clear and made refactoring for performance super easy for me.

If every method starts blindly using clone() internally for the sake of having a nicer API, I think we'll have missed the point.

my 2¢ :-)

[dsherret]

@jdurand I would rather explicitly mutate than explicitly clone.

[jdurand]

@dsherret I don't mind explicitly anything. My main concern is about not implicitly cloning as it is a very costly operation.
Someone mentioned that setters would need to return a cloned copy and that scared me; It would be very inefficient.

[dsherret]

@jdurand it might be more inefficient, but I think an implicit clone would benefit the majority of applications where the difference between cloned and mutated objects would make no noticeable difference to the end user's experience. I think developer ease and trying to prevent developer error should take precedence over a few saved milliseconds, as the majority of developers aren't doing thousands of date operations at once. For those who are, they could maybe explicitly say they want to mutate instead of cloning.

Sidenote: with this change I think some performance improvements could be made though... for example, cloning could be eliminated by instead referencing the past immutable object and then store the operation to be done for this object (ex. add(1, 'days')). The result would then only be computed by executing the operations when something like .toString(), .format(), .toDate(), .day(), etc... is called. That would be a dramatic change and it might not end up being faster... some unit tests would need to be done to compare the performance (additionally there could be other issues I haven't considered as I've never looked at any code in moment.js other than how it clones).

[jdurand]

@dsherret I like the builder/lazy approach; with hindsight, it should probably have been built like this from the start.
As you said, I think an immutable-ish fork with API compatibility in mind would be the best.

[d--b]

2 more cents:

1. Should we really be worried about performance, when moment is clearly built for convenience rather than performance? I would think that parsing 'year' in m.add('year',1) is much slower than cloning.
2. Unless there is a total fork (different name, different doc), it will be a pain in the ass to maintain 2 versions. I think somebody clever should come up with an idea to generate moment.immutable.min.js and moment.min.js from the same code base...

[AlexGalays]

Since when are we so afraid of breaking changes? The current version is stable and people can still use it without refactoring their code base.

Maintaining two codebases is a pain, slows you down and isn't really worth it just to have a mutable/immutable version.

So let's just go with a fully immutable moment, bump the major version and be done with it 👯

[kevinhooke]

Found this thread as I just started using this library but have been pulling my hair out debugging some code and dealing with what to me seems to be unpredictable side effects of being mutable. Would love to see a fully immutable version of the library!

[butterflyhug]

Shameless plug: I got tired of waiting for a firm decision here. Over the last few days, I've resurrected Frozen Moment and re-written it to act as a plugin for Moment itself. Hat tip to @wyantb for helping me get the first preview release done over the weekend.

Frozen Moment provides an immutable type that works just like Moment. Basically, the immutable version wraps Moment's functionality and calls .clone() for you whenever appropriate.

For those who like builder APIs, I'd challenge you to think about Moment itself as a very nice implementation of a builder object! Frozen Moment adds the core immutable type that we all want, and a mechanism for building an immutable Frozen Moment from a mutable Moment.

For those who just want to work with a convenient immutable API -- well, I intend to support that, too. I haven't built a constructor that will directly build a Frozen instance yet, but that's on the TODO list. In the short term, the workaround is to create everything with moment().freeze() or moment.utc().freeze().

Frozen Moment is obviously a young codebase, so there are probably a few rough edges -- but I'd encourage anyone here to give it a try and file issues for anything that doesn't work the way you'd expect.

Oh, one more thing: I'm not advertising this yet, but Frozen Moment instances should "just work" with most Moment plugins. Just make sure that all your other Moment plugins register themselves before Frozen Moment. If you find a plugin that doesn't work as expected with immutable Frozen Moments, file a bug and I'll look into it.

[sandstrom]

+1 on immutability

[lee101]

+1 for immutable

[wmertens]

Did anybody look at implementing moment on top of Immutable JS? It's an optimized library for immutability in JS, it reuses unchanged parts of an immutable object, reducing memory concerns.

[jbest84]

+1
I just fixed a 3 year old bug due to this: Saleslogix/argos-saleslogix@87b35b2

[AlexGalays]

So the users ask for immutability but the core team find excuses not to do it ? :p
Come on guys, this change is much more important than rewriting the code in ES6 ^^ In its current form, the API is simply bad, a bit like the JS Array one where some methods are immutable (filter, concat, etc) but some others are not (reverse, sort) except their constraint for backward compatibility is infinitely higher than for a library.

[faebser]

+1

[faebser]

a little trap that always gets me (and IMHO this is good reason for immutability):

var today = moment();
for (var i = 0; i < 7; i++) {
   week.push(today.subtract(i, 'days').format('dd').toUpperCase());
}

this sadly does not generate a array with the day names but something strange because you you actually compute the date like this:

i = 0 = today -0;
i = 1 = today -0 -1;
i = 2 = today -0 -1 -2;
etc

so you have to refactor it into this:

var today = moment();
for (var i = 0; i < 7; i++) {
            if (i == 0) {
                week.push(today.subtract(0, 'days').format('dd').toUpperCase());
            }
            else {
                week.push(today.subtract(1, 'days').format('dd').toUpperCase());
            }
        }

[SystemParadox]

@faebser excellent example

[mrcnkoba]

+1 for immutability

[webpapaya]

+1

[Wirwing]

@faebser happened to me this morning. Angular's two-way binding + mutability makes a pain in the **s to keep cloning dates to keep off modifying the current ones.

[spovich]

+1 for immutability, this just cost me a couple hours.

[rhys-vdw]

👍

[davehughes]

+1 for immutability

[thomasvanlankveld]

I'm a little torn about this topic.

The purist in me wants to shout out "+1 for immutability! Moment objects are clearly of the ValueObject kind."

However, we can't ignore that moment.js is the 13th most popular javascript repo on GitHub (and 24th all-around), and that searching for "moment" on bower.io returns 111 matched results. Even if we can provide application builders with a gradual way to implement an immutable version of moment, it's going to cause a huge mess among its dependencies.

@ichernev Perhaps a more humble suggestion: Would it be possible devote a small part of moment's documentation page to talking about mutability vs immutability? You could briefly state your official stance on the topic, and maybe add a brief summary of arguments, place a link to this thread, or if you want specific discussion input it could work like a CTA.

Right now the documentation page doesn't mention the term "immutable". Googling "moment immutable" takes you to this page, but it took me two hours reading and I'm still not sure about your current stance on the topic. It'd be great if Google's top hit for "moment immutable" gave a quick answer on the future of immutability in moment :)

To quote @jehoshua02:
"I sympathize that this is a hard decision to make. I appreciate your work. Moment.js is amazing."

[pitaj]

Here's my proposal. Instead of making the base moment immutable, add a moment.immutable factory. It would be a property of the moment function and would enclose the same exact API signature as moment, but immutable.

It could even just be a prototype extension of the mutable moment factory, using the functions of that prototype, but cloning instead.

EDIT: It appears WhoopInc/frozen-moment is exactly what I'm looking for.

[punmechanic]

@thomasvanlankveld breaking changes are what major version numbers are for. Anyone using Bower and npm will have the option of sticking to the current major version number; we should not worry about backwards compatibility here - except maybe those people who are just serving this from CDN. But if they are using momentjs from CDN they likely aren't all that interested in updating the library from time to time anyway.

I would argue that having immutability in the next major version - or the major version after that - should be on the road map.

[KrunchMuffin]

So looks like I just ran into this issue as well.

http://stackoverflow.com/questions/33002430/moment-js-formatting-incorrect-date

So I am all for immutability across the board.

[scandox]

+1 For immutability

[RobertMcCarter]

Just lost an evening to this surprising and unexpected behaviour.
Another +1 for immutability with a corresponding major version change to make it clear.

[zbarnett]

+1 For immutability

[hanbzu]

+1 for full immutability.
The collective amount of hours lost due to "sometimes mutable, sometimes immutable" must be quite big.

[magwo]

Yeah I mean seriously. Who cares about performance in a datetime library? Like.. really? I'm guessing 99.9% of users aren't doing anything that even remotely requires good performance. Typically you handle a couple of dates, or in worst case a few hundred. The few users who are handling millions of dates per second can use optimised mutable API points.

Immutability is the only sane design choice. There are several studies that show that programming with immutable types is much less bug prone than with mutable types.

[sixsided]

+1 for immutability. This cost me a couple hours.

Part of the problem is that method names like .startOf() don't imply mutation of the underlying object.

[schmod]

I've hit performance bottlenecks with Moment, so I can assure you that this can sometimes happen.

However, I'm not convinced that immutable moments would be inherently less efficient, and can envision a few cases where they would be more efficient.

This debate was settled a long time ago in the Java world. Immutable dates won, and the most popular implementation (JodaTime) eventually became part of the standard library in Java 8.

Working with Java 8's LocalTime has been one of those slap-your-forehead "Why didn't we always do this?" moments. I rarely evangelize technologies, but I honestly can't see any upsides to mutable date objects.

So, yeah.... I hate that these threads get flooded with +1s, but the truth of the matter is that somebody else is going to create an immutable JS date library if Moment doesn't.

[schmod]

I recently stumbled upon a new npm module that purports to port much of the JodaTime API (ie. Java 8 dates) to JS.

This would bring things like immutability, LocalDate, and LocalTime to node and the browser.

Having worked with these concepts in Java, everything else feels kludge-y and bug-prone.

[wmertens]

Link?

On Fri, Dec 11, 2015, 4:30 PM Andrew Schmadel notifications@github.com
wrote:

I recently stumbled upon a new npm module that purports to port much of
the JodaTime API (ie. Java 8 dates) to JS.

This would bring things like immutability, LocalDate, and LocalTime to
node and the browser.

Having worked with these concepts in Java, everything else feels kludge-y
and bug-prone.

—
Reply to this email directly or view it on GitHub
#1754 (comment).

Wout.
(typed on mobile, excuse terseness)

[mattjohnsonpint]

Since I haven't chimed in yet, I'll just state that I am in favor of immutability in moment 3.0. Primarily because I come from the DDD school of thought, where objects like moment would be considered value objects, and thus are best implemented as immutable.

Even if there's a significant performance hit, it's still the right thing to do. Moment should fit naturally into others' designs. Intuitive API trumps perf, and mutation is not intuitive on value objects (IMHO).

I also think moment 3.0 should remove its dependency on the Date object, but that's a discussion for a different thread.

[kruncher]

I completely agree with @mj1856 here.

I have been using Object.freeze on moment instances and this has generally achieved what I have needed; except I just discovered that the following fails:

let now = Object.freeze(moment());
if (now.isSameOrBefore(anotherTime)) { // throws exception
}

The exception:

TypeError: Can't add property _isValid, object is not extensible
 at valid__isValid (C:\git\quick-test\node_modules\moment\moment.js:93:24)
 at Moment.moment_valid__isValid [as isValid] (C:\git\quick-test\node_modules\moment\moment.js:2195:16)
 at Moment.isSame (C:\git\quick-test\node_modules\moment\moment.js:1945:44)
 at Moment.isSameOrBefore (C:\git\quick-test\node_modules\moment\moment.js:1962:21)

Can this be fixed so that Object.freeze can be used when desired?

[gabrielmaldi]

@wmertens I'd assume this is it: https://github.com/pithu/js-joda

[timrwood]

@ichernev, @mj1856, as I've been haven't been involved in moment core development for a while and immutability has fairly significant interest, I'm retracting my previous stance.

I'm not sure if I was the only blocker on this, but I am comfortable moving forward with immutability in 3.0.

[schmod]

@gabrielmaldi @wmertens Yep. That was it. Apologies for my borderline-incoherent comment -- I clearly clicked the 'Submit' button on a halfway-written post.

To gather up some of my disjointed thoughts:

• There is clearly some interest in immutable date objects for JS. There are mature immutable date libraries in several other languages, and there's a lot of general momentum toward immutable objects in JS (immutable.js has 10,500 stars if that's any indication). At the very least, I think this deserves a proof-of-concept.
• In spite of this interest, very little code appears to have been written. js-joda appears to be the first serious attempt to write an immutable date library for JS.
• Immutable moments would be a huge breaking change, which raises some philosophical questions. While I would hate to lose the support of the very large MomentJS community, it wouldn't necessarily be a horrible thing for those of us who are interested in immutable JS dates to make a clean break, and contribute to js-joda instead of trying to push a fairly radical change on Moment (a mature library with a large and established user-base).
• Aside: js-joda is still very young, and it's unclear what the author's goals and intentions are for the library. In particular, he needs to pick a license, and we may want to consider whether the needs of the typical JS developer would be met by a faithful re-implementation of Joda Time or JSR-310.

[tacomanator]

+1 for an immutability, and the saner code that will result.

It will be a major effort, so a sincerest thank you to those who'll be (and have been) making it happen.

[wyantb]

Moment is widely used enough that I think it'd be actionable to go with something approaching the following, assuming the is implemented similarly to @butterflyhug's https://github.com/WhoopInc/frozen-moment

3.x: immutable as an option, defaulted to false, and have a flag set on the global moment export that would set to true; console.warn on library load (in dev mode)
4.x: immutable as an option, defaulted to true, flag still available to be set as false; console.warn about schedule for 5.x (in dev mode)
5.x: immutable as the only way

Have a page up-front and center describing the long-term immutability vision - say, one major release a year along the 3.x/4.x/5.x outline I laid out - and I think it'd give a reasonable amount for anyone affected to update their code.

[butterflyhug]

A few observations:

1. I built WhoopInc/frozen-moment hoping that a bunch of folks here might be willing to build things with it, as a way to find problem points in the moment ecosystem where immutability would break plugins, etc. So far, few people have done that, which has made me less eager to work on frozen-moment and/or community evangelism to help plugin authors support immutable moments.

   That said, I'm still willing to help fix bugs that people find when using frozen-moment -- regardless of whether those bugs are mine or whether they're in another moment plugin that never considered the possibility that moments might become immutable. Just file issues at frozen-moment and I'll take a look. And I still think that a community effort around something like frozen-moment could help us understand and ease the pain of a transition to immutable moments.

2. If moment implements immutability in 3.0, it would be pretty straightforward for someone to write and maintain a wrapper to implement the mutable moment 2.x API on top of the immutable 3.x API. Conceptually, this would actually look a lot like frozen-moment: everywhere that frozen-moment implicitly clone()s, this mutability wrapper would instead implicitly change its internal reference to a new immutable moment value. This could also help ease the transition for folks using moment in large codebases.

3. I would be willing to help think through potential issues and implement immutability in moment 3, if desired.

4. js-joda will be directly porting JSR-310 into JavaScript using a BSD license.

[RobertMcCarter]

@butterflyhug - Thanks for building frozen-moment! I was excited when I found that, but worried about introducing a dependency from my project to frozen-moment, given that if support was dropped changing from an immutable moment to a mutable one would be a big job in my code. If your goal was to get feedback and you're actively supporting it then I'm more comfortable doing that. :-)

I don't know how many other people might have had the same thought processes.

[schmod]

I'm curious if folks here think that immutability should be explored in conjunction with the notion of breaking the dependency on the Date object (or even adding in some sort of lazy evaluation) for performance reasons.

[punmechanic]

I would suggest taking things one step at a time, @schmod. This is already a (apparently) big change

[butterflyhug]

@RobertMcCarter Yep, I am planning to support frozen-moment for the foreseeable future, unless/until some option for immutability gets implemented in the core library -- in large part because I'm personally using it for some things that I expect to be maintaining for a while. That said, my use cases don't span the full Moment ecosystem, so I rely on feedback and usage from other folks to help identify what's important.