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

withTime() and withDate() methods on ZonedDateTime and PlainDateTime #1181

Merged
merged 2 commits into from
Nov 13, 2020

Conversation

justingrant
Copy link
Collaborator

@justingrant justingrant commented Nov 13, 2020

This PR implements a subset of the changes described in #522 (comment):

  • Add PlainDateTime.withPlainDate(), PlainDateTime.withPlainTime(), ZonedDateTime.withPlainDate(), and ZonedDateTime.withPlainTime() to address the use case of combining dates and times without spreading. These methods use the 'consolidate' behaviour for calendars (throw if non-ISO and different).

This PR includes docs, polyfill implementation, docs, and TS types but not spec updates for these methods.

BTW, I found a bug in ES.ConsolidateCalendars that is fixed here. That method never worked if the second calendar was ISO.

@codecov
Copy link

codecov bot commented Nov 13, 2020

Codecov Report

Merging #1181 (8ada529) into main (5930868) will decrease coverage by 2.73%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1181      +/-   ##
==========================================
- Coverage   93.55%   90.82%   -2.74%     
==========================================
  Files          19       17       -2     
  Lines        7961     9381    +1420     
  Branches     1264     1279      +15     
==========================================
+ Hits         7448     8520    +1072     
- Misses        506      851     +345     
- Partials        7       10       +3     
Flag Coverage Δ
test262 ?
tests 90.82% <ø> (+1.38%) ⬆️

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

Impacted Files Coverage Δ
polyfill/lib/shim.mjs
polyfill/lib/slots.mjs
polyfill/lib/legacydate.mjs
polyfill/lib/intrinsicclass.mjs
lib/intrinsicclass.mjs 56.81% <0.00%> (ø)
lib/plaindatetime.mjs 90.94% <0.00%> (ø)
lib/plainmonthday.mjs 84.15% <0.00%> (ø)
lib/intl.mjs 99.74% <0.00%> (ø)
lib/plaintime.mjs 87.45% <0.00%> (ø)
lib/calendar.mjs 77.68% <0.00%> (ø)
... and 11 more

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 4b1fd22...1bb72ac. Read the comment docs.

docs/zoneddatetime.md Outdated Show resolved Hide resolved
@justingrant
Copy link
Collaborator Author

All review feedback so far has been addressed. My git skills weren't good enough to keep @Ms2ger's spec commit separate so I just squashed it into the latest force-push.

@Ms2ger Ms2ger merged commit d0ce4dc into tc39:main Nov 13, 2020
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

Successfully merging this pull request may close these issues.

4 participants