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

fix(unstable/temporal): implement Temporal.ZonedDateTime.getTimeZoneTransition #27770

Merged
merged 2 commits into from
Feb 17, 2025

Conversation

printfn
Copy link
Contributor

@printfn printfn commented Jan 22, 2025

This patches Temporal to support the getTimeZoneTransition method on ZonedDateTime.

See #27731.

@CLAassistant
Copy link

CLAassistant commented Jan 22, 2025

CLA assistant check
All committers have signed the CLA.

@printfn printfn force-pushed the temporal-time-zone-transition branch 6 times, most recently from 02b2914 to ee7293d Compare January 22, 2025 07:28
@printfn printfn force-pushed the temporal-time-zone-transition branch from ee7293d to be7d981 Compare January 22, 2025 07:40
Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this implementation based on some existing code?

@printfn
Copy link
Contributor Author

printfn commented Jan 27, 2025

The parameter parsing of getTimeZoneTransition is based on https://github.com/tc39/proposal-temporal/blob/1b1fd9f5cfd46c95189691f08b36993af5019989/polyfill/lib/zoneddatetime.mjs#L432. The actual implementation is just a very thin wrapper around the existing V8 implementation.

@bartlomieju
Copy link
Member

Sorry for a slow response @printfn; we're just working on upgrading to V8 13.4 - I'm not sure, but maybe it already implements this functionality? If not we can still land it.

@bartlomieju bartlomieju added this to the 2.2.0 milestone Feb 11, 2025
@printfn
Copy link
Contributor Author

printfn commented Feb 11, 2025

Makes sense, though based on https://github.com/search?q=repo%3Av8%2Fv8+getTimeZoneTransition&type=code I don't think v8 implements this yet.

Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@bartlomieju bartlomieju enabled auto-merge (squash) February 17, 2025 10:48
@bartlomieju bartlomieju merged commit 70d775c into denoland:main Feb 17, 2025
18 checks passed
@printfn printfn deleted the temporal-time-zone-transition branch February 17, 2025 18:45
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.

3 participants