Skip to content

Commit d25e922

Browse files
fix: fix double division, fix loss of millisecond unit (#1221)
This PR fixes an issue I introduced in the initial `useDate=temporal` PR [here](#1219). The generated code for `toTimestamp` was dividing the `epochMilliseconds` twice, causing the date to get truncated to `1970-01-01`, so the tests would seem to work for dates. There was _also_ a bug in how `nanos` was calculated: ```typescript Temporal.Instant .fromEpochMilliseconds(instant.epochMilliseconds) .until(instant); ``` since we were deriving the start date from `epochMilliseconds`, so the remainder only included micro/nanoseconds (milliseconds were always `000`). The PR addresses the first issue by ensuring we only divide once, in the generated `seconds` code. It fixes the latter by using the built-in [Temporal.Instant.prototype.round](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Temporal/Instant/round) method to round to the nearest second without needing to do any explicit math. Lastly, it also updates the tests, as the `toStrictEqual` check was succeeding even for `Instant`s whose string representations were clearly different, so now we will test against the output string.
1 parent feab2c9 commit d25e922

File tree

5 files changed

+23
-15
lines changed

5 files changed

+23
-15
lines changed

integration/grpc-js-use-date-temporal-bigint/grpc-js-use-date-temporal-bigint-test.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,23 +5,28 @@ import 'temporal-polyfill/global';
55

66
import { TestService, TimestampMessage } from "./grpc-js-use-date-temporal-bigint";
77

8-
const jan1 = Temporal.Instant.from("1970-01-01T14:27:59.987654321Z");
8+
const jan1 = Temporal.Instant.from("1973-02-01T14:27:59.987654321Z");
99

10-
describe("grpc-js-use-date-temporal", () => {
10+
describe("grpc-js-use-date-temporal-bigint", () => {
1111
it("compiles", () => {
1212
expect(TestService).not.toBeUndefined();
1313
});
1414

1515
it("returns simple temporal instant", async () => {
1616
const encoded = TestService.simpleNow.requestSerialize(jan1);
1717
const decoded = TestService.simpleNow.responseDeserialize(encoded);
18-
expect(decoded).toStrictEqual(jan1);
18+
expect(decoded.toString()).toStrictEqual(jan1.toString());
1919
});
2020

2121
it("returns wrapped temporal instant", async () => {
2222
const data: TimestampMessage = { timestamp: jan1 };
2323
const encoded = TestService.wrappedNow.requestSerialize(data);
2424
const decoded = TestService.wrappedNow.responseDeserialize(encoded);
25-
expect(decoded).toStrictEqual(data);
25+
26+
expect({
27+
timestamp: decoded.timestamp!.toString(),
28+
} satisfies Record<keyof TimestampMessage, string>).toStrictEqual({
29+
timestamp: jan1.toString()
30+
} satisfies Record<keyof TimestampMessage, string>);
2631
});
2732
});

integration/grpc-js-use-date-temporal-bigint/grpc-js-use-date-temporal-bigint.ts

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

integration/grpc-js-use-date-temporal/grpc-js-use-date-temporal-test.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import 'temporal-polyfill/global';
55

66
import { TestService, TimestampMessage } from "./grpc-js-use-date-temporal";
77

8-
const jan1 = Temporal.Instant.from("1970-01-01T14:27:59.987654321Z");
8+
const jan1 = Temporal.Instant.from("1973-02-01T14:27:59.987654321Z");
99

1010
describe("grpc-js-use-date-temporal", () => {
1111
it("compiles", () => {
@@ -15,13 +15,18 @@ describe("grpc-js-use-date-temporal", () => {
1515
it("returns simple temporal instant", async () => {
1616
const encoded = TestService.simpleNow.requestSerialize(jan1);
1717
const decoded = TestService.simpleNow.responseDeserialize(encoded);
18-
expect(decoded).toStrictEqual(jan1);
18+
expect(decoded.toString()).toStrictEqual(jan1.toString());
1919
});
2020

2121
it("returns wrapped temporal instant", async () => {
2222
const data: TimestampMessage = { timestamp: jan1 };
2323
const encoded = TestService.wrappedNow.requestSerialize(data);
2424
const decoded = TestService.wrappedNow.responseDeserialize(encoded);
25-
expect(decoded).toStrictEqual(data);
25+
26+
expect({
27+
timestamp: decoded.timestamp!.toString(),
28+
} satisfies Record<keyof TimestampMessage, string>).toStrictEqual({
29+
timestamp: jan1.toString()
30+
} satisfies Record<keyof TimestampMessage, string>);
2631
});
2732
});

integration/grpc-js-use-date-temporal/grpc-js-use-date-temporal.ts

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/main.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -993,12 +993,10 @@ function makeTimestampMethods(
993993
? code`
994994
function toTimestamp(instant: Temporal.Instant): ${Timestamp} {
995995
const date = {
996-
getTime: (): number => Math.trunc(instant.epochMilliseconds / 1_000),
996+
getTime: (): number => instant.epochMilliseconds,
997997
} as const;
998998
const seconds = ${seconds};
999-
const remainder = ${bytes.globalThis}.Temporal.Instant
1000-
.fromEpochMilliseconds(instant.epochMilliseconds)
1001-
.until(instant);
999+
const remainder = instant.round({ smallestUnit: "seconds", roundingMode: "floor" }).until(instant);
10021000
const nanos = (remainder.milliseconds * 1_000_000) + (remainder.microseconds * 1_000) + remainder.nanoseconds;
10031001
10041002
return { ${maybeTypeField} seconds, nanos };

0 commit comments

Comments
 (0)