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

Normative: Fix fractionalSecondDigits behaviour in Duration.toString() #1956

Merged
merged 1 commit into from
Dec 17, 2021

Conversation

ptomato
Copy link
Collaborator

@ptomato ptomato commented Dec 3, 2021

Previously, code that combined zero subsecond units with a specified
precision, such as the following:

Temporal.Duration.from({ years: 3 }).toString({ fractionalSecondDigits: 5 })

would not render the seconds with the correct precision. The above example
would return 'P3Y' and not 'P3YT0.00000S'.

Closes: #1763

@ptomato ptomato added spec-text Specification text involved needs plenary input Needs to be presented to the committee and feedback incorporated labels Dec 3, 2021
@ptomato ptomato marked this pull request as draft December 3, 2021 22:31
@ptomato
Copy link
Collaborator Author

ptomato commented Dec 3, 2021

Draft until presented at TC39.

cc @rkirsling

@codecov
Copy link

codecov bot commented Dec 3, 2021

Codecov Report

Merging #1956 (228f183) into main (8b7ba00) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1956   +/-   ##
=======================================
  Coverage   94.96%   94.96%           
=======================================
  Files          19       19           
  Lines       10967    10967           
  Branches     1743     1743           
=======================================
  Hits        10415    10415           
  Misses        535      535           
  Partials       17       17           
Flag Coverage Δ
test262 79.29% <100.00%> (ø)
tests 89.86% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
polyfill/lib/ecmascript.mjs 95.25% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8b7ba00...228f183. Read the comment docs.

@ptomato ptomato force-pushed the 1763-duration-fractionalseconddigits branch from 0cf44aa to 1690ed4 Compare December 3, 2021 22:32
@ptomato
Copy link
Collaborator Author

ptomato commented Dec 17, 2021

This achieved consensus at the December 2021 TC39 meeting.

@ptomato ptomato marked this pull request as ready for review December 17, 2021 18:40
Previously, code that combined zero subsecond units with a specified
precision, such as the following:

Temporal.Duration.from({ years: 3 }).toString({ fractionalSecondDigits: 5 })

would not render the seconds with the correct precision. The above example
would return 'P3Y' and not 'P3YT0.00000S'.

Closes: #1763
@ptomato ptomato force-pushed the 1763-duration-fractionalseconddigits branch from 1690ed4 to 228f183 Compare December 17, 2021 18:53
@ptomato ptomato merged commit 3ee771e into main Dec 17, 2021
@ptomato ptomato deleted the 1763-duration-fractionalseconddigits branch December 17, 2021 19:44
justingrant added a commit to justingrant/temporal-polyfill that referenced this pull request Dec 20, 2021
justingrant added a commit to js-temporal/temporal-polyfill that referenced this pull request Jan 8, 2022
ptomato added a commit to ptomato/test262 that referenced this pull request Feb 4, 2022
tc39/proposal-temporal#1956 clarified that the
fractionalSecondDigits and smallestUnit options to
Temporal.Duration.p.toString() specified the exact number of digits after
the decimal point, no more and no less.

This was a normative change that achieved consensus at the December 2021
TC39 meeting.
ptomato added a commit to ptomato/test262 that referenced this pull request Feb 4, 2022
tc39/proposal-temporal#1956 clarified that the
fractionalSecondDigits and smallestUnit options to
Temporal.Duration.p.toString() specified the exact number of digits after
the decimal point, no more and no less.

This was a normative change that achieved consensus at the December 2021
TC39 meeting.
ptomato added a commit to ptomato/test262 that referenced this pull request Feb 5, 2022
tc39/proposal-temporal#1956 clarified that the
fractionalSecondDigits and smallestUnit options to
Temporal.Duration.p.toString() specified the exact number of digits after
the decimal point, no more and no less.

This was a normative change that achieved consensus at the December 2021
TC39 meeting.
rwaldron pushed a commit to tc39/test262 that referenced this pull request Feb 8, 2022
tc39/proposal-temporal#1956 clarified that the
fractionalSecondDigits and smallestUnit options to
Temporal.Duration.p.toString() specified the exact number of digits after
the decimal point, no more and no less.

This was a normative change that achieved consensus at the December 2021
TC39 meeting.
webkit-commit-queue pushed a commit to WebKit/WebKit that referenced this pull request May 7, 2022
https://bugs.webkit.org/show_bug.cgi?id=240193

Reviewed by Yusuke Suzuki.

This patch implements the spec correction of tc39/proposal-temporal#1956:
`new Temporal.Duration.from(1).toString({ fractionalSecondDigits: 2 })` should be P1Y0.00S, not just P1Y.

* stress/temporal-duration.js: Add a test case.
* test262/expectations.yaml: Mark two test cases as passing.

* runtime/TemporalDuration.cpp:
(JSC::TemporalDuration::toString):

Canonical link: https://commits.webkit.org/250388@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@293942 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs plenary input Needs to be presented to the committee and feedback incorporated spec-text Specification text involved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duration.toString with fractionalSecondDigits while s = ms = µs = ns = 0
2 participants