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

Some Temporal.Instant instances produce identical logging/Deno.inspect output to Date instances #27585

Closed
lionel-rowe opened this issue Jan 8, 2025 · 2 comments · Fixed by #27684
Labels
bug Something isn't working correctly ext/console Related to the ext/console crate temporal

Comments

@lionel-rowe
Copy link
Contributor

Temporal.Instant instances with an integer number of epoch milliseconds but not an integer number of seconds produce identical Deno.inspect output to Date instances with the corresponding value:

function log(ns: bigint) {
    const date = new Date(Number(ns / 1_000_000n))
    const instant = Temporal.Instant.fromEpochNanoseconds(ns)

    console.log(date)
    console.log(instant)
    console.log(Deno.inspect(date, { colors: true }) === Deno.inspect(instant, { colors: true }))
}

log(1736321470_000_000_000n)
log(1736321470_888_000_000n)
log(1736321470_888_888_000n)
log(1736321470_888_888_888n)

Output:

2025-01-08T07:31:10.000Z
2025-01-08T07:31:10Z
false
2025-01-08T07:31:10.888Z
2025-01-08T07:31:10.888Z
true
2025-01-08T07:31:10.888Z
2025-01-08T07:31:10.888888Z
false
2025-01-08T07:31:10.888Z
2025-01-08T07:31:10.888888888Z
false

Possible solutions:

  • Always show the full 9 ns places for instances of Temporal classes, similar to how the full 3 ms places are always shown for Date instances. However, that leads to unnecessarily verbose output for many cases
  • Use different colors/other different formatting

Version: Deno 2.1.4

@marvinhagemeister marvinhagemeister added bug Something isn't working correctly temporal labels Jan 9, 2025
@bartlomieju bartlomieju added the ext/console Related to the ext/console crate label Jan 14, 2025
@bartlomieju
Copy link
Member

bartlomieju commented Jan 14, 2025

The suggestion to change the color output to be different than Date makes sense to me. This can be done by updating this line:

temporal: "magenta",

That said, the problem seems to stem from the fact that temporal.toString() produces unexpected value. This is actually implemented in V8, so I suggest to open an upstream bug about that, if it is actually a bug.

@lionel-rowe
Copy link
Contributor Author

the problem seems to stem from the fact that temporal.toString() produces unexpected value

I think toString is correct based on the spec, it's only the fact that this produces indistinguishable logging output between Instants and Dates that's a problem. toString can be supplied fractionalSecondDigits or smallestUnit options, but with no/default options it truncates trailing zeros.

bartlomieju added a commit that referenced this issue Jan 16, 2025
This commit changes output color of `Temporal` instances from
"magenta" to "cyan" to discriminate them from `Date` instances.

Closes #27585
bartlomieju added a commit that referenced this issue Jan 16, 2025
This commit changes output color of `Temporal` instances from
"magenta" to "cyan" to discriminate them from `Date` instances.

Closes #27585
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly ext/console Related to the ext/console crate temporal
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants