search

urish / angular-moment

Moment.JS directives for Angular.JS (timeago and more)
MIT License
2.6k stars 397 forks source link

clone moment before applying timezone in amTimezone #213

Closed jarcoal closed 7 years ago

jarcoal commented 8 years ago

Currently the amTimezone filter mutates the moment object before returning it. This creates all sorts of subtle bugs if that moment is used later.

This patch clones the moment before applying the timezone to prevent this from happening.

urish commented 8 years ago

Great catch, thanks!

Can you please also add a unit test that fails without this change to make sure we don't introduce any regressions in the future?

jarcoal commented 8 years ago

Looks like the default preprocessor actually does a clone here.

Do you think this is still a worthwhile patch given that developers with custom preprocessors might hit the issue?

urish commented 8 years ago

Did you verify the calling moment() on that line actually returns a new object?

I believe that if the preprocessDate() method already takes care of cloning the object, we don't need to clone it again in the other preprocessors. Thoughts?