feat(typescript): let the caller omit an optional request body - #17368
feat(typescript): let the caller omit an optional request body#17368devin-ai-integration[bot] wants to merge 7 commits into
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![nitpickybot[bot]](https://avatars.githubusercontent.com/in/2901896?s=60&v=4){kind=small}
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| private mayOmitRequestBody(): boolean { | ||
| return this.requestBody?.type === "reference" && this.requestBody.required === false; | ||
| } | ||
|
|
||
| /** | ||
| * An omittable body is typed as the body itself rather than `optional<Body>`, so its schema | ||
| * rejects `undefined`. Serialize only once the caller has supplied a body. | ||
| */ | ||
| private skipSerializationWhenBodyIsOmitted( | ||
| referenceToRequestBody: ts.Expression, | ||
| serializedRequestBody: ts.Expression | ||
| ): ts.Expression { | ||
| return ts.factory.createConditionalExpression( | ||
| ts.factory.createBinaryExpression( | ||
| referenceToRequestBody, | ||
| ts.factory.createToken(ts.SyntaxKind.EqualsEqualsToken), | ||
| ts.factory.createNull() | ||
| ), | ||
| undefined, | ||
| ts.factory.createIdentifier("undefined"), | ||
| undefined, | ||
| serializedRequestBody | ||
| ); |
There was a problem hiding this comment.
🟡 Omittable request body is still sent when request-parameter flattening is turned on
The body is only skipped when the value the caller passed is empty (request == null ? undefined : ... at generators/typescript/sdk/client-class-generator/src/endpoint-request/GeneratedDefaultEndpointRequest.ts:300-314), but with request-parameter flattening the value being checked is always a freshly built object, so an empty body is still sent with a JSON content type instead of no body at all.
Impact: For SDKs generated with flattening enabled, endpoints whose body may be omitted always send an empty JSON body, which servers may reject or treat differently from a bodyless request.
Why the emitted null check can never fire under flattenRequestParameters
With flattenRequestParameters: true and a named object body, hasBodyProperty returns false (generators/typescript/sdk/request-wrapper-generator/src/GeneratedRequestWrapperImpl.ts:871-899) and areBodyPropertiesInlined() returns true (generators/typescript/sdk/request-wrapper-generator/src/GeneratedRequestWrapperImpl.ts:457-462). RequestWrapperParameter.getReferenceToRequestBody therefore returns either the rest-spread variable _body (generators/typescript/sdk/client-class-generator/src/request-parameter/RequestWrapperParameter.ts:90-98,122-134) or the wrapper parameter itself, which defaults to {} when all properties are optional. Both are always non-null objects, so mayOmitRequestBody() emits a check that never evaluates true and serializers...jsonOrThrow({}) runs, producing an empty JSON body plus a Content-Type header.
Relatedly, the flattened branch of getFlattenedReferencedRequestBodyProperties (the named-object path, generators/typescript/sdk/request-wrapper-generator/src/GeneratedRequestWrapperImpl.ts:961-990) does not consult mayOmitReferencedBody, so required properties of a body that may be omitted entirely remain required on the wrapper interface.
Prompt for agents
When a referenced request body carries `required: false` and the generator runs with `flattenRequestParameters` enabled, the new omit handling in GeneratedDefaultEndpointRequest (mayOmitRequestBody / skipSerializationWhenBodyIsOmitted) is ineffective: for a named object body the wrapper flattens the body into individual properties, so `getReferenceToRequestBody` yields the rest-spread `_body` variable (or the wrapper parameter, which defaults to `{}`), neither of which can ever be null. The generated `x == null ? undefined : serialize(x)` therefore always serializes, and the SDK sends `{}` with a JSON Content-Type instead of omitting the body. Additionally, getFlattenedReferencedRequestBodyProperties only applies mayOmitReferencedBody on the non-flattened fallback path, so required properties of an omittable body stay required in the wrapper interface. Consider either disabling flattening for omittable referenced bodies, or emitting an emptiness check the flattened case can actually satisfy (e.g. checking that none of the flattened body keys were supplied) and propagating omittability onto the flattened properties.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Confirmed the mechanism, and it is a real gap — but only under flattenRequestParameters, which defaults to false (sdk/cli/src/SdkGeneratorCli.ts:97). With it on, a named object body flattens into individual wrapper properties, so getReferenceToRequestBody yields _body/the wrapper (never null) and the emitted x == null check can't fire; getFlattenedReferencedRequestBodyProperties likewise doesn't consult omittability.
It isn't a regression: today those endpoints get a required body under that config, so the flattened output is unchanged by this PR — omission simply isn't expressed there yet.
Closing it needs a product call rather than a mechanical fix, because "the body was omitted" has no representation once the body's properties are the parameters: either omittable bodies opt out of flattening (changes signatures for flattening users), or the generator emits "none of the body keys were supplied" and makes the flattened properties optional (widens required properties to optional, and makes an all-defaults call ambiguous with an explicitly empty body). Leaving as-is for this PR and raising it with @will.kendall.
Docs Generation Benchmark ResultsComparing PR branch against median of 5 nightly run(s) on
Docs generation runs |
SDK Generation Benchmark ResultsComparing PR branch against median of 5 nightly run(s) on Full benchmark table (click to expand)
main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via |
Reads the IR's new request-body `required` field: `false` makes the body parameter optional while keeping its own type, so `refund(id, request?: RefundRequest)` rather than a parameter typed `optional<RefundRequest>`. An absent body is not serialized. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…ture Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
With the flag on, an endpoint whose IR request body carries `required: false` takes an optional body parameter and sends no body when the caller omits it; the parameter keeps the body's own type instead of widening to `Body | undefined`. Defaults to off, so existing signatures are untouched until a user opts in. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
99b003c to
19a6ba4
Compare
…ature Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…-optional-request-body
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
1 participant
Description
First generator adoption of
HttpRequestBodyReference.requiredfrom #17360, which this PR is based on.required: falsemeans the call may omit the body; the body type itself is untouched. So the parameter's question mark now comes fromrequiredrather than from the type beingoptional<Body>:Three call shapes, all driven by the same field:
refund(id, request?: RefundRequest)bulkRefund(request?: RefundRequest)body?: RefundRequestin the wrapper, sorequest: RefundWithHeaderRequest = {}Absent
requiredstill means required, so every endpoint predating the field generates byte-identical output —git diff feat/ir-request-body-required -- seed/touches only the new fixture.Serialization needed one change. An omittable body is typed as
Body, notoptional<Body>, so its schema rejectsundefined; the body is now serialized only once the caller supplies one:Blocked on the IR release.
@fern-fern/ir-sdkis pinned at 67.15.0 here and the latest published is 67.20.0;requiredarrives in 67.21.0, which only exists once #17360 merges and releases. Compile will be red until then, at which point the pin moves to 67.21.0 in a follow-up commit. IR parsing passes unrecognized keys through, so the seed evidence below is real: the local CLI produced IR carryingrequired: falseand the generator read it.Changes Made
RequestBodyParameter— parameter is optional whenrequired === false, keepingrequestBodyTypeas the parameter typeGeneratedRequestWrapperImpl— same for the wrapper'sbodyproperty, both flattened and non-flattenedGeneratedDefaultEndpointRequest— skip serializing an omitted bodyts-optional-request-bodyexercising the four shapes above (including a required body as the control), language-prefixed so it runs forts-sdkonlyTesting
client-class-generator607 passed,request-wrapper-generator166 passedseed test --generator ts-sdk --fixture ts-optional-request-bodyand a regression pass overrespect-optional-request-bodyandexhaustive(no output change)Link to Devin session: https://app.devin.ai/sessions/1795b2f90c804736b12138567df6e981