Skip to content

inspect: add Temporal support#63154

Draft
Renegade334 wants to merge 1 commit intonodejs:mainfrom
Renegade334:inspect-temporal
Draft

inspect: add Temporal support#63154
Renegade334 wants to merge 1 commit intonodejs:mainfrom
Renegade334:inspect-temporal

Conversation

@Renegade334
Copy link
Copy Markdown
Member

Example output:

> Temporal.Now.instant()
Temporal.Instant 2026-05-06T16:19:21.226404053Z

> class XYZ extends Temporal.Duration {}
> new XYZ(1, 2, 3, 4, 5, 6, 7, 8, 9)
XYZ [Temporal.Duration] P1Y2M3W4DT5H6M7.008009S

> Object.assign(Temporal.Now.zonedDateTimeISO('Pacific/Honolulu'), { foo: 'bar' })
Temporal.ZonedDateTime 2026-05-06T06:24:22.920191895-10:00[Pacific/Honolulu] {
  foo: 'bar'
}

The Temporal global obviously has to be lazily accessed, and there's no way to do it that's immune from potential user tampering. The approach taken here is just to fetch it from the global object every time, since the output of inspect() is inherently aimed at the human reader, so it doesn't really need to be particularly hardened.

WIP: tests

Signed-off-by: Renegade334 <contact.9a5d6388@renegade334.me.uk>
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. util Issues and PRs related to the built-in util module. labels May 6, 2026
@ljharb
Copy link
Copy Markdown
Member

ljharb commented May 6, 2026

i'm confused, why would it need to be lazily accessed instead of using primordials?

// Returns void if either the type checks fail or the attempt at calling
// .toString() fails, so that format() can fall back to standard object
// formatting in either case.
// TODO: Hopefully the V8 API will expose typechecks at some point.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Temporal itself exposes type checks via internal slots and builtin methods.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, but we can't guarantee a non-mutable path to get our hands on those methods.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

of course we can, via primordials

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not while Temporal remains an optional feature, unfortunately.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

it's not? node 26 ships it unconditionally.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It's enabled by default, but remains behind both runtime (cf. --no-harmony-temporal) and build flags, so this is unlikely to change any time soon.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ahhh gotcha. surely we can still capture the builtins at startup, though, even if they're not real primordials yet?

Copy link
Copy Markdown
Member Author

@Renegade334 Renegade334 May 6, 2026

Choose a reason for hiding this comment

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

It's too early. Attempting to capture the global with something like const { Temporal } = globalThis in the header of inspect.js always hits thin air (I guess it's snapshotted?). We have to do it lazily from inspect(), at which point anything that intends to modify the Temporal global (polyfills etc.) will almost certainly already have run.

We could go full ham to hack around this, but it's not really a huge issue in the context of a function which only uses those methods to decide what flavour of human-readable output gets generated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. util Issues and PRs related to the built-in util module.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants