search

urish / angular-moment

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

Update amMoment.changeLocale() #273

Open geroyche opened 7 years ago

geroyche commented 7 years ago

as per the current moment.js docs, moment.locale() is deprecated (and produces warnings in the console). it should be replaced with updateLocale()

https://github.com/moment/moment/issues/3043#issuecomment-197017584

urish commented 7 years ago

It seems like only the second parameter to locale was deprecated (i.e. customizing strings). Can you please send a PR that will call updateLocale if the second parameter is provided?

Thanks!

geroyche commented 7 years ago

not only so. it's also about whether the locale already exists or not.

.locale(locale) to simply load an existing locale .locale(locale, opt) to load a newly created one .updateLocale(locale, opt) to load an existing locale and customize

maybe it would be best to additionally provide .locale() and updateLocale() to your directive? that way existing code will continue to work, and in new code users can decide what to call, based on their knowledge about whether a locale exists already or not.

urish commented 7 years ago

Yes, that makes sense. I think that the best solution would be to deprecate the second parameter to changeLocale. If you want to define a new locale, or update an existing one - just go directly to moment API.