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

CreateTemporalZonedDateTime: Wrong argument type for "timeZone" #3055

Open
anba opened this issue Dec 9, 2024 · 8 comments
Open

CreateTemporalZonedDateTime: Wrong argument type for "timeZone" #3055

anba opened this issue Dec 9, 2024 · 8 comments
Assignees
Labels
editorial spec-text Specification text involved

Comments

@anba
Copy link
Contributor

anba commented Dec 9, 2024

CreateTemporalZonedDateTime:

timeZone (an available time zone identifier)

But timeZone can also be an offset time zone, so it's type should instead be "time zone identifiers".

@ptomato
Copy link
Collaborator

ptomato commented Dec 10, 2024

This seems correct as-is to me:

An available time zone identifier is either an available named time zone identifier or an offset time zone identifier.

Feel free to reopen if I overlooked something.

@ptomato ptomato closed this as not planned Won't fix, can't repro, duplicate, stale Dec 10, 2024
@justingrant justingrant reopened this Dec 13, 2024
@justingrant
Copy link
Collaborator

Reopening. How does "available time zone identifier" differ from the existing term "time zone identifier" defined in current ECMA-262's time zone identifiers section? Based on my intent when I wrote that ECMA-262 text, it seeems like "available time zone identifier" is redundant and could be removed. But perhaps my 262 text was unclear, or you had something else in mind?

OTOH, there may (or may not) be a need for a new term for a time zone identifier in its normalized form, i.e. the result of ToTemporalTimeZoneIdentifier.

BTW, here's my mental map of the terms defined in ECMA-262, using bullet nesting to imply "IS A" relationships:

  • time zone identifier
    • available named time zone identifier
      • primary time zone identifier
      • non-primary time zone identifier
    • offset time zone identifier

@anba
Copy link
Contributor Author

anba commented Dec 13, 2024

Feel free to reopen if I overlooked something.

I've incorrectly treated available named time zones and available time zone identifier as the same type.

OTOH, there may (or may not) be a need for a new term for a time zone identifier in its normalized form, i.e. the result of ToTemporalTimeZoneIdentifier.

Is this covered by normalized format of a time zone identifier (defined in https://tc39.es/proposal-temporal/#sec-time-zone-identifiers)?

Temporal adds available time zone identifier as another subtype, so the relationships are now:

  • time zone identifier
    • available time zone identifier
      • available named time zone identifier
        • primary time zone identifier
        • non-primary time zone identifier
      • offset time zone identifier

And for the actual time zones:

  • time zone
    • available named time zone
    • offset time zone

@justingrant
Copy link
Collaborator

Temporal adds available time zone identifier as another subtype,

I don't understand (yet!) what is the delta between available time zone identifier and time zone identifier.

@ptomato could you clarify?

@ptomato
Copy link
Collaborator

ptomato commented Jan 13, 2025

Here is the difference as far as I understand it from https://tc39.es/proposal-temporal/#sec-time-zone-identifiers:

  • time zone identifier: string composed entirely of code units in the inclusive interval from 0x0021 to 0x007E.
  • available time zone identifier: either an available named time zone identifier or an offset time zone identifier

So all available time zone identifiers are time zone identifiers, but not vice versa. Examples of time zone identifiers that are not available time zone identifiers are:

  • Etc/North_Pole
  • +99:99
  • !!!d3l3t3m3~~~

@justingrant
Copy link
Collaborator

time zone identifiers that are not available time zone identifiers

Do we really need this category, of strings that are syntactically valid but are not valid named or offset IDs? Why is that needed?

@ptomato
Copy link
Collaborator

ptomato commented Jan 14, 2025

I did a census of the places where it's used:

  1. the input type of GetAvailableNamedTimeZoneIdentifier
  2. the type of the [[Name]] field of the returned record from ParseTimeZoneIdentifier and ParseTemporalTimeZoneString
  3. the type of the [[TimeZone]] field of Calendar Fields Record
  4. in prose in 15.2 Use of the IANA Time Zone Database (2x)
  5. in prose in the note after AvailableNamedTimeZoneIdentifiers
  6. the type of the [[TimeZone]] slot of Intl.DateTimeFormat

Of these, I think (3), (4), and (6) are probably mistakes and should be available time zone identifier. The others seem correct and necessary to me, so I'm not sure we can get rid of it entirely.

@ptomato ptomato self-assigned this Jan 14, 2025
@ptomato ptomato added spec-text Specification text involved editorial labels Jan 14, 2025
ptomato added a commit that referenced this issue Jan 14, 2025
In a few places, we used "time zone identifier" (any string consisting of
allowed code units) where "available time zone identifier" would be
correct. Use that instead.

See: #3055
ptomato added a commit that referenced this issue Jan 14, 2025
In a few places, we used "time zone identifier" (any string consisting of
allowed code units) where "available time zone identifier" would be
correct. Use that instead.

See: #3055
ptomato added a commit that referenced this issue Jan 15, 2025
In a few places, we used "time zone identifier" (any string consisting of
allowed code units) where "available time zone identifier" would be
correct. Use that instead.

See: #3055
ptomato added a commit that referenced this issue Jan 15, 2025
In a few places, we used "time zone identifier" (any string consisting of
allowed code units) where "available time zone identifier" would be
correct. Use that instead.

See: #3055
@ptomato
Copy link
Collaborator

ptomato commented Jan 15, 2025

I fixed the cases that I believe are mistakes. The remaining uses of time zone identifier are used to denote parse results before validation, or in case (5) above to talk about which strings are or aren't in the TZDB. So I think this is OK to close now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial spec-text Specification text involved
Projects
None yet
Development

No branches or pull requests

3 participants