Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ export const TypescriptCustomConfigSchema = z.strictObject({
useDefaultRequestParameterValues: z.optional(z.boolean()),
packageManager: z.optional(z.enum(["pnpm", "yarn"])),
flattenRequestParameters: z.optional(z.boolean()),
respectOptionalRequestBody: z.optional(z.boolean()),
exportAllRequestsAtRoot: z.optional(z.boolean()),
customReadmeSections: z.optional(z.array(CustomReadmeSectionSchema)),
testFramework: z.optional(z.enum(["jest", "vitest"])),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ export class EndpointSnippetGenerator {
this.context.errors.unscope();

this.context.errors.scope(Scope.RequestBody);
if (request.body != null) {
if (request.body != null && !this.callOmitsRequestBody({ request, snippet })) {
const bodyArg = this.getBodyRequestArg({ body: request.body, value: snippet.requestBody });
// a nop literal writes nothing (e.g. an example that omits an optional request body),
// so including it would emit a dangling argument delimiter.
Expand All @@ -443,6 +443,27 @@ export class EndpointSnippetGenerator {
return args;
}

/**
* Whether the call leaves the body out entirely, which the optional parameter allows. Applies
* only to a body the caller may omit, and only once the generator opts in to that.
*/
private callOmitsRequestBody({
request,
snippet
}: {
request: FernIr.dynamic.BodyRequest;
snippet: FernIr.dynamic.EndpointSnippetRequest;
}): boolean {
if (this.context.customConfig?.respectOptionalRequestBody !== true) {
return false;
}
if (request.bodyRequired !== false) {
return false;
}
const value = snippet.requestBody;
return value == null || (typeof value === "object" && !Array.isArray(value) && Object.keys(value).length === 0);
}

private getBodyRequestArg({
body,
value
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import { FernIr } from "@fern-api/dynamic-ir-sdk";
import { AbsoluteFilePath, join } from "@fern-api/path-utils";

import { buildDynamicSnippetsGenerator } from "./utils/buildDynamicSnippetsGenerator.js";
import { buildGeneratorConfig } from "./utils/buildGeneratorConfig.js";

const DYNAMIC_IR_TEST_DEFINITIONS_DIRECTORY = AbsoluteFilePath.of(
`${__dirname}/../../../../../packages/cli/generation/ir-generator-tests/src/dynamic-snippets/__test__/test-definitions`
);

const IR_FILEPATH = AbsoluteFilePath.of(
join(DYNAMIC_IR_TEST_DEFINITIONS_DIRECTORY, "respect-optional-request-body.json")
);

// `bulkRefund` takes a body the API does not require. An example that supplies nothing for it
// reaches the snippet generator as an empty body, since that is how the importer spells it.
const bulkRefundWithoutBody: FernIr.dynamic.EndpointSnippetRequest = {
endpoint: {
method: "POST",
path: "/refunds"
},
baseURL: undefined,
environment: undefined,
auth: undefined,
pathParameters: undefined,
queryParameters: undefined,
headers: undefined,
requestBody: undefined
};

describe("optional request body", () => {
it("drops the body argument once the generator opts in", async () => {
const generator = buildDynamicSnippetsGenerator({
irFilepath: IR_FILEPATH,
config: buildGeneratorConfig({ customConfig: { respectOptionalRequestBody: true } })
});

for (const requestBody of [undefined, {}]) {
const response = await generator.generate({ ...bulkRefundWithoutBody, requestBody });

expect(response.errors).toBeUndefined();
expect(response.snippet).toContain("client.bulkRefund()");
expect(response.snippet).not.toContain("{}");
}
});

it("still passes a body by default", async () => {
const generator = buildDynamicSnippetsGenerator({
irFilepath: IR_FILEPATH,
config: buildGeneratorConfig()
});

const response = await generator.generate({ ...bulkRefundWithoutBody, requestBody: {} });

expect(response.snippet).toContain("client.bulkRefund({})");
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# yaml-language-server: $schema=../../../../../fern-changes-yml.schema.json

- summary: |
Add `respectOptionalRequestBody`. With it enabled, an endpoint whose request body the API does
not require takes an optional body parameter — `refund(id: string, request?: RefundRequest)` —
keeping the body's own type rather than widening it to `RefundRequest | undefined`, and sending
no body when the caller omits it. Examples that supply no body render as `client.refund(id)`,
and the generated wire test for such an example no longer asserts a request body. Defaults to
`false`, so signatures and snippets are unchanged until you opt in.
type: feat
2 changes: 2 additions & 0 deletions generators/typescript/sdk/cli/src/SdkGeneratorCli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ export class SdkGeneratorCli extends AbstractGeneratorCli<SdkCustomConfig> {
packageManager: parsed?.packageManager ?? "pnpm",
generateReadWriteOnlyTypes: parsed?.experimentalGenerateReadWriteOnlyTypes ?? false,
flattenRequestParameters: parsed?.flattenRequestParameters ?? false,
respectOptionalRequestBody: parsed?.respectOptionalRequestBody ?? false,
exportAllRequestsAtRoot: parsed?.exportAllRequestsAtRoot ?? false,
testFramework: parsed?.testFramework ?? "vitest",
consolidateTypeFiles: parsed?.consolidateTypeFiles ?? false,
Expand Down Expand Up @@ -264,6 +265,7 @@ export class SdkGeneratorCli extends AbstractGeneratorCli<SdkCustomConfig> {
packageManager: customConfig.packageManager,
generateReadWriteOnlyTypes: customConfig.generateReadWriteOnlyTypes,
flattenRequestParameters: customConfig.flattenRequestParameters ?? false,
respectOptionalRequestBody: customConfig.respectOptionalRequestBody ?? false,
exportAllRequestsAtRoot: customConfig.exportAllRequestsAtRoot ?? false,
testFramework: customConfig.testFramework,
consolidateTypeFiles: customConfig.consolidateTypeFiles ?? false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ export interface SdkCustomConfig {
packageManager: "pnpm" | "yarn";
generateReadWriteOnlyTypes: boolean;
flattenRequestParameters: boolean | undefined;
respectOptionalRequestBody: boolean | undefined;
exportAllRequestsAtRoot: boolean | undefined;
testFramework: "jest" | "vitest";
consolidateTypeFiles: boolean | undefined;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,20 @@ const STRING_TYPE = FernIr.TypeReference.primitive({ v1: "STRING", v2: undefined
const OPTIONAL_STRING_TYPE = FernIr.TypeReference.container(FernIr.ContainerType.optional(STRING_TYPE));
const INTEGER_TYPE = FernIr.TypeReference.primitive({ v1: "INTEGER", v2: undefined });

interface EndpointRequestMockContextOpts {
shouldInlinePathParams?: boolean;
respectOptionalRequestBody?: boolean;
}

/**
* Creates a mock FileContext for endpoint request tests.
* This is more comprehensive than the basic mock contexts because endpoint requests
* exercise many more context properties (requestWrapper, sdkInlinedRequestBodySchema, etc.).
*/
// biome-ignore lint/suspicious/noExplicitAny: test mock needs to satisfy complex FileContext interface
function createEndpointRequestMockContext(opts?: { shouldInlinePathParams?: boolean }): any {
function createEndpointRequestMockContext(opts?: EndpointRequestMockContextOpts): any {
const context = {
respectOptionalRequestBody: opts?.respectOptionalRequestBody ?? false,
includeSerdeLayer: true,
retainOriginalCasing: false,
inlineFileProperties: false,
Expand Down Expand Up @@ -500,6 +506,59 @@ describe("GeneratedDefaultEndpointRequest", () => {
expect(params[0]?.type).toBe("string");
});

it("keeps the request parameter required when the generator has not opted in", () => {
const sdkRequest = createSdkRequestBody({ required: false });
const request = new GeneratedDefaultEndpointRequest({
ir: createMinimalIR(),
packageId: { isRoot: true },
sdkRequest,
service: createHttpService(),
endpoint: createHttpEndpoint({ sdkRequest }),
requestBody: FernIr.HttpRequestBody.reference({
requestBodyType: STRING_TYPE,
required: false,
contentType: undefined,
docs: undefined,
v2Examples: undefined
}),
generatedSdkClientClass: createMockSdkClientClass(),
retainOriginalCasing: false,
parameterNaming: "default",
caseConverter
});
const params = request.getEndpointParameters(createEndpointRequestMockContext());
expect(params[0]?.hasQuestionToken).toBe(false);
});

it("makes the request parameter optional when the referenced body is not required", () => {
const sdkRequest = createSdkRequestBody({ required: false });
const request = new GeneratedDefaultEndpointRequest({
ir: createMinimalIR(),
packageId: { isRoot: true },
sdkRequest,
service: createHttpService(),
endpoint: createHttpEndpoint({ sdkRequest }),
requestBody: FernIr.HttpRequestBody.reference({
requestBodyType: STRING_TYPE,
required: false,
contentType: undefined,
docs: undefined,
v2Examples: undefined
}),
generatedSdkClientClass: createMockSdkClientClass(),
retainOriginalCasing: false,
parameterNaming: "default",
caseConverter
});
const context = createEndpointRequestMockContext({ respectOptionalRequestBody: true });
const params = request.getEndpointParameters(context);
expect(params).toHaveLength(1);
expect(params[0]?.name).toBe("request");
// the type is untouched: only the question token comes from `required`
expect(params[0]?.type).toBe("string");
expect(params[0]?.hasQuestionToken).toBe(true);
});

it("includes wrapper parameter with wrapper sdkRequest", () => {
const sdkRequest = createSdkRequestWrapper();
const request = new GeneratedDefaultEndpointRequest({
Expand Down Expand Up @@ -689,6 +748,36 @@ describe("GeneratedDefaultEndpointRequest", () => {
);
});

it("serializes an omittable reference request body only once the caller supplies one", () => {
const sdkRequest = createSdkRequestBody({ required: false });
const referenceBody = FernIr.HttpRequestBody.reference({
requestBodyType: STRING_TYPE,
required: false,
contentType: undefined,
docs: undefined,
v2Examples: undefined
});
const request = new GeneratedDefaultEndpointRequest({
ir: createMinimalIR(),
packageId: { isRoot: true },
sdkRequest,
service: createHttpService(),
endpoint: createHttpEndpoint({ sdkRequest, requestBody: referenceBody }),
requestBody: referenceBody,
generatedSdkClientClass: createMockSdkClientClass(),
retainOriginalCasing: false,
parameterNaming: "default",
caseConverter
});
const context = createEndpointRequestMockContext({ respectOptionalRequestBody: true });
request.getBuildRequestStatements(context);
const args = request.getFetcherRequestArgs(context);
assert(args.body != null, "body should not be null");
expect(getTextOfTsNode(args.body)).toBe(
"mergeAdditionalBodyParameters(request == null ? undefined : serializers.testEndpoint.Request.jsonOrThrow(request), requestOptions?.additionalBodyParameters)"
);
});

it("returns form request type for x-www-form-urlencoded content type", () => {
const inlinedBody = createInlinedRequestBody({
properties: [createInlinedRequestBodyProperty("name", STRING_TYPE)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,16 @@ const INTEGER_TYPE = FernIr.TypeReference.primitive({ v1: "INTEGER", v2: undefin
// Mock context
// ──────────────────────────────────────────────────────────────────────────────

interface MockContextOpts {
useDefaultValues?: boolean;
useBigInt?: boolean;
respectOptionalRequestBody?: boolean;
}

// biome-ignore lint/suspicious/noExplicitAny: test mock for FileContext
function createMockContext(opts?: { useDefaultValues?: boolean; useBigInt?: boolean }): any {
function createMockContext(opts?: MockContextOpts): any {
return {
respectOptionalRequestBody: opts?.respectOptionalRequestBody ?? false,
includeSerdeLayer: true,
retainOriginalCasing: false,
inlineFileProperties: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,47 @@ export class GeneratedDefaultEndpointRequest implements GeneratedEndpointRequest
referenceToRequestBody,
context
);
return this.mergeAdditionalBodyParameters(serializedRequestBody, context);
const needsNullCheck = this.mayOmitRequestBody(context) && serializedRequestBody !== referenceToRequestBody;
return this.mergeAdditionalBodyParameters(
needsNullCheck
? this.skipSerializationWhenBodyIsOmitted(referenceToRequestBody, serializedRequestBody)
: serializedRequestBody,
context
);
}

/**
* Whether the caller may leave the body out of the call. Absent `required` means required,
* so endpoints predating the field keep serializing unconditionally, as do SDKs that have not
* opted into reading the field.
*/
private mayOmitRequestBody(context: FileContext): boolean {
return (
context.respectOptionalRequestBody &&
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
);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,16 @@ export class RequestBodyParameter extends AbstractRequestParameter {

public isOptional({ context }: { context: FileContext }): boolean {
const type = context.type.getReferenceToType(this.requestBodyReference.requestBodyType);
return type.isOptional;
return type.isOptional || this.mayBeOmitted(context);
}

/**
* Whether the caller may leave the body out of the call entirely. Absent `required` means
* required, which is what every endpoint predating the field relies on, and reading the field
* at all is opt-in so that existing SDKs keep their signatures.
*/
private mayBeOmitted(context: FileContext): boolean {
return context.respectOptionalRequestBody && this.requestBodyReference.required === false;
}

public generateExample({
Expand All @@ -77,7 +86,7 @@ export class RequestBodyParameter extends AbstractRequestParameter {
const type = context.type.getReferenceToType(this.requestBodyReference.requestBodyType);
return {
type: type.requestTypeNodeWithoutUndefined ?? type.typeNodeWithoutUndefined,
hasQuestionToken: type.isOptional
hasQuestionToken: type.isOptional || this.mayBeOmitted(context)
};
}
}
2 changes: 2 additions & 0 deletions generators/typescript/sdk/generator/src/SdkGenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ export declare namespace SdkGenerator {
packageManager: "pnpm" | "yarn";
generateReadWriteOnlyTypes: boolean;
flattenRequestParameters: boolean;
respectOptionalRequestBody: boolean;
exportAllRequestsAtRoot: boolean;
testFramework: "jest" | "vitest";
consolidateTypeFiles: boolean;
Expand Down Expand Up @@ -2164,6 +2165,7 @@ export class SdkGenerator {
useDefaultRequestParameterValues: this.config.useDefaultRequestParameterValues,
generateReadWriteOnlyTypes: this.config.generateReadWriteOnlyTypes,
flattenRequestParameters: this.config.flattenRequestParameters,
respectOptionalRequestBody: this.config.respectOptionalRequestBody,
parameterNaming: this.config.parameterNaming,
resolveQueryParameterNameConflicts: this.config.resolveQueryParameterNameConflicts
} satisfies Omit<FileContextImpl.Init, "sourceFile" | "importsManager" | "isForSnippet">;
Expand Down
Loading
Loading