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

"Etc/UTC" is treated differently than 'UTC'. #1358

Open
jsumners opened this issue Jan 7, 2023 · 18 comments
Open

"Etc/UTC" is treated differently than 'UTC'. #1358

jsumners opened this issue Jan 7, 2023 · 18 comments

Comments

@jsumners
Copy link

jsumners commented Jan 7, 2023

Describe the bug
Consider (runkit playground -- https://runkit.com/63b9e8148ba93c0008d85c98/63b9e81423d01f0008ee1a6d):

const luxon = require('luxon')
const dt = luxon.DateTime.fromISO('2022-01-07T16:00:00-05:00')

console.log(dt.toISO())
console.log(dt.setZone('Etc/UTC').toISO())
console.log(dt.setZone('UTC').toISO())

The output will be:

2022-01-07T16:00:00.000-05:00
2022-01-07T21:00:00.000+00:00
2022-01-07T21:00:00.000Z

It seems reasonable to expect that both representations of the UTC offset would result in the same string. That is, console.log(dt.setZone('Etc/UTC').toISO()) should output 2022-01-07T21:00:00.000Z.

The issue seems to stem from the following line:

else if (lowered === "utc" || lowered === "gmt") return FixedOffsetZone.utcInstance;

Desktop (please complete the following information):

  • Luxon version: 3.2.1
@dobon
Copy link
Contributor

dobon commented Jan 7, 2023

Great catch, thank you very much for the report. I've reviewed the IETF documentation and agree that this is a Luxon bug (and also I agree with your analysis about the root cause). 'Etc/UTC' is supposed to be exactly identical to 'UTC', so simply adding || lowered == 'etc/utc' to the if statement you identified should resolve the bug with minimal surgery.

I'll publish a PR with the above described patch and a matching unit test tonight, probably, and I'll look around for any other related bugs while I'm at it.

@icambron
Copy link
Member

icambron commented Jan 9, 2023

@dobon yeah, I think I'd take that. It would also help perf on that zone, because we wouldn't have to ask Intl to compute offsets for us. etc/gmt may be a thing too.

@diesieben07
Copy link
Collaborator

Unfortunately this stuff seems highly inconsistent... According to the tz database list on Wikipedia, there are tons of names for UTC. First there are the obvious ones, like UTC, Etc/UTC. But then there is also "Etc/Greenwich" or "Greenwich", which is supposed to be equivalent. V8 does not treat them all the same. "Etc/Greenwich" produces "Greenwich Mean Time", but "Etc/GMT" produces "Coordinated Universal Time".
We could add a huge list of them (or build a complex regex), but the question is how useful this is.

@jsumners
Copy link
Author

jsumners commented Mar 7, 2023

I don't think it's that complicated. Ignoring that GMT and UTC are technically different, as the original post outlines, anything that is supposed to map to UTC should behave the same as if the UTC offset were directly used.

const utcAliases = new Set([
  'UTC',
  'Etc/UTC // keep going
]) 

if (utcAliases.has(something) === true) {
  // do stuff
}

@diesieben07
Copy link
Collaborator

Yes, that part is trivial. But where do we draw the line? Currently, Luxon already treats GMT the same as UTC. Should we also include Greenwich in that list then? Chrome/V8 doesn't treat them the same (but it also doesn't treat GMT and Greenwich the same, so there is that...).

My gut says go with the most commonly used options, i.e. UTC, GMT and their Etc/ prefixed versions. The rest will still work absolutely correctly, it just won't use the more performant FixedZone implementation.

@jsumners
Copy link
Author

jsumners commented Mar 7, 2023

I wouldn't base anything on what a browser does or doesn't do. I'd follow the already linked spec https://www.ietf.org/timezones/data/etcetera

@diesieben07
Copy link
Collaborator

What about https://www.ietf.org/timezones/data/backward then? It lists many more aliases for UTC. Should all of these be checked as well? That might get into the territory of being significant for bundle size.

@jsumners
Copy link
Author

jsumners commented Mar 7, 2023

I'm not sure I can offer an opinion on that as I am not familiar with Luxon's commitment level for support of the tzdata spec and such backward compatibility. It would make sense to me to include some sort of hash that prevents issues such as this one in the future, though. I filed this issue because a contributor was trying to update some code to replace moment with luxon and they were tripped up by the usage of Etc/UTC and the difference in .toISO.

@diesieben07
Copy link
Collaborator

toISO still behaves correctly in either case. Both +00:00 and Z are valid outputs. So I do not think it is worth it to include the whole list. Luxon's philosophy is to rely on the Intl API for support.

@jsumners
Copy link
Author

jsumners commented Mar 8, 2023

Yes, they are both accurate. The issue is that they are inconsistent for the same "zone".

@diesieben07
Copy link
Collaborator

What issues does that cause, though? In terms of ISO format, Z is simply a shortcut for +00:00. I would argue that trying to cover all edge-cases here is not worth the effort, maintenance burden and bundle size.
If someone has a way to make the Intl API to expose this information, that would be great.

@jsumners
Copy link
Author

jsumners commented Mar 8, 2023

I think the original report shows what issues it causes.

@diesieben07
Copy link
Collaborator

There are no problems listed in the original report, only facts are stated and your desire for the output to be different.

@icambron
Copy link
Member

icambron commented Mar 9, 2023

@diesieben07 we generally think of Z as implying UTC. Compare to, say, London time when the offset happens to be 0. So when possible we try to format UTC dates with a Z in order to convey that extra piece of information about the zone. In practice there are limits to what we can reasonably do, of course.

@diesieben07
Copy link
Collaborator

Shouldn't that then mean that gmt should be removed from that list as well, because gmt means "Greenwich Mean Time" and not "UTC"?

@icambron
Copy link
Member

I think of GMT as an (outdated) alias for UTC as well. I'm certainly open to being wrong about this, but I don't see a meaningful difference between them in theory or in usage.

@diesieben07
Copy link
Collaborator

diesieben07 commented Mar 10, 2023

It's not though. GMT was used as the basis of time around the world just like UTC is today, but it uses a totally different source of time than UTC does (astronomy vs an atomic clock).
For most intents and purposes they really are the same, but in GMT seconds are not always the same length where as in UTC we have leap seconds to account for earth's uneven rotation.
In my opinion, only the "real" UTC should output Z, where as GMT should really use 00:00. Unfortunately removing the check for gmt here will not lead to that behavior, because browsers (I checked Chrome and Firefox) also treat gmt as Coordinated Universal Time (aka. UTC).

Edit: So, after some more reading and thinking, my opinion is that we should change this check to check for utc and etc/utc only. There are other aliases for UTC (like Zulu), but I don't know how widely they are used.
Then we can "blame" browsers for treating "gmt" as UTC and it is no longer something we can fix.

@Paul-Bob
Copy link

Hello, noticed that this issue still persisting and we kinda have a real problem with it on Avo.

We're using Flatpickr and we want to allow our users to force a timezone to a date & time field.

Lets say the user is on CET timezone and he want to force UTC, when using UTC the offset result outputs Z and Flatpickr automatically changes it to the local timezone (CET in this example). When using Etc/UTC the offset output is +00:00 and Flatpickr keeps the time without converting it to local which is the desired behaviour.

"Z" expresses UTC itself, while "+00"/"+0000"/"+00:00" expresses a local time with UTC offset of zero.

How can I convert a date to UTC and get the offset output as +00:00?

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

No branches or pull requests

5 participants