fix: fix double division, fix loss of millisecond unit #1221
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.
This PR fixes an issue I introduced in the initial
useDate=temporalPR here. The generated code fortoTimestampwas dividing theepochMillisecondstwice, causing the date to get truncated to1970-01-01, so the tests would seem to work for dates.There was also a bug in how
nanoswas calculated:since we were deriving the start date from
epochMilliseconds, so the remainder only included micro/nanoseconds (milliseconds were always000).The PR addresses the first issue by ensuring we only divide once, in the generated
secondscode. It fixes the latter by using the built-in Temporal.Instant.prototype.round method to round to the nearest second without needing to do any explicit math.Lastly, it also updates the tests, as the
toStrictEqualcheck was succeeding even forInstants whose string representations were clearly different, so now we will test against the output string.