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

Add GetUtcNow to ITimeSystem #1701

Merged
merged 3 commits into from Dec 12, 2023
Merged

Add GetUtcNow to ITimeSystem #1701

merged 3 commits into from Dec 12, 2023

Conversation

ejsmith
Copy link
Contributor

@ejsmith ejsmith commented Dec 11, 2023

Fixes #1700

@sebastienros
Copy link
Owner

Or since ITypeSystem currently doesn't provide the date/time, maybe it could expose TimeProvider (passed by ctor) so this would make mocking the time easier by reusing DefaultTimeSystem instead of creating a new class.

Let's see what @lahma thinks is best. Either way implementation is trivial.

@lahma
Copy link
Collaborator

lahma commented Dec 11, 2023

I think the current implementation should suffice quite nicely. TimeProvider is a bit problematic as it's a NET 8 feature and would require separate NuGet dependency added to get the feature to older targets (https://www.nuget.org/packages/Microsoft.Bcl.TimeProvider). This is why I would implement the support on consumer code instead of the library (it's very limited set of features where Jint needs current time as you can see).

Maybe a test case would be a nice addition showcasing the new possibilities?

@lahma
Copy link
Collaborator

lahma commented Dec 11, 2023

The TimeProvider uses DateTimeOffset as return value though: https://learn.microsoft.com/en-us/dotnet/api/system.timeprovider.getutcnow?view=net-8.0#system-timeprovider-getutcnow

The real use cases for Jint are for the timestamp I guess https://learn.microsoft.com/en-us/dotnet/api/system.timeprovider.gettimestamp?view=net-8.0#system-timeprovider-gettimestamp as we only need to subtract to get the different in milliseconds.

@sebastienros
Copy link
Owner

TimeProvider is a bit problematic as it's a NET 8 feature

No, it's been backported down to full framework. Will investigate why the doc says .NET 8 only.

@sebastienros
Copy link
Owner

Oh, different assemblies, yes, probably.

@ejsmith
Copy link
Contributor Author

ejsmith commented Dec 11, 2023

Yeah, might be nice to have the full TimeProvider available and could eventually be used to virtualize timers, timeouts and such as well as providing current time. I was just trying to keep this PR simple.

@ejsmith
Copy link
Contributor Author

ejsmith commented Dec 11, 2023

Added a test showing how to integrate TimeProvider

@lahma
Copy link
Collaborator

lahma commented Dec 12, 2023

Jint is is using central package management so you need to add the new dependency to Directory.Packages.props in root folder and reference it without version number in csproj. Can you also change the return type in ITmeProvider to DateTimeOffset? I know it doesn't matter for UTC but if we would ever add the GetLocalNow() we would be interested in offset part to and the methods would have parity (also parity with TimeProvider)

@ejsmith
Copy link
Contributor Author

ejsmith commented Dec 12, 2023

@lahma I'm confused by the build error saying it can't target .NET 8. I didn't change any framework version. This test project was already targeting net8.0.

@lahma
Copy link
Collaborator

lahma commented Dec 12, 2023

It's OK, macos runners are lagging behind with net8.0 support.

Copy link
Collaborator

@lahma lahma left a comment

Choose a reason for hiding this comment

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

Looks good, thank you!

@lahma lahma merged commit 93c5ee5 into sebastienros:main Dec 12, 2023
2 of 3 checks passed
@ejsmith
Copy link
Contributor Author

ejsmith commented Dec 12, 2023

Thank you!

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.

Support .NET 8 TimeProvider for virtualized time
3 participants