Normative: Revert U+2212 (Unicode minus sign) as a valid character in timezone offsets #3334
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
justingrant
added a commit
to justingrant/proposal-temporal
that referenced
this pull request
May 23, 2024
This PR is the Temporal counterpart to tc39/ecma262#3334. See that PR for details. This PR is dependent on that PR's approval.
michaelficarra
added
normative change
|
FWIW as far as I understand it, this change would not be observable if made in ECMA-262 today, without Temporal. (But it should probably be discussed in plenary anyway.) |
|
Clarifying @ptomato's comment above: this U+2212 character is currently only exposed via Temporal, because the only way an offset can get into So the current plan is to present a normative Temporal PR in Helsinki to remove U+2212 from the Temporal grammar, and mention as part of this discussion that consensus on that PR also means percolating this grammar change back to 262. Sound OK? |
|
It's normative for ECMA-402, where |
concrete example: const dtf = new Intl.DateTimeFormat("pt", {
dateStyle: "short",
timeStyle: "full",
timeZone: "\u{2212}01:00",
});
dtf.format(new Date("2024-01-01T00:00Z"))
// => "31/12/2023, 23:00:00 GMT-01:00" |
|
This normative change reached consensus at the TC39 meeting of 2024-06-12. |
ptomato
added
has consensus
91 tasks
ptomato
pushed a commit
to justingrant/proposal-temporal
that referenced
this pull request
Jun 27, 2024
This PR is the Temporal counterpart to tc39/ecma262#3334. See that PR for details. This PR is dependent on that PR's approval.
ptomato
added a commit
to ptomato/test262
that referenced
this pull request
Jun 28, 2024
… IDs These are the test adjustments corresponding to the normative PR tc39/ecma262#3334 which reached consensus at the June 2024 TC39 meeting.
ptomato
added a commit
to ptomato/test262
that referenced
this pull request
Jun 28, 2024
These are the test adjustments corresponding to the normative PR tc39/ecma262#3334 which reached consensus at the June 2024 TC39 meeting.
|
Tests are in tc39/test262#4120. (The first commit specifically contains tests for this PR. The second commit for the companion Temporal PR.) |
ptomato
added
has test262 tests
and removed
needs test262 tests
This comment was marked as spam.
This comment was marked as spam.
michaelficarra
approved these changes
Jul 2, 2024
justingrant
force-pushed
the
remove-unicode-minus
branch
from
c44edd4
to
7c3ae1d
Compare
|
I rebased to latest main. Hopefully ready to merge? |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
We typically require 2 editor approvals for anything that's not completely trivial. |
ptomato
added a commit
to ptomato/test262
that referenced
this pull request
Jul 3, 2024
… IDs These are the test adjustments corresponding to the normative PR tc39/ecma262#3334 which reached consensus at the June 2024 TC39 meeting.
ptomato
added a commit
to ptomato/test262
that referenced
this pull request
Jul 3, 2024
These are the test adjustments corresponding to the normative PR tc39/ecma262#3334 which reached consensus at the June 2024 TC39 meeting.
ptomato
added a commit
to tc39/test262
that referenced
this pull request
Jul 3, 2024
… IDs These are the test adjustments corresponding to the normative PR tc39/ecma262#3334 which reached consensus at the June 2024 TC39 meeting.
ptomato
added a commit
to tc39/test262
that referenced
this pull request
Jul 3, 2024
These are the test adjustments corresponding to the normative PR tc39/ecma262#3334 which reached consensus at the June 2024 TC39 meeting.
ptomato
pushed a commit
to tc39/proposal-temporal
that referenced
this pull request
Jul 4, 2024
This PR is the Temporal counterpart to tc39/ecma262#3334. See that PR for details. This PR is dependent on that PR's approval.
ptomato
pushed a commit
to tc39/proposal-temporal
that referenced
this pull request
Jul 4, 2024
This PR is the Temporal counterpart to tc39/ecma262#3334. See that PR for details. This PR is dependent on that PR's approval.
This comment was marked as spam.
This comment was marked as spam.
moz-v2v-gh
pushed a commit
to mozilla/gecko-dev
that referenced
this pull request
Jul 16, 2024
…ffsets. r=spidermonkey-reviewers,mgaudet Also simplify the implementation to use normal string comparison instead of using character codes. Changes from: tc39/ecma262#3334 Differential Revision: https://phabricator.services.mozilla.com/D216275
syg
approved these changes
Jul 17, 2024
michaelficarra
added
the
ready to merge
Following ISO-8601, tc39#2781 introduced U+2212 (Unicode minus) as an alias for the regular ASCII minus sign for use in time zone offsets. There's two new data that lead me to believe that this was a mistake, and that we should revert this change. The first is that the newly-released RFC 9557 (the string format standard that Temporal uses) disallows non-ASCII characters. Its predecessor RFC 3339 also disallows non-ASCII characters. So strings that follow the current (since 2022) ECMAScript spec could be rejected by RFC 9557 clients. The second new data is feedback from implementers of a Rust version of Temporal that this single obscure character in the grammar will incur a performance cost because they must now use Rust strings instead of plain U8 ASCII data. See tc39/proposal-temporal#2843 (comment) This performance issue doesn't seem to be limited to Rust. Any native implementation would likely benefit from being able to know that valid date/time input (both Date and Temporal) is always ASCII-only. I don't know whether all engines have actually implemented this 2022 grammar change. But it's also a safe bet that real-world usage of this Unicode character is likely minimal. So the web-compat risk seems small. If this PR is accepted, then we'll follow up with a normative Temporal PR to remove this character from Temporal as well.
ljharb
force-pushed
the
remove-unicode-minus
branch
from
7c3ae1d
to
e7f1e5d
Compare
ErichDonGubler
pushed a commit
to erichdongubler-mozilla/firefox
that referenced
this pull request
Jul 18, 2024
…ffsets. r=spidermonkey-reviewers,mgaudet Also simplify the implementation to use normal string comparison instead of using character codes. Changes from: tc39/ecma262#3334 Differential Revision: https://phabricator.services.mozilla.com/D216275
i3roly
pushed a commit
to i3roly/firefox-dynasty
that referenced
this pull request
Jul 18, 2024
…ffsets. r=spidermonkey-reviewers,mgaudet Also simplify the implementation to use normal string comparison instead of using character codes. Changes from: tc39/ecma262#3334 Differential Revision: https://phabricator.services.mozilla.com/D216275
bakkot
added a commit
to bakkot/proposal-temporal
that referenced
this pull request
Sep 5, 2024
ptomato
pushed a commit
to tc39/proposal-temporal
that referenced
this pull request
Sep 6, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Following ISO-8601, #2781 introduced U+2212 (Unicode minus) as an alias for the regular ASCII minus sign for use in time zone offsets.
There's two new data that lead me to believe that this was a mistake, and that we should revert this change.
The first is that the newly-released RFC 9557 (the string format standard that Temporal uses) disallows non-ASCII characters. Its predecessor RFC 3339 also disallows non-ASCII characters. So strings that follow the current (since 2022) ECMAScript spec could be rejected by RFC 9557 clients.
The second new data is feedback from implementers of a Rust version of Temporal that this single obscure character in the grammar will incur a performance cost because they must now use Rust strings instead of plain U8 ASCII data. See
tc39/proposal-temporal#2843 (comment)
This performance issue doesn't seem to be limited to Rust. Any native implementation would likely benefit from being able to know that valid date/time input (both
Dateand Temporal) is always ASCII-only.I don't know whether all engines have actually implemented this 2022 grammar change. But it's also a safe bet that real-world usage of this Unicode character is likely minimal. So the web-compat risk seems small.
If this PR is accepted, then I'll follow up with a normative Temporal PR to remove this character from Temporal as well.
FYI @ptomato @gibson042.