feat(ts): protobuf-es runtime mode for TS client/server (native messages + sebuf transport)#207
feat(ts): protobuf-es runtime mode for TS client/server (native messages + sebuf transport)#207hishamank wants to merge 33 commits into
Conversation
Oneofs without an explicit sebuf.http oneof_config annotation now render as `$case` discriminated unions instead of flattened optional fields, in both TS generators (protoc-gen-ts-client, protoc-gen-ts-server). Synthetic oneofs (proto3 `optional`) are unaffected. This is the default behavior — there is no opt-in flag. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Both TypeScript generators (protoc-gen-ts-client, protoc-gen-ts-server) now emit one canonical type module per proto file (<proto>.ts) plus a shared errors.ts, and slim the service files down to service classes / handlers that import their request/response types and error helpers. This replaces the previous behavior of inlining the full transitive type closure into every service file, which duplicated shared types (e.g. SongID, ValidationError) across files. Types are now declared once and imported via relative specifiers. This is the default and only layout — there is no configuration flag. Requires 'strategy: all' in buf.gen.yaml so shared modules are emitted once across the whole graph rather than per-directory. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ions
The synthesized $case discriminated union described JSON that never exists
on the wire. Un-annotated oneofs serialize as plain protojson, which puts
the set member's JSON key directly on the parent object and emits no
discriminator field and no property named after the oneof. The generated
clients do raw JSON.stringify / resp.json() casts with no conversion layer,
so the $case shape was unreachable at runtime in both directions: reads
found undefined where the type promised a wrapper property, and writes sent
a "$case" key that protojson.Unmarshal rejects as an unknown field.
Un-annotated oneofs now render as a union discriminated by key presence,
intersected flat into the parent type — assignable to and from the exact
JSON the Go server produces and consumes, with no runtime conversion:
export type PlainEventContent =
| { text: TextContent; image?: never }
| { image: ImageContent; text?: never }
| { text?: never; image?: never };
export type PlainEvent = PlainEventBase & PlainEventContent;
The `?: never` guards keep the exactly-one construction guarantee and let
TypeScript narrow on presence (`if (e.text)`); the trailing all-never arm
models the unset oneof, which the server's protojson output permits. Set
oneof members are non-optional because oneof fields have explicit presence
— protojson always emits a set member, even at its zero value.
Messages whose fields all belong to oneofs skip the empty Base interface
and alias the union directly.
Annotated oneofs (oneof_config) are unchanged: their wire format is
produced by generated MarshalJSON/UnmarshalJSON pairs, so their TS shape
is a separate concern.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add unit coverage for the un-annotated oneof presence-union emitter and interface routing in tscommon, compiling a hand-built proto3 FileDescriptorProto into a protogen.Plugin to exercise real protogen inputs: - synthesizeOneofInfo: empty Discriminator, one variant per field with JSON names, correct IsMessage for scalar vs message members. - generatePresenceOneofUnionType: set member non-optional plus ?: never sibling guards and a final all-never arm, across a 3-variant message oneof and a scalar+message oneof. - GenerateInterface: base field emits XBase + intersection alias; a message whose fields all belong to one oneof aliases the union directly with no XBase interface. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The doc comment referenced a discriminated-oneof style field on EmitContext that does not exist on this branch. Describe only the behavior the function actually has: modules-mode cross-module imports via ctx, discriminated-union rendering for annotated oneofs, flattened optional fields otherwise. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every existing golden fixture is single-file, so the layout's core feature (importing types across proto packages via relative specifiers) had no golden coverage. Add a two-package fixture: crosspkg.common.v1 defines ItemID and a Category enum; crosspkg.shop.v1 imports them and embeds both in its service request/response messages. Wire the fixture into both generators' golden tests (protoc now takes a list of proto files) and assert explicitly that the emitted shop type module contains the relative cross-package import 'from "../../common/v1/types"', not just via golden comparison. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The TypeScript sections documented the old inline single-file output. Describe the current per-proto modules layout (one type module per source proto, slim client/server modules importing their types, shared errors.ts at the output root, cross-package relative imports) and document the required strategy: all in buf.gen.yaml. Fix the error-handling import examples to point at the type module and errors.ts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
#201) * fix(ts): flatten annotated non-flatten oneofs to match the wire format The TS generators rendered annotated non-flatten oneofs as a wrapper property named after the oneof: export interface NestedEvent { id: string; content?: { kind: "text"; text?: TextContent } | ...; } but no such property exists on the wire. The generated MarshalJSON writes the discriminator directly onto the parent object (raw["kind"] = "text") and leaves the variant key where protojson put it — also on the parent — so the actual JSON is {"id": ..., "kind": "text", "text": {...}}. The OpenAPI generator already documents this flat shape; TypeScript was the only generator disagreeing with the Go serialization. Non-flatten annotated oneofs now render flat, intersected with the base fields like flattened ones: export type NestedEventContent = | { kind: "text"; text: TextContent; image?: never; video?: never } | { kind: "image"; image: ImageContent; text?: never; video?: never } | { kind: "vid"; video: VideoContent; text?: never; image?: never } | { kind?: never; text?: never; image?: never; video?: never }; export type NestedEvent = NestedEventBase & NestedEventContent; Payloads are non-optional because oneof fields have explicit presence — a set member is always emitted. Sibling variant keys are `?: never` so narrowing on the discriminator also types the payload keys correctly, and the all-never arm models the unset oneof, which MarshalJSON emits with no discriminator at all. Flattened oneofs gain the same unset arm ({ type?: never }) for that reason. With every oneof now rendering flat on the parent, the wrapper-property branch in GenerateStandardInterface is unreachable and removed — which also removes the bug where the wrapper property kept the oneof's raw snake_case name (e.g. `super_title_image?:`) instead of a camelCase one. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(ts): cover multi-word oneof names in TS oneof emission Add a TS-only fixture (multi_word_oneof.proto) whose un-annotated oneof has a multi-word name (super_title_image), exercising the presence-union path in tscommon. This locks in the regression fixed on this branch: the oneof name must surface only as the PascalCase union type name (MultiWordEventSuperTitleImage) and never leak into the generated TS as a raw snake_case wrapper property. The fixture is scoped to the ts-client and ts-server generators only, leaving the shared oneof_discriminator.proto byte-identical across all six generators so the Go/OpenAPI/Python goldens and cross-generator consistency tests are untouched. - New golden test cases in tsclientgen/tsservergen; the new proto is added to the tsservergen cross-generator consistency proto list. - Explicit regression assertion (TestMultiWordOneofNameDoesNotLeak): generated TS contains MultiWordEventSuperTitleImage and not super_title_image. - Unit tests for nonFlattenedOneofBranch in tscommon (message, middle, and scalar variants), asserting discriminator/value/payload/never-guards. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
golangci-lint flagged the protogen test harness: msgFieldProto's signature and the oneofTestFile msg closure exceeded the 120-char golines limit, and oneofIndex's parameter always received 0 (unparam). Wrap the signatures and inline proto.Int32(0) at the call sites. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The golden tests exercise the TS client/server generators as subprocess protoc plugins, so their statements never counted toward Go coverage. Add an in-process golden runner per generator: build a protogen.Plugin from the existing fixture protos (via protoc --descriptor_set_out --include_imports, reusing the same protoc-absent skip) and call Generate() directly, then diff every emitted file against the checked-in golden files byte-for-byte. This lights up generator.go, modules.go, imports.go, and the EmitContext paths in types.go with real assertions. Add targeted error-path coverage: a reserved-name fixture (message named ValidationError) asserts CheckReservedNames fails loudly, and the invalid path-param / uncovered-field fixtures now run in-process to cover the server route-config error branches. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Integrates the oneof rework (presence-discriminated unions for un-annotated oneofs, flat annotated non-flatten unions with never guards and unset arms) into the per-proto type-modules layout. Resolution notes: - tscommon/types.go: routed all discriminated oneofs through GenerateFlattenedOneofInterfaceCtx; ported the union emitters to the import-aware path (nonFlattenedOneofBranch and generatePresenceOneofUnionType now resolve message payload types via ctx.RefMessage so cross-module references record imports). - golden_test.go (both TS generators): kept the modules-layout harness (protoFiles walk + import assertions) and added the multi_word_oneof fixture entry; TestMultiWordOneofNameDoesNotLeak now reads the type module plus the client file, since the union type lives in the module. - Golden fixtures regenerated with freshly built plugins. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Node ESM and TypeScript's node16/nodenext module resolution require an explicit file extension on every relative import specifier. The modules layout emitted extensionless specifiers (e.g. "./service", "../../errors"), so the generated package could not be imported under nodenext without a TS2835 error. Append ".js" in tscommon.RelativeImportSpecifier, the single chokepoint through which every relative import flows (cross-module type refs via ref and the shared errors module via NeedErrors). Regenerate all client and server goldens; every relative import line now ends in ".js". Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add tscommon.EmitPackageBarrels, called by both TS generators after they
emit their modules. It writes an index.ts into each non-root output
directory that holds generated modules, re-exporting every sibling module
(type modules plus the client/server module) with ".js" specifiers and a
deterministic sort. This lets consumers import a whole proto package by its
directory, e.g. import { Album, AlbumServiceClient } from ".../album/v1".
The output root, where the shared errors module lives, never receives a
barrel: aggregating packages at the root is the consumer's concern
(anghamna generates its own root index). "export *" is safe within a proto
package because message names are unique per package.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…t/ts-modules-oneof-integration
Nested proto messages and enums are hoisted to top-level TypeScript declarations using only their leaf name, which collides when a package also defines a top-level type of that name (e.g. a nested GetSubscriptionStatusResponse.SubscriptionStatus vs a top-level SubscriptionStatus). The per-package barrel's `export *` then fails to compile with TS2308 (duplicate export). Add QualifiedTSName, which prefixes a message/enum descriptor's leaf name with its enclosing message names (WrapperStatus), matching the convention proto authors already use for flattened messages. Route the type-name declaration sites (GenerateInterfaceCtx, GenerateEnumType) and the reference sites (RefMessage, RefEnum) through it so the declaration and every reference stay byte-identical. Top-level types are unaffected. Add a nested_collision fixture (top-level Status/Kind plus a Wrapper with nested Status/Kind of the same leaf names) exercised by both TS generators; the emitted package typechecks clean under nodenext. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…t/ts-modules-oneof-integration
…ess proof Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reject unknown parameter keys and invalid ts_runtime values in the plugin ParamFunc so a typo fails loudly instead of silently falling back to hand-rolled output. Collapse the ParseMessageRuntime pass- through wrapper into a single exported function. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…oreUnknownFields) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extend ts_runtime=protobuf-es to server-streaming (SSE) RPCs: the async
generator now takes a MessageInitShape request, encodes any request body via
create/toJson, and decodes each streamed event through
fromJson(<Res>Schema, JSON.parse(data), { ignoreUnknownFields: true }) instead
of a raw cast, importing schemas/types from protoc-gen-es <proto>_pb.js.
Derive protoc-gen-es local symbol names via ESQualifiedName (underscore-joined
nested names, e.g. Outer_Inner / Outer_InnerSchema) rather than QualifiedTSName,
matching @bufbuild/protoc-gen-es v2.12.1 output (verified empirically).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nknown fields ignored) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…onses) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…afe) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Enum path/query parameters (including repeated enum query params) are not representable in protobuf-es mode: enums are numeric there but URL params arrive as strings, and RefEnum resolves to the hand-rolled ./<proto>.js type module that es-mode deliberately does not emit -> downstream TS2307. Instead of emitting uncompilable output, both generators now reject enum path/query params at generation time via checkNoEnumParamsES with a clear error. Adds a non-enum unary GET golden (string path param + scalar query params, get_params.proto) wired into the es golden tests and typechecked under --strict --noUnusedLocals, plus in-process tests asserting the fail-loud path. Updates docs to cover enum path AND query params. Removes obsolete TODO(es). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The @bufbuild/protobuf import list was derived by regex word-boundary
scanning of the generated body, which had two failure modes:
- an RPC named Create renders `async create(`, injecting
`import { create } from "@bufbuild/protobuf"` into HAND-ROLLED output,
whose consumers do not install that package;
- a field named create in an es-mode GET method (`req.create`) imported
the helper unused, failing consumers under noUnusedLocals.
Each es-mode emission site now records exactly the helpers it prints
(MessageInitShape/fromJson per method; create/toJson only when a body or
response is encoded; create for the server GET init wrap), the body scan
is deleted, and EmitContext.NeedProtobufES is a no-op outside protobuf-es
runtime mode as a structural invariant. Existing goldens are
byte-identical; regression tests cover both failure modes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Byte-comparing goldens locks down what sebuf emits but not that it compiles against the real @bufbuild/protobuf types and the symbols protoc-gen-es actually exported — the check that catches an ESQualifiedName naming divergence in CI instead of a consumer build. Ports the tscommon/typecheck harness (strict nodenext + noUnusedLocals, tsc from PATH or pinned via npx, skips when unavailable) and runs it over testdata/golden/es in both generators, resolving @bufbuild/protobuf through the same git-ignored node_modules discovery the wire-conformance test uses (SEBUF_ES_NODE_MODULES override supported). Also checks in the transitive sebuf/http/annotations_pb.ts goldens the fixtures' *_pb.ts import — without them the es golden tree never typechecked (TS2307 on ./sebuf/http/annotations_pb.js). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
URL path/query parameters do not route through create/toJson/fromJson — only request bodies, response bodies, and SSE events do — and MessageInitShape makes every request field optional, so a missing path parameter compiles and surfaces only as a runtime "undefined" URL segment. Also notes the es golden typecheck tests as the guard for protoc-gen-es naming drift. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
SebastienMelki
left a comment
There was a problem hiding this comment.
The approach is right; the wire contract is the problem.
Using the official protoc-gen-es types and building sebuf's transport around them is exactly what we do in Go, and it's the correct direction. But that's precisely the bar this has to clear: in Go we don't hand the raw runtime encoder the wire — we wrap protoc-gen-go with a sebuf marshal layer (MarshalJSONSebuf, dispatched at internal/httpgen/generator.go:808) that renders sebuf's JSON, which deliberately diverges from canonical protojson wherever a sebuf.http annotation is set.
This PR uses protoc-gen-es types and its raw canonical codec (toJson/fromJson) — it skips the equivalent of that marshal layer. So for any annotated proto, es mode is on a different wire than both the sebuf Go server and our current hand-rolled TS client. It is not a drop-in replacement; it's a second, incompatible wire format that happens to coincide only when no JSON-mapping annotation is used.
The current hand-rolled client is wire-correct by construction: it does zero runtime transform (JSON.stringify(req) / resp.json() as T) and shapes its interfaces to match sebuf's wire. es mode swaps that for canonical-shaped types plus an active codec that enforces canonical protojson — so it regresses exactly the protos annotations exist for.
Regression table — es mode vs current wire (Go server + hand-rolled TS)
"Current wire" is what both the Go server and today's TS client speak. ✅ = consistent, 🟡 = tolerated but annotation intent lost, 🔴 = broken (throws or silent data loss).
sebuf.http annotation |
Current wire (Go + hand-rolled TS) | es mode (canonical protojson) | Status | Needed for drop-in |
|---|---|---|---|---|
| (none — plain scalars, nested msgs, repeated, maps) | canonical | canonical | ✅ | — works today |
unwrap (root list/map, map-value) |
[…] / {"AAPL":[…]} |
{"users":[…]} (wrapped) |
🔴 fromJson throws on array / shape mismatch |
re-wrap on encode, unwrap on decode |
oneof_config flatten + discriminator |
{"type":"text","body":…} |
{"text":{…}} |
🔴 silent data loss (ignoreUnknownFields drops it) |
flatten/inline + discriminator inject/extract |
flatten / flatten_prefix |
{"billing_street":…} |
{"billing":{"street":…}} |
🔴 silent data loss | promote/collapse nested keys w/ prefix |
enum_value (custom string) |
"active" |
"STATUS_ACTIVE" |
🔴 fromJson throws (unknown enum) |
name↔custom-value map both directions |
timestamp_format (unix/date) |
1705312200 / "2024-01-15" |
"2024-01-15T09:30:00Z" |
🔴 fromJson throws (expects RFC3339) |
re-encode/parse per format |
bytes_encoding (hex/base64url/raw) |
"48656c6c6f" |
std base64 | 🔴 wrong/throws on decode | transcode to/from configured encoding |
nullable (explicit null) |
null vs absent vs value |
null≡absent (protojson) | 🟡 null/absent distinction lost | preserve explicit null on encode |
empty_behavior (PRESERVE/NULL/OMIT) |
{} / null / omitted |
canonical (omit/{}) |
🟡 NULL/OMIT intent not honored | apply per-field empty policy on encode |
enum_encoding=NUMBER (no custom value) |
1 |
name string on encode, accepts number on decode | 🟡 tolerated on decode; wrong on encode | emit number when annotated |
int64_encoding=NUMBER |
12345 (number) |
"12345" (string) |
🟡 tolerated (protojson accepts both) | emit number when annotated |
Net: 6 annotations are outright broken, 4 more silently drop their intent. The wire-conformance test only exercises the ✅ row (defaults filled, unknown fields ignored), so none of this is caught today.
What "drop-in replacement" requires
To match Go, the es transport needs the TS analog of MarshalJSONSebuf: an annotation-aware shim layered over toJson/fromJson that rewrites the canonical JSON into sebuf's wire (and back) — flatten oneofs, apply enum_value maps, unwrap, transcode timestamps/bytes, honor null/empty policy. protoc-gen-es types don't carry sebuf annotations, so the sebuf plugin has to emit these transforms per message from the descriptors it already reads. That's real scope — effectively porting the internal/httpgen/*.go custom-JSON pipeline to TS.
Given that, I'd suggest landing this in two steps:
- Now — make es mode safe by construction: fail loud. Extend the
checkNoEnumParamsESpattern into a full guard that walks each RPC's transitive input/output/SSE-event message closure and errors at generation time if any field/message/oneof carries a JSON-mapping annotation es can't honor (all 🔴/🟡 rows above). Theannotationspackage already exposes every getter needed (IsRootUnwrap,FindUnwrapField,GetOneofDiscriminatorInfo,GetEnumValueMapping,IsTimestampField,GetBytesEncoding,IsNullableField,GetEmptyBehavior,GetInt64Encoding,GetEnumEncoding). This ships a correct es mode restricted to protos it can actually round-trip. - Later — the transform layer, if/when we want annotated protos supported, tracked as a follow-up.
- Docs — correct the framing now. "Known limitations" omits this, and the prose currently claims the opposite ("Go-server parity", "both directions go through protobuf-es's canonical JSON"). It should say plainly: es mode speaks canonical protojson and honors no
sebuf.httpJSON-mapping annotation; it is only wire-compatible with a sebuf server for protos that use none of them (enforced by the guard in step 1).
Two smaller correctness issues (independent of the above)
- Non-string path params emit uncompilable es code. Path params are assigned the raw URL string (
tsservergen/generator.go:629,:649); for a numeric/bool field the branded type isnumber/bigint/boolean→ TS2322. Theget_paramsgolden only covers a string path param. Fold this into the same guard (or coerce). - Repeated non-string query params.
req.x.forEach(v => params.append("p", v))(tsclientgen/generator.go:467) lacksString(v), sorepeated int32/boolwon't typecheck. AddString(v).
Caveat on this report
This is a preliminary report: the wire shapes above are derived from reading the Go marshal pipeline and the protobuf-es codec, not from an executed cross-runtime round-trip. The exact throw-vs-silent behavior for the 🟡 rows in particular should be confirmed empirically. More broadly — this is the gap that matters most: if we want to trust this system, es mode needs real correctness checks, i.e. a conformance harness that round-trips each annotation through an actual sebuf Go server (or the hand-rolled client's wire) and asserts byte-equivalence, rather than only asserting protobuf-es's canonical behavior in isolation. That harness is also what would let us safely lift the fail-loud guard later, annotation by annotation.
Requesting changes on the wire-incompatibility blocker (fail-loud guard + docs at minimum). Happy to pair on the guard — it's a small, contained addition that turns this from an incompatible second wire format into a safe, honest opt-in.
…am coercion es-mode serializes with protobuf-es's canonical protojson codec (toJson/fromJson) and applies none of sebuf's sebuf.http JSON-mapping annotations. A sebuf Go server layers an annotation-aware transform (MarshalJSONSebuf) on top of protojson that deliberately diverges from canonical protojson, so for any annotated proto es-mode is on a different, incompatible wire than the Go server (and the hand-rolled TS output). Rather than emit a silently-incompatible second wire format, fail loud. - Add tscommon.CheckESMessageAnnotations: walks each RPC's transitive request/response/SSE-event message closure and errors at generation time on unwrap, oneof_config flatten, flatten/flatten_prefix, enum_value, timestamp_format, bytes_encoding, nullable, empty_behavior, enum_encoding=NUMBER, and int64_encoding=NUMBER. Wired into both the ts-client and ts-server generators alongside the existing enum-param guard. Regression tests cover all ten annotation fixtures in both generators. - Fix non-string path params in es-mode (server): coerce the raw URL string to the protobuf-es field type at both emission sites (create() init shape and branded-body merge) via esPathParamInitExpr — Number() for 32-bit/float, BigInt() for 64-bit, === "true" for bool, strings verbatim. Previously these emitted uncompilable code (TS2322). Extend the get_params fixture with numeric/bool path params (GET) and a numeric path param merged into a POST body, so the es golden tsc --noEmit typecheck compiles the coercion. - Fix repeated non-string query params (client): wrap the element in String(v) so URLSearchParams.append typechecks for repeated int32/bool/etc. - Docs: replace the implied Go-server-parity framing with an explicit wire-contract note (es-mode speaks canonical protojson, honors no JSON-mapping annotation, only wire-compatible for protos using none, enforced by the guard); add it as the headline Known limitation; correct the URL-param note to describe the new path-param coercion. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…t roadmap
Close the server-side mirror of the client repeated-query fix: in protobuf-es
mode a repeated non-string query param was emitted as `params.getAll("x")`
(string[]) into a strongly-typed protobuf-es element type (number[]/bigint[]/
boolean[]), which fails tsc. Coerce per element via esQueryListInitExpr —
.map(Number) for 32-bit/float, .map(BigInt) for 64-bit, .map(v => v === "true")
for bool, string[] verbatim. Enum lists never reach here (rejected up front by
checkNoEnumParamsES). Hand-rolled mode is unchanged.
Extend the get_params es fixture with repeated string/int32/int64/bool query
params so the es golden `tsc --noEmit` typecheck compiles the coercion.
Document the es-mode roadmap in docs/client-generation.md: the conformance
harness (cross-runtime byte-equivalence oracle from the Go consistency tests),
the transform layer (TS analog of MarshalJSONSebuf as a runtime shim over
toJson/fromJson), and the annotation-by-annotation order for lifting the
fail-loud guard.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Update — addresses the "fail-loud + docs" blocker, plus the two smaller correctness issuesPushed in Wire-incompatibility blocker → fail loud (
|
|
Heads-up on review order: a follow-up PR #210 ( |
8c9672a to
bed82f3
Compare
Rebase the protobuf-es transport work onto squash-merged #199 (oneof) and #200 (modules). Preserves all es-mode code paths (ts_runtime=protobuf-es, es goldens + typecheck harness, NeedProtobufES/EmitContext plumbing, SSE es transport, fail-loud guards, URL-param coercion) and folds in main's fixes: oneof variant typing via TSFieldTypeCtx + flattenedChildJSONNames unset-arm guard, index-signature proto maps ({ [key: string]: T }), reserved-name aliasing + validatePathMode, plugintest.Build harness, and hand-rolled repeated query-param coercion. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Rebase #210's ts_error_handling=result work onto the freshly-rebased #207. Combines #210's Result-mode surfaces (Result<T,ClientError> return path, result.ts shared module, decodeError, ValidateRuntimeOptions, es-result goldens) with everything #207 pulled from main (#199 oneof fixes, record-map `{ [key: string]: T }`, reserved-name aliasing/harness). - Resolve inprocess_golden_test.go reserved-name conflict: adopt #207's new aliasing semantics (reserved names aliased, not fatal) while keeping #210's 3-arg New(plugin, runtime, errorHandling) signature. - Regenerate hand-rolled + es + es-result TS goldens. - gitignore the es-result node_modules typecheck symlink (parity with es). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nctions Brings the rebased #210 (es transport #207 + Result mode + main fixes) underneath #211's standalone tree-shakeable es-mode client. Combined result: - #211 standalone functions preserved: each RPC is an `export async function` (unary) / `export async function*` (SSE) taking a required per-call `RequestOptions`, the shared client.ts base RequestOptions + RefClient/EmitClientModule, `{Service}RequestOptions extends RequestOptions` for typed-header services, top-level handleError, and `const path` when there are no path params. - #210 Result mode preserved underneath: standalone unary functions in result mode return `Promise<Result<T, ClientError>>` via decodeError; #207 es transport, oneof fixes, and reserved-name handling remain. Regenerated es, es-result, and hand-rolled client goldens plus ts-server goldens. Added the missing sebuf/http/headers_pb.ts transitive es fixture (analogue of the committed annotations_pb.ts) that es_headers_pb.ts imports, so the es golden typecheck resolves. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
SebastienMelki
left a comment
There was a problem hiding this comment.
Great work on this, Hicham — the architecture is clean, the fail-loud guards are thorough, and the hand-rolled default is byte-identical (zero regressions). We're very close to merge, just one required fix.
Required
Bug: single-value int64/uint64 query params not coerced in es-mode (server)
internal/tsservergen/generator.go:929-937 — the single-value query param path uses TSScalarTypeForField() which returns TSString for int64 (the hand-rolled representation). In es-mode protobuf-es expects bigint, but the code falls through to the default case and emits params.get("x") ?? "" — assigning a string where bigint is expected.
The repeated query param path (line 898) and path params (esPathParamInitExpr) both handle this correctly; the single-value path is the only one missing the es-mode branch.
Fix: add an es-mode check before the TSScalarTypeForField switch (analogous to the repeated field handling at line 898), reusing esPathParamInitExpr for the coercion. Then add a single-value int64 query param to get_params.proto so the es golden typecheck locks it in.
Suggestions (non-blocking, good follow-ups)
-
Expand wire-conformance test. The current conformance fixture covers omitted zero values + unknown field rejection — the core value prop. But it's narrow: no large int64 values (precision boundary), no nested messages with mixed populated/omitted fields, no map fields. Especially worth testing int64 boundary values since es-mode uses
bigintvs hand-rolledstring. -
Add server-side wire conformance. The client has a wire-conformance test proving
fromJsonmaterializes defaults. The server has no equivalent provingtoJson(create(...))produces correct wire output. A symmetric test (encode a known message, assert the JSON matches a canonical fixture) would close that gap.
What looks good
- Fail-loud guards are comprehensive — JSON-mapping annotations, enum path/query params all rejected at generation time with clear errors.
- Emission-site import tracking is the right fix for the regex scanning bug.
NeedProtobufESis structurally safe throughEmitContext. - es golden typecheck (
tsc --noEmitwithnoUnusedLocals) is real CI protection against protoc-gen-es naming drift. - Documentation is excellent — wire contract,
MessageInitShapecaveats, known limitations, and the roadmap (conformance harness → transform layer → annotation-by-annotation lift) are all clearly laid out. - No regressions — all pre-existing hand-rolled goldens unchanged.
Thanks for the thorough work here — once the int64 query param fix lands this is ready to go. 🚀
The single-value query-param path in the TS server generator used
TSScalarTypeForField(), which reports int64/uint64 as TSString (the
hand-rolled representation). In protobuf-es mode the message shape types
64-bit fields as bigint, so the value fell through to the string default
(`params.get("x") ?? ""`), assigning a string where a bigint was
expected. The repeated-query and path-param es paths already coerce
correctly; only the single-value scalar path was missing the es branch.
Add an es-mode branch before the TSScalarTypeForField switch that emits a
coerced init expression via a new esQueryScalarInitExpr helper: 64-bit ->
BigInt(...), 32-bit/float -> Number(...), bool -> === "true", string
verbatim. Output is byte-identical to hand-rolled for every type except
64-bit (now BigInt-wrapped), so only the es goldens change.
The client had a related gap: the single-value query zero-check used the
string "0" comparison for int64, which compares bigint to string in
es-mode (TS2367). Add esZeroCheckForField so 64-bit fields check `!== 0n`
in es-mode; String(bigint) serialization was already correct.
Add a single-value int64 `cursor` query param to the get_params.proto
fixtures (client + server) so the es golden typecheck locks in the bigint
coercion.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
🔍 CI Pipeline Status❌ Lint: failure 📊 Coverage Report: Available in checks above |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #207 +/- ##
=========================================
- Coverage 11.75% 5.17% -6.58%
=========================================
Files 65 61 -4
Lines 11121 10412 -709
=========================================
- Hits 1307 539 -768
- Misses 9780 9848 +68
+ Partials 34 25 -9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
What
Adds an opt-in
ts_runtime=protobuf-esmode toprotoc-gen-ts-clientandprotoc-gen-ts-server. In this mode, message types + JSON codec come from Buf's nativeprotoc-gen-es(*_pb.ts), and sebuf generates only the HTTP transport on top — routing every request/response throughcreate/toJson/fromJson(..., { ignoreUnknownFields: true }). This mirrors how Go usesprotoc-gen-go+protoc-gen-go-http.The existing hand-rolled mode stays the default — no behavior change unless
ts_runtime=protobuf-esis passed.Why
sebuf's Go output uses the language's native protobuf runtime, so it can't drift from protojson. The TS output hand-rolls message types with no codec, so it can silently disagree with the wire — e.g. a server omits zero-values per protojson, but the hand-rolled TS types declare them present, so
resp.list.map(...)throws on an empty list. This brings TS into the native-runtime tier. It is not adopting Connect — sebuf keeps its own HTTP transport (paths, headers,ValidationError); only the message layer goes native.How it works
ts_runtime=protobuf-esplugin option (invalid values fail loudly).protoc-gen-esowns them) but still emits the sharederrors.ts.<Msg>Schema(value) +<Msg>(type) from./<proto>_pb.js.@bufbuild/protobufimport lists only the symbols actually used: each es-mode emission site records the helpers it prints (MessageInitShape/fromJsonper method,create/toJsononly when a body or response is encoded), so the import mirrors real usage and compiles undernoUnusedLocals.MessageInitShape<typeof ReqSchema>params;body: JSON.stringify(toJson(ReqSchema, create(ReqSchema, req)));fromJson(ResSchema, await resp.json(), { ignoreUnknownFields: true }).fromJsonrequest decode;toJson(create(ResSchema, result))response encode; handler takes branded<Req>, returnsMessageInitShape<...>.Review fixes (second round of commits)
A review pass found two real issues in the first iteration; both are fixed on this branch:
@bufbuild/protobufimport (fixed in8c1e4c1). The runtime-symbol import list was originally derived by regex-scanning the generated body, and the scan ran in both modes. An RPC literally namedCreaterendersasync create(, which matched\bcreate\band injectedimport { create } from "@bufbuild/protobuf"into hand-rolled output — a package hand-rolled consumers don't install. A field namedcreatein an es-mode GET method had the mirror problem: an unused import that failsnoUnusedLocals. The body scan is now deleted; each es-mode emission site records exactly the helpers it prints, andNeedProtobufESis a no-op outside es mode as a structural invariant. Regression tests cover both failure modes (runtime_symbols_test.goin both generators), and all pre-existing goldens are byte-identical under the new tracking.96b33c9). The es goldens' typecheck was a manual verification, and it turned out the checked-in golden tree didn't actually compile: the fixtures'*_pb.tsimport./sebuf/http/annotations_pb.js, which was never checked in (TS2307). NewTestTSClientGenESGoldenTypecheck/TestTSServerGenESGoldenTypecheckruntsc --noEmit(strict nodenext +noUnusedLocals, via the portedtscommon/typecheckharness) overtestdata/golden/esagainst the real@bufbuild/protobufpackage, skipping cleanly when the toolchain is absent. The missingannotations_pb.tsgoldens are now checked in. This is also the machine guard forESQualifiedNamedrift: a symbol-name divergence from protoc-gen-es now fails CI with aTS2724: no exported membererror instead of surfacing in a consumer build (verified by fault injection).Also documented (
5854efd): URL path/query params do not route through the codec (only bodies and SSE events do — by design, URL params have no protojson form), andMessageInitShapemakes all request fields optional, so a missing path param compiles and only surfaces as a runtime"undefined"URL segment.Tests / verification
go test ./..., gofmt, vet, staticcheck all green.testdata/golden/es/).tsc --noEmit, strict nodenext,noUnusedLocals) against the real@bufbuild/protobufpackage — no longer a manual claim.@bufbuild/protobuf(RPC namedCreate) and that es-mode imports only the runtime symbols actually used (GET method with a field namedcreate).[]), and unknown fields don't throw (ignoreUnknownFields) — but do throw without the flag.Known limitations (documented in
docs/client-generation.md)ESQualifiedName, verified against@bufbuild/protoc-gen-es@2.12.1; the es golden typecheck catches drift on other versions).MessageInitShape— see docs.Base
Targets
feat/ts-modules-oneof-integration(the merged #199 + #200 result) since the es transport reuses that modules-layout import machinery. Rebase ontomainonce those land.🤖 Generated with Claude Code