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

TimeZone.from and ISO "Z" strings #1075

Closed
justingrant opened this issue Oct 29, 2020 · 2 comments
Closed

TimeZone.from and ISO "Z" strings #1075

justingrant opened this issue Oct 29, 2020 · 2 comments

Comments

@justingrant
Copy link
Collaborator

justingrant commented Oct 29, 2020

In #313, it was decided that ISO Z strings would be accepted by TimeZone.from. We may want to revisit this, for the same reason that ZonedDateTime.from doesn't accept bracketless ISO strings: because it makes it too easy for developers to perform calendar operations or ZDT math in the UTC time zone. Instead developers should should be using the Temporal.Instant type and not trying to pretend that their Instant is really a ZonedDateTime.

If developers find themselves wanting to operate on months or to add calendar days or months, then they need a real time zone! (Or if they really want UTC, they should opt in via 'UTC').

Example:

Temporal.TimeZone.from('1976-11-18T14:23:30.123Z')
// => UTC

s = '1976-11-18T14:23:30.123Z';
zdt = Temporal.Instant.from(s).toZonedDateTimeISO(s); // USUALLY A BUG
zdt.with({month: 3}); // USUALLY A BUG
zdt.add({months: 1, days: 10}); // USUALLY A BUG

Anyway, my suggestion would be to stop accepting ISO "Z" strings in TimeZone.from.

@ptomato
Copy link
Collaborator

ptomato commented Oct 29, 2020

Meeting, Oct 29: The above code is not necessarily a bug but may occur in upgrade paths. We'll keep TimeZone.from as is, where accepting an ISO string with Z is consistent with accepting an ISO string with e.g. +06:00.

@ptomato ptomato closed this as completed Oct 29, 2020
@justingrant
Copy link
Collaborator Author

After the meeting, @ptomato and I agreed on the following fallback position:

  1. For Stage 3 review, ZDT.from with "Z" strings should throw in all cases, regardless of the offset option. This ensures that however we decide to handle Z strings (including to leave them permanently disallowed), it will be a non-breaking change.
  2. We won't block Stage 3 review on this. If we want to change behavior later in response to feedback or more discussion, we can do this, and since it's a non-breaking change it won't hurt any working use cases.

In other words, the rationale throwing from "Z" strings with a bracket suffix in ZDT.from should be that "Z" strings simply aren't accepted, rather than because they conflict with the IANA bracketed zone.

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

2 participants