Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix locale fa loading based on moment documentation. #134

Merged
merged 2 commits into from
May 9, 2018

Conversation

VahidBo
Copy link
Contributor

@VahidBo VahidBo commented Apr 22, 2018

Motivation

When you use the moment.loadPersian() function you will face an error that says the fa locale is not loaded.
Based on this document of moment this is a bug that prevents moment.locale from being loaded. And recommended solution is using this workaround:

var moment = require('moment');
require('moment/locale/cs');
console.log(moment.locale()); // cs

I applied this to solve the problem. And this PR also fix the #131.

And we want to change the fa locale using the moment.defineLocale function but we should use the moment.updateLocale() to change an existing locale. moment.defineLocale() should only be used for creating a new locale based on this.

@alitaheri
Copy link
Member

This looks good 👍 But I'm not very familiar with the inner workings of moment.

@behrang This one needs your attention.

@behrang
Copy link
Member

behrang commented May 9, 2018

Sorry, I was very busy, and didn't find a time to look into this.

@VahidBo VahidBo deleted the fixLocaleLoading branch May 9, 2018 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants