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

Use Intl API for guessing when available #291

Merged
merged 2 commits into from
Mar 1, 2016

Conversation

mattjohnsonpint
Copy link
Contributor

Since the ECMA-402 Internationalization API provides a mechanism to get the current time zone, we should use it when available and fully implemented.

Note that we also need to guard against a few bad android implementations that return non-IANA time zone IDs. (jstzdetect ran into this)

This will address #290

@maxnikulin
Copy link

Perhaps an additional sanity check is required. Moment-timezone may have newer timezone info than the system due to the device vendor does not provide updates. That is why another timezone may be set by the user to get correct time offset.

var mLocal = moment();
var mGuess = mLocal.clone().tz(zone);
if (mLocal.hours() === mGuess.hours() && mLocal.minutes() === mGuess.minutes()) {
   return zone;
}

Also I would suggest to add try-catch.

@maxnikulin
Copy link

Sorry, just realized that it is more complicated. After moment.tz.setDefault(...) my suggestion will not work.

// use Intl API when available and returning valid time zone
if (typeof Intl === 'object' && isFunction(Intl.DateTimeFormat)) {
var zone = Intl.DateTimeFormat().resolvedOptions().timeZone;
if (zone && (zone.indexOf("/") > -1 || zone === 'UTC')) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about something like if (typeof zone === 'string' && names[normalizeName(zone)]) { to make sure a identifier returned from Intl actually has data loaded? Maybe we could add a logError if Intl returns an identifier that was not loaded in the data set?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I like that idea. I'll work on that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I continued this work in #304

@timrwood timrwood merged commit 2d0d3df into moment:develop Mar 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants