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

Decouple 262 IsValidTimeZone from 402 IsValidTimeZone #1996

Open
ptomato opened this issue Jan 10, 2022 · 8 comments
Open

Decouple 262 IsValidTimeZone from 402 IsValidTimeZone #1996

ptomato opened this issue Jan 10, 2022 · 8 comments
Labels
ecma402 Behavior specific to implementations supporting ecma402
Milestone

Comments

@ptomato
Copy link
Collaborator

ptomato commented Jan 10, 2022

In the Temporal champions meeting of 2022-01-06 we discussed decoupling the IsValidTimeZone abstract operation in the ECMA-262 part of Temporal from the one already existing in ECMA-402, and making sure that an exception is thrown when passing a valid time zone name from the 262 domain into the 402 domain if it is not valid there (e.g., in Intl.DateTimeFormat).

This is because implementations without Intl may choose to support non-IANA time zones, while 402-compliant implementations must not support any non-IANA time zones. To the best of our knowledge, if an implementation decides to adopt 402, that is a fully backwards-compatible change. IsValidTimeZone would be the only place where conforming to 402 would potentially break existing code. We believe we should avoid this.

Additionally, we should add a note discouraging implementations from supporting non-IANA time zones. Although we didn't reach consensus on forbidding it, we all agreed that such divergence would be undesirable.

cc @gibson042

@FrankYFTang
Copy link
Contributor

@FrankYFTang

@ptomato
Copy link
Collaborator Author

ptomato commented Oct 27, 2022

From the Temporal champions meeting of 2022-10-27: we think the way this should be resolved is by completing tc39/ecma402#683 so that ECMA-402 doesn't have to override ECMA-262's DefaultTimeZone. Once that happens, there is no need to build a "firewall" between 262 time zones and 402 time zones. We'll keep this issue open to reassess the situation once ECMA-402 adapts, in case we need to add some assertions to Temporal's spec text, but expect that this issue will probably become a no-op.

@ptomato ptomato removed spec-text Specification text involved needs plenary input Needs to be presented to the committee and feedback incorporated normative Would be a normative change to the proposal labels Oct 27, 2022
@ptomato ptomato removed this from the Next batch of normative changes milestone Oct 27, 2022
@ptomato
Copy link
Collaborator Author

ptomato commented Dec 8, 2022

Verify before closing.

@ptomato ptomato added this to the Stage "3.5" milestone Dec 8, 2022
@ptomato
Copy link
Collaborator Author

ptomato commented Jan 11, 2023

As per the decision in the champions meeting of 2022-10-27, this is blocked on tc39/ecma402#683 being addressed. However, since this is a change in ECMA-402 that is independent of Temporal, I don't think any significant change would actually be necessary on the Temporal side. We would just remove the override for AvailableTimeZones in the ECMA-402 section.

I'm fine to keep this open in a later milestone to track that, although I'd also be fine with closing the issue.

@ptomato ptomato modified the milestones: Stage "3.5", Stage 4 Jan 11, 2023
@ptomato
Copy link
Collaborator Author

ptomato commented Mar 29, 2024

@gibson042 @justingrant You're probably more familiar than I am about the current state of time zone canonicalization in ECMA-402.

It's clear to me that we need to do the following before landing Temporal, simply to bring 402 and 262 into alignment:

What's not clear to me is whether we still need to have a 402-overridden version of 262's AvailableNamedTimeZoneIdentifiers. Do we need it to contain what is currently steps 2 and 3 of CanonicalizeTimeZoneName (the steps about the backward file and the hardcoded aliases of UTC)? Or is the intention that those should disappear?

After clearing that up, we can 'rebase' Temporal and close this issue.

I'll put this on the agenda for next week, but if we can clear it up asynchronously in this thread, that'd be even better.

@justingrant
Copy link
Collaborator

Very close! Although I never got around to filing it as a 402 PR, last year as part of the canonnical-tz proposal I built a branch (https://github.com/tc39/ecma402/compare/main...justingrant:refactor-time-zone-text?expand=1) for updating ECMA-402 to match the new ECMA-262 AOs and spec text. The specific changes I recommended are in the commit message of that branch's single commit.

The content in that branch still looks pretty good, although some of the prose (and perhaps the algorithm steps in AvailableNamedTimeZoneIdentifiers) may need to be updated to match the discussion in tc39/ecma402#825.

Anyway, I just rebased that branch to the latest 402 main.

I'm not sure I'll have time to shepherd this on my own through TG2, but if someone wants to partner with me on it then that'd be great.

I think we do need 402 to override AvailableNamedTimeZoneIdentifiers, because 402 is more specific about adopting the IANA TZDB while 262 is more generic.

But happy to discuss some other solution that puts the implementation-dependent stuff into a different AO and makes AvailableNamedTimeZoneIdentifiers into an implementation-independent caller of that implementation-dependent AO.. Honestly I'm not sure that juice is worth the squeeze though, given how hard it was to get AvailableNamedTimeZoneIdentifiers into 262 in the first place. It's probably easiest to just let 402 continue to override it.

@justingrant
Copy link
Collaborator

Actually, I went ahead and turned this branch into an editorial PR for 402: tc39/ecma402#876

Reviews welcome!

@justingrant
Copy link
Collaborator

And I just figured I'd go the extra mile and put up a 402 normative PR tc39/ecma402#877, stacked on the editorial PR above, for the long-term alignment of ID handling across browsers. @gibson042 you'll probably care about this.

@ptomato I added you as as reviewer but I suspect you may care less... except about how it's tested where I could definitely use your advice and help because I don't know how we can run those tests without the Temporal polyfill.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ecma402 Behavior specific to implementations supporting ecma402
Projects
None yet
Development

No branches or pull requests

3 participants