feat(http): stabilize structured fields - #7232
Conversation
bartlomieju
left a comment
bartlomieju
left a comment
There was a problem hiding this comment.
Approving. The question I most wanted answered before freezing this API was whether we'd be stabilizing an RFC 8941-shaped surface that would need a break to pick up RFC 9651's Date and Display String types — and the answer is no, this is already 9651 throughout. date() and displayString() are first-class, BareItem carries both variants, and the date.json / display-string.json conformance vectors run. Since BareItem is a discriminated union, any future RFC type is additive. That's the right footing to stabilize on.
Process matches the prior stabilizations (fe0fb93, b97383d) exactly: rename, deno.json remap, @experimental removal, mod.ts re-export, and dropping the old export path without an alias, which is our established precedent for unstable APIs.
Inline notes below. The mod.ts one is the only thing I'd genuinely like reconsidered before merge, since it's the one decision here we can't walk back.
| export * from "./negotiation.ts"; | ||
| export * from "./server_sent_event_stream.ts"; | ||
| export * from "./server_sent_event_parse_stream.ts"; | ||
| export * from "./structured_fields.ts"; |
There was a problem hiding this comment.
This is the piece I'd reconsider before merging, because it's permanent once shipped.
The wildcard hoists string, token, date, boolean, integer, decimal, binary and item — plus the types Item, List and Dictionary — into the root @std/http namespace. Those are about as generic as identifiers get, and anyone doing import * as http from "@std/http" inherits all of them permanently.
The strongest evidence is in this very PR: unstable_message_signatures.ts:70-79 imports them as sfString, sfToken and sfInnerList, because the bare names don't survive contact with surrounding code. If our own sibling module has to rename them on import, callers will too.
I'd suggest leaving this module subpath-only — the same treatment unstable_cache_control and friends get. Users reach it via @std/http/structured-fields, which reads better at the call site anyway.
There was a problem hiding this comment.
Good call, fixed in 9301b98. Subpath-only now, and structured_fields.ts is on the check_mod_exports skip list so CI does not demand it back.
| items: Item[]; | ||
| /** Parameters associated with the inner list. */ | ||
| parameters: Parameters; | ||
| parameters: ParameterMap; |
There was a problem hiding this comment.
Small inconsistency that freezes with this PR: Dictionary and ParameterMap are ReadonlyMap, and the Dictionary JSDoc explicitly promises "returned dictionaries are immutable" — but InnerList.items is a plain Item[] and List is a mutable Array, so a parsed list can be mutated in place while a parsed dictionary can't.
Not worth blocking on, but if you want them consistent, now is the moment.
There was a problem hiding this comment.
Fixed in b6957da. List is ReadonlyArray and InnerList.items is readonly Item[], with the same immutability note Dictionary has.
| * @see {@link https://www.rfc-editor.org/rfc/rfc9651#section-3.1.2} | ||
| */ | ||
| export type Parameters = ReadonlyMap<string, BareItem>; | ||
| export type ParameterMap = ReadonlyMap<string, BareItem>; |
There was a problem hiding this comment.
Renaming away from the RFC's own term "Parameters" is the right call given the clash with TypeScript's built-in Parameters<T> — just flagging that the divergence becomes permanent here, so it's worth being a deliberate choice rather than an artifact of the rename.
There was a problem hiding this comment.
Deliberate, and now documented as such in the ParameterMap JSDoc (b6957da).
| string, | ||
| token, | ||
| } from "./unstable_structured_fields.ts"; | ||
| } from "./structured_fields.ts"; |
There was a problem hiding this comment.
Not blocking, but worth knowing what we're stabilizing on: the httpwg conformance suite is only partially vendored. key-generated.json, large-generated.json and the rest of the *-generated.json family are absent, as is the upstream serialisation-tests/ directory — so serialization is currently only exercised through canonical round-trips rather than against the suite's dedicated serialization cases.
The generated files are where the nastier boundary cases live (key length, list size, byte-sequence padding). Worth pulling in as a follow-up now that this is stable surface.
There was a problem hiding this comment.
Pulled in with 006d785 rather than waiting on a follow-up: the *-generated.json family, listlist.json (also missing), and serialisation-tests/ with a dedicated runner. You were right about the boundary cases. serializeDecimal rounded half away from zero instead of half to even, so 0.0025 serialized as "0.003". Fixed in the same commit.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7232 +/- ##
==========================================
- Coverage 95.03% 95.03% -0.01%
==========================================
Files 618 618
Lines 51596 51853 +257
Branches 9340 9392 +52
==========================================
+ Hits 49035 49279 +244
- Misses 2021 2030 +9
- Partials 540 544 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Adds the missing *-generated.json parsing vectors, listlist.json, and the serialisation-tests/ directory, with a runner for serialisation-only cases. Fixes serializeDecimal to round half to even per RFC 9651 4.1.5, which the serialisation vectors exercise.
|
Thanks for the review, all four notes addressed.
|
2 participants
Stabilizes the RFC 9651 Structured Field Values module (added in #6963).
unstable_structured_fields.ts→structured_fields.ts; export is now@std/http/structured-fieldsParameterstype toParameterMap, as it shadowed TypeScript's built-inParameters<T>utility type@experimentaltags and exports the module frommod.tsunstable_message_signatures.tsimports accordinglyThe module passes the official HTTPWG conformance suite and has had no behavioral changes since landing.
Note
Recreates #7207, which was closed automatically when I accidentally deleted the head fork. The branch is identical to the original PR head.