Add support for Temporal types

[srikrsna]

No description provided.

[changeset-bot]

🦋 Changeset detected

Latest commit: 6cbd973

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
capnweb	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/cloudflare/capnweb@278

npm i https://pkg.pr.new/cloudflare/capnweb/capnweb-validate@278

commit: 6cbd973

[teamchong]

/bonk review this

[ask-bonk]

The encoding isn't lossless for Temporal.Duration. toJSON() folds sub-second units into the seconds field, so the round trip changes the duration's fields even though the total stays the same. Checked with Node --harmony-temporal:

D.from({milliseconds: 1500}).toJSON()          // "PT1.5S"
D.from("PT1.5S")                               // seconds: 1, milliseconds: 500 (was milliseconds: 1500)
D.from({seconds: 59, milliseconds: 1000})      // -> "PT60S" -> seconds: 60, milliseconds: 0

That's a spec limitation of Duration's ISO string form, so the wire format can't avoid it without encoding the fields one by one. Please narrow the claim here (and in the comment at src/serialize.ts:394), for example: "Durations keep their total length, but sub-second units may be rebalanced into seconds." The alternative is a field-wise encoding if callers need exact fields.

[ask-bonk]

Review: 1 finding.

I posted 1 inline comment on the PR.

Ran Node with --harmony-temporal to check: a Temporal.Duration doesn't round-trip exactly. Converting it to a string merges sub-second units into the seconds field, so {milliseconds: 1500} comes back as {seconds: 1, milliseconds: 500}. The total length is the same but the individual fields aren't. The protocol docs call the encoding "lossless", and so does the comment at src/serialize.ts:394. That wording should be narrowed. The other option is to send durations field by field.

Not raised inline:

• packages/capnweb-validate (type-introspector.ts BUILTIN_VALUE_TYPES) doesn't know about the new Temporal types yet. Validated interfaces that use them will likely be reported as unsupported. This could be done in a follow-up.

github run