From 0e679afad4cbfc1fbedf15acb3225530d9b2a015 Mon Sep 17 00:00:00 2001 From: Urata Daiki <7nohe@users.noreply.github.com> Date: Tue, 11 Aug 2026 21:53:54 +0900 Subject: [PATCH 1/5] fix: resolve Data type names from the SDK signature for digit-leading operationIds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When an operationId starts with a digit, hey-api prefixes the SDK function name (`_123NumericLead`) but strips the digits from the Data type (`NumericLeadData`). Deriving the type name from the method name (`${capitalizedMethodName}Data`) therefore missed, which silently disabled pagination (no infinite hooks despite a valid page parameter) and emitted `Options`, failing to compile with TS2344. Read the Data type name from the SDK function's own `Options` parameter instead — the signature is the authoritative source — and key the paginatable-methods map by that name so the pagination lookup can never diverge from it. The `unknown` fallback now only triggers when the SDK signature exposes no Data type at all; the tests pinning the previously broken output are rewritten accordingly, and the #209 guard invariant (every paginatable operation has its Data type in modelNames) is preserved via `op.dataTypeName`. Closes #213 --- src/parseOperations.mts | 38 +++++++-- src/tsmorph/buildCommon.mts | 23 ++---- src/tsmorph/buildMutationHooks.mts | 10 +-- src/tsmorph/buildQueryHooks.mts | 39 ++++----- src/tsmorph/buildQueryOptions.mts | 3 +- src/tsmorph/generateFiles.mts | 8 +- src/types.mts | 8 ++ tests/inputs/digit-leading.yaml | 26 ++++++ tests/parseOperations.test.ts | 49 +++++++++-- tests/tsmorph/buildCommon.test.ts | 34 ++++++-- tests/tsmorph/buildMutationHooks.test.ts | 28 +++++-- tests/tsmorph/buildQueryHooks.test.ts | 100 +++++++++++++++-------- tests/tsmorph/buildQueryOptions.test.ts | 12 +-- 13 files changed, 253 insertions(+), 125 deletions(-) create mode 100644 tests/inputs/digit-leading.yaml diff --git a/src/parseOperations.mts b/src/parseOperations.mts index e5d8971..89da40e 100644 --- a/src/parseOperations.mts +++ b/src/parseOperations.mts @@ -1,4 +1,4 @@ -import type { Project, VariableDeclaration } from "ts-morph"; +import { Node, type Project, type VariableDeclaration } from "ts-morph"; import ts from "typescript"; import { capitalizeFirstLetter, @@ -68,8 +68,29 @@ function extractParameters( } /** - * Get paginatable methods by checking if their Data type has the pageParam in query property. + * Read the operation's Data type name from the SDK function signature. + * hey-api types every SDK function as `(options: Options) => ...`, + * and the first type argument of Options is the authoritative Data type name. + * Deriving it from the method name instead breaks for digit-leading + * operationIds, where hey-api prefixes the function (`_123NumericLead`) but + * strips the digits from the type (`NumericLeadData`) (#213). + */ +function getDataTypeNameFromSignature( + method: VariableDeclaration, +): string | undefined { + const [optionsParam] = getVariableArrowFunctionParameters(method); + const typeNode = optionsParam?.getTypeNode(); + if (!typeNode || !Node.isTypeReference(typeNode)) return undefined; + if (typeNode.getTypeName().getText() !== "Options") return undefined; + const [dataTypeArg] = typeNode.getTypeArguments(); + return dataTypeArg ? getShortType(dataTypeArg.getText()) : undefined; +} + +/** + * Get paginatable Data types by checking if they have the pageParam in their query property. * Uses TypeScript compiler API for accurate AST traversal. + * The map is keyed by the Data type name (e.g., "FindPetsData") so callers can + * look it up with the name read from the SDK signature. */ function getPaginatableMethods( project: Project, @@ -115,18 +136,13 @@ function getPaginatableMethods( ); if (pageParamNode) { - // Extract method name from Data type name (e.g., "FindPetsData" -> "findPets") - const methodName = key.slice(0, -4); // Remove "Data" suffix - // Convert first letter to lowercase - const methodNameLower = - methodName.charAt(0).toLowerCase() + methodName.slice(1); const pageParamType = pageParamNode.type?.getText( modelsFile.compilerNode, ); const resolvedType = typeChecker.getTypeAtLocation( pageParamNode.type ?? pageParamNode, ); - paginatableMethods.set(methodNameLower, { + paginatableMethods.set(key, { type: pageParamType ?? "unknown", typeKind: getPageParamTypeKind(resolvedType), }); @@ -157,12 +173,16 @@ export async function parseOperations( const sdkParams = getVariableArrowFunctionParameters(desc.method); const allParamsOptional = sdkParams.length === 0 || sdkParams[0].isOptional(); - const pageParamInfo = paginatableMethods.get(methodName); + const dataTypeName = getDataTypeNameFromSignature(desc.method); + const pageParamInfo = dataTypeName + ? paginatableMethods.get(dataTypeName) + : undefined; const isPaginatable = httpMethod === "GET" && pageParamInfo !== undefined; return { methodName, capitalizedMethodName: capitalizeFirstLetter(methodName), + dataTypeName, httpMethod, jsDoc: desc.jsDoc, isDeprecated: desc.isDeprecated, diff --git a/src/tsmorph/buildCommon.mts b/src/tsmorph/buildCommon.mts index 4b9877a..787291a 100644 --- a/src/tsmorph/buildCommon.mts +++ b/src/tsmorph/buildCommon.mts @@ -5,6 +5,7 @@ import { type VariableStatementStructure, } from "ts-morph"; import type { GenerationContext, OperationInfo } from "../types.mjs"; +import { getDataTypeName } from "./buildQueryHooks.mjs"; /** * Build the default response type alias. @@ -100,15 +101,8 @@ export function buildMutationKeyConst( * Example: export const UseFindPetsKeyFn = (clientOptions: Options = {}, queryKey?: Array) => * [useFindPetsKey, ...(queryKey ?? [clientOptions])]; */ -export function buildQueryKeyFn( - op: OperationInfo, - ctx: GenerationContext, -): VariableStatementStructure { - const dataTypeName = ctx.modelNames.includes( - `${op.capitalizedMethodName}Data`, - ) - ? `${op.capitalizedMethodName}Data` - : "unknown"; +export function buildQueryKeyFn(op: OperationInfo): VariableStatementStructure { + const dataTypeName = getDataTypeName(op); const params: string[] = []; const defaultValue = op.allParamsOptional ? " = {}" : ""; @@ -159,16 +153,17 @@ export function buildMutationKeyFn( * export type FindPaginatedPetsInfiniteClientOptions = Omit, "query"> & * { query?: Omit, "page"> }; * - * The `Data` type is always in scope here: `parseOperations` only marks - * an operation paginatable when it found the page parameter inside that very - * type, and it discovers it through the same exported declarations that become - * `ctx.modelNames`. So there is no missing-Data case to fall back on. + * The Data type is always in scope here: `parseOperations` only marks an + * operation paginatable when it resolved `dataTypeName` from the SDK signature + * and found the page parameter inside that very type, and it discovers it + * through the same exported declarations that become `ctx.modelNames`. So + * there is no missing-Data case to fall back on. */ export function buildInfiniteClientOptionsType( op: OperationInfo, ctx: GenerationContext, ): TypeAliasDeclarationStructure { - const dataTypeName = `${op.capitalizedMethodName}Data`; + const dataTypeName = getDataTypeName(op); const type = `Omit, "query"> & { query?: Omit, "${ctx.pageParam}"> }`; return { diff --git a/src/tsmorph/buildMutationHooks.mts b/src/tsmorph/buildMutationHooks.mts index 489b973..9b2bfd5 100644 --- a/src/tsmorph/buildMutationHooks.mts +++ b/src/tsmorph/buildMutationHooks.mts @@ -4,7 +4,7 @@ import { type VariableStatementStructure, } from "ts-morph"; import type { GenerationContext, OperationInfo } from "../types.mjs"; -import { SDK_CALL_ARGS } from "./buildQueryHooks.mjs"; +import { getDataTypeName, SDK_CALL_ARGS } from "./buildQueryHooks.mjs"; /** * Get the error type string based on client type. @@ -41,13 +41,7 @@ export function buildUseMutationHook( const errorType = getErrorType(op, ctx); const dataTypeDefault = `Common.${op.capitalizedMethodName}MutationResult`; - const dataTypeName = ctx.modelNames.includes( - `${op.capitalizedMethodName}Data`, - ) - ? `${op.capitalizedMethodName}Data` - : "unknown"; - - const optionsType = `Options<${dataTypeName}, true>`; + const optionsType = `Options<${getDataTypeName(op)}, true>`; const mutationFn = `clientOptions => ${op.methodName}(${SDK_CALL_ARGS}) as unknown as Promise`; diff --git a/src/tsmorph/buildQueryHooks.mts b/src/tsmorph/buildQueryHooks.mts index 24e766e..f0f4d71 100644 --- a/src/tsmorph/buildQueryHooks.mts +++ b/src/tsmorph/buildQueryHooks.mts @@ -22,15 +22,13 @@ function getErrorType(op: OperationInfo, ctx: GenerationContext): string { /** * Resolve the generated Data type name for an operation, falling back to - * unknown when the operation has no generated Data type. + * unknown when the SDK signature exposes no Data type. The name is read from + * the SDK function's own `Options` parameter rather than + * derived from the method name, which diverges for digit-leading + * operationIds (#213). */ -export function getDataTypeName( - op: OperationInfo, - ctx: GenerationContext, -): string { - return ctx.modelNames.includes(`${op.capitalizedMethodName}Data`) - ? `${op.capitalizedMethodName}Data` - : "unknown"; +export function getDataTypeName(op: OperationInfo): string { + return op.dataTypeName ?? "unknown"; } /** @@ -53,11 +51,8 @@ export function getPageParamType(op: OperationInfo): string { /** * Build the client options parameter string. */ -export function buildClientOptionsParam( - op: OperationInfo, - ctx: GenerationContext, -): string { - const dataTypeName = getDataTypeName(op, ctx); +export function buildClientOptionsParam(op: OperationInfo): string { + const dataTypeName = getDataTypeName(op); const hasParams = op.parameters.length > 0; if (!hasParams) { @@ -95,14 +90,14 @@ export function getPageType(op: OperationInfo): string { * a cast rather than `!` because generated code is linted downstream, and a * non-null assertion trips biome's noNonNullAssertion. * - * Only ever called for paginatable operations, so the `Data` type is + * Only ever called for paginatable operations, so the Data type is * guaranteed to exist — see buildInfiniteClientOptionsType for why. */ export function buildPagedQueryFn( op: OperationInfo, ctx: GenerationContext, ): string { - const dataTypeName = `${op.capitalizedMethodName}Data`; + const dataTypeName = getDataTypeName(op); const thenClause = `.then(response => response.data as ${getPageType(op)})`; const pageParamType = getPageParamType(op); // When the initial page param is omitted, the first request must send no @@ -208,7 +203,7 @@ export function buildUseQueryHook( const hookName = `use${op.capitalizedMethodName}`; const errorType = getErrorType(op, ctx); const dataTypeDefault = `Common.${op.capitalizedMethodName}DefaultResponse`; - const clientOptionsParam = buildClientOptionsParam(op, ctx); + const clientOptionsParam = buildClientOptionsParam(op); const queryFn = `({ signal }) => ${op.methodName}(${QUERY_SDK_CALL_ARGS}).then(response => response.data as TData) as TData`; @@ -239,7 +234,7 @@ export function buildUseSuspenseQueryHook( const hookName = `use${op.capitalizedMethodName}Suspense`; const errorType = getErrorType(op, ctx); const dataTypeDefault = `NonNullable`; - const clientOptionsParam = buildClientOptionsParam(op, ctx); + const clientOptionsParam = buildClientOptionsParam(op); const queryFn = `({ signal }) => ${op.methodName}(${QUERY_SDK_CALL_ARGS}).then(response => response.data as TData) as TData`; @@ -339,10 +334,7 @@ export function buildUseSuspenseInfiniteQueryHook( * ...options * }); */ -export function buildPrefetchFn( - op: OperationInfo, - ctx: GenerationContext, -): VariableStatementStructure { +export function buildPrefetchFn(op: OperationInfo): VariableStatementStructure { const fnName = `prefetchUse${op.capitalizedMethodName}`; const queryFn = `({ signal }) => ${op.methodName}(${QUERY_SDK_CALL_ARGS}).then(response => response.data)`; @@ -359,7 +351,7 @@ export function buildPrefetchFn( declarations: [ { name: fnName, - initializer: `(queryClient: QueryClient, ${buildClientOptionsParam(op, ctx)}, ${optionsParam}) => ${body}`, + initializer: `(queryClient: QueryClient, ${buildClientOptionsParam(op)}, ${optionsParam}) => ${body}`, }, ], }; @@ -428,7 +420,6 @@ export function buildPrefetchInfiniteQueryFn( */ export function buildEnsureQueryDataFn( op: OperationInfo, - ctx: GenerationContext, ): VariableStatementStructure { const fnName = `ensureUse${op.capitalizedMethodName}Data`; @@ -446,7 +437,7 @@ export function buildEnsureQueryDataFn( declarations: [ { name: fnName, - initializer: `(queryClient: QueryClient, ${buildClientOptionsParam(op, ctx)}, ${optionsParam}) => ${body}`, + initializer: `(queryClient: QueryClient, ${buildClientOptionsParam(op)}, ${optionsParam}) => ${body}`, }, ], }; diff --git a/src/tsmorph/buildQueryOptions.mts b/src/tsmorph/buildQueryOptions.mts index 98052cd..c2ce04e 100644 --- a/src/tsmorph/buildQueryOptions.mts +++ b/src/tsmorph/buildQueryOptions.mts @@ -28,10 +28,9 @@ import { */ export function buildQueryOptionsFn( op: OperationInfo, - ctx: GenerationContext, ): VariableStatementStructure { const fnName = `${op.methodName}Options`; - const clientOptionsParam = buildClientOptionsParam(op, ctx); + const clientOptionsParam = buildClientOptionsParam(op); const queryFn = `({ signal }) => ${op.methodName}(${QUERY_SDK_CALL_ARGS}).then(response => response.data)`; const body = `queryOptions({ queryKey: Common.Use${op.capitalizedMethodName}KeyFn(clientOptions, queryKey), queryFn: ${queryFn} })`; diff --git a/src/tsmorph/generateFiles.mts b/src/tsmorph/generateFiles.mts index fbaf85c..bf52ed7 100644 --- a/src/tsmorph/generateFiles.mts +++ b/src/tsmorph/generateFiles.mts @@ -93,7 +93,7 @@ function generateCommonFile( sourceFile.addTypeAlias(buildDefaultResponseType(op)); sourceFile.addTypeAlias(buildQueryResultType(op)); sourceFile.addVariableStatement(buildQueryKeyConst(op)); - sourceFile.addVariableStatement(buildQueryKeyFn(op, ctx)); + sourceFile.addVariableStatement(buildQueryKeyFn(op)); } // Add dedicated infinite query types and keys for paginatable operations @@ -170,7 +170,7 @@ function generateQueryOptionsFile( const getOperations = operations.filter((op) => op.httpMethod === "GET"); for (const op of getOperations) { - sourceFile.addVariableStatement(buildQueryOptionsFn(op, ctx)); + sourceFile.addVariableStatement(buildQueryOptionsFn(op)); } // Add infiniteQueryOptions factories @@ -264,7 +264,7 @@ function generatePrefetchFile( // Add prefetch functions for (const op of getOperations) { - sourceFile.addVariableStatement(buildPrefetchFn(op, ctx)); + sourceFile.addVariableStatement(buildPrefetchFn(op)); } // Add prefetchInfiniteQuery functions @@ -297,7 +297,7 @@ function generateEnsureQueryDataFile( // Add ensureQueryData functions for (const op of getOperations) { - sourceFile.addVariableStatement(buildEnsureQueryDataFn(op, ctx)); + sourceFile.addVariableStatement(buildEnsureQueryDataFn(op)); } return sourceFile.getFullText(); diff --git a/src/types.mts b/src/types.mts index f10457a..805a3d8 100644 --- a/src/types.mts +++ b/src/types.mts @@ -7,6 +7,14 @@ export interface OperationInfo { methodName: string; /** Capitalized method name (e.g., "FindPets") */ capitalizedMethodName: string; + /** + * Generated Data type name read from the SDK function's own + * `Options` signature (e.g., "FindPetsData"). + * Not derivable from the method name: for digit-leading operationIds + * hey-api prefixes the function but strips the digits from the type (#213). + * Undefined when the SDK signature exposes no Data type. + */ + dataTypeName?: string; /** HTTP method (e.g., "GET", "POST", "PUT", "PATCH", "DELETE") */ httpMethod: string; /** JSDoc comment string (if present) */ diff --git a/tests/inputs/digit-leading.yaml b/tests/inputs/digit-leading.yaml new file mode 100644 index 0000000..ca1de1a --- /dev/null +++ b/tests/inputs/digit-leading.yaml @@ -0,0 +1,26 @@ +openapi: "3.0.0" +info: + version: 1.0.0 + title: Digit-leading operationId + description: > + Regression spec for #213. hey-api names the SDK function `_123NumericLead` + (identifiers cannot start with a digit) but names the Data type + `NumericLeadData` (digits are dropped), so the two names diverge. +paths: + /d: + get: + operationId: 123numericLead + parameters: + - name: page + in: query + schema: + type: integer + responses: + "200": + description: ok + content: + application/json: + schema: + type: array + items: + type: string diff --git a/tests/parseOperations.test.ts b/tests/parseOperations.test.ts index 1eee9f7..a1a8283 100644 --- a/tests/parseOperations.test.ts +++ b/tests/parseOperations.test.ts @@ -26,6 +26,8 @@ describe("parseOperations", () => { expect(findPets).toBeDefined(); expect(findPets?.httpMethod).toBe("GET"); expect(findPets?.capitalizedMethodName).toBe("FindPets"); + // Read from the SDK signature's Options type argument + expect(findPets?.dataTypeName).toBe("FindPetsData"); }); it("should parse POST operations", async () => { @@ -70,13 +72,14 @@ describe("parseOperations", () => { expect(findPaginatedPets?.pageParamTypeKind).toBe("number"); }); - // The infinite-query builders spell the Data type as `${capitalizedMethodName}Data` - // with no fallback, because `getPaginatableMethods` only marks an operation - // paginatable after finding the page parameter inside that very type, and it reads - // the same exported declarations that become `modelNames`. That coupling is what - // makes the fallback unnecessary, so it is pinned here rather than left implicit: - // resolving Data types by any other route must keep this invariant or the - // generated code will reference a type that does not exist. + // The infinite-query builders spell the Data type as `op.dataTypeName` with no + // fallback, because an operation is only marked paginatable after the name read + // from its SDK signature matched a `*Data` type whose query property contains the + // page parameter — and that type comes from the same exported declarations that + // become `modelNames`. That coupling is what makes the fallback unnecessary, so it + // is pinned here rather than left implicit: resolving Data types by any other + // route must keep this invariant or the generated code will reference a type that + // does not exist. it("should expose a Data type in modelNames for every paginatable operation", async () => { const project = new Project({ skipAddingFilesFromTsConfig: true }); project.addSourceFilesAtPaths(`${outputPath(fileName)}/**/*`); @@ -96,7 +99,8 @@ describe("parseOperations", () => { expect(paginatable.length).toBeGreaterThan(0); for (const op of paginatable) { - expect(ctx.modelNames).toContain(`${op.capitalizedMethodName}Data`); + expect(op.dataTypeName).toBeDefined(); + expect(ctx.modelNames).toContain(op.dataTypeName); } }); @@ -248,6 +252,35 @@ describe("parseOperations", () => { }); }); +describe("parseOperations - digit-leading operationId (#213)", () => { + const digitFileName = "parseOperations-digit-leading"; + + beforeAll( + async () => await generateTSClients(digitFileName, "digit-leading.yaml"), + ); + afterAll(async () => await cleanOutputs(digitFileName)); + + // hey-api names the SDK function `_123NumericLead` but the Data type + // `NumericLeadData`, so deriving the type from the method name misses. + // The name must come from the SDK signature for pagination to be detected + // and for the generated output to compile. + it("should resolve the Data type from the SDK signature and keep pagination", async () => { + const project = new Project({ skipAddingFilesFromTsConfig: true }); + project.addSourceFilesAtPaths(`${outputPath(digitFileName)}/**/*`); + + const operations = await parseOperations(project, "page"); + const op = operations.find((o) => o.httpMethod === "GET"); + + expect(op).toBeDefined(); + expect(op?.methodName).toBe("_123NumericLead"); + expect(op?.dataTypeName).toBe("NumericLeadData"); + // The two names diverge — this is exactly what #213 is about + expect(op?.dataTypeName).not.toBe(`${op?.capitalizedMethodName}Data`); + expect(op?.isPaginatable).toBe(true); + expect(op?.pageParamTypeKind).toBe("number"); + }); +}); + describe("parseOperations - string pagination", () => { const stringFileName = "parseOperations-string-pagination"; diff --git a/tests/tsmorph/buildCommon.test.ts b/tests/tsmorph/buildCommon.test.ts index 4508631..740831c 100644 --- a/tests/tsmorph/buildCommon.test.ts +++ b/tests/tsmorph/buildCommon.test.ts @@ -17,6 +17,7 @@ import type { GenerationContext, OperationInfo } from "../../src/types.mjs"; const mockOperation: OperationInfo = { methodName: "findPets", capitalizedMethodName: "FindPets", + dataTypeName: "FindPetsData", httpMethod: "GET", isDeprecated: false, parameters: [{ name: "limit", typeName: "number", optional: true }], @@ -27,6 +28,7 @@ const mockOperation: OperationInfo = { const mockMutationOperation: OperationInfo = { methodName: "addPet", capitalizedMethodName: "AddPet", + dataTypeName: "AddPetData", httpMethod: "POST", isDeprecated: false, parameters: [{ name: "body", typeName: "NewPet", optional: false }], @@ -37,6 +39,7 @@ const mockMutationOperation: OperationInfo = { const mockPaginatableOperation: OperationInfo = { methodName: "findPaginatedPets", capitalizedMethodName: "FindPaginatedPets", + dataTypeName: "FindPaginatedPetsData", httpMethod: "GET", isDeprecated: false, parameters: [{ name: "page", typeName: "number", optional: true }], @@ -136,7 +139,7 @@ describe("buildCommon", () => { describe("buildQueryKeyFn", () => { it("should build query key function with parameters", () => { - const result = buildQueryKeyFn(mockOperation, mockContext); + const result = buildQueryKeyFn(mockOperation); expect(result.kind).toBe(StructureKind.VariableStatement); expect(result.isExported).toBe(true); @@ -157,15 +160,12 @@ describe("buildCommon", () => { ...mockOperation, methodName: "getPetById", capitalizedMethodName: "GetPetById", + dataTypeName: "GetPetByIdData", parameters: [{ name: "id", typeName: "number", optional: false }], allParamsOptional: false, }; - const ctx: GenerationContext = { - ...mockContext, - modelNames: [...mockContext.modelNames, "GetPetByIdData"], - }; - const result = buildQueryKeyFn(op, ctx); + const result = buildQueryKeyFn(op); const initializer = result.declarations[0].initializer as string; expect(initializer).toContain( @@ -174,18 +174,36 @@ describe("buildCommon", () => { expect(initializer).not.toContain("= {}"); }); - it("should use unknown for missing data type", () => { + it("should fall back to unknown when the SDK signature exposes no Data type", () => { const op: OperationInfo = { ...mockOperation, methodName: "unknownMethod", capitalizedMethodName: "UnknownMethod", + dataTypeName: undefined, }; - const result = buildQueryKeyFn(op, mockContext); + const result = buildQueryKeyFn(op); const initializer = result.declarations[0].initializer as string; expect(initializer).toContain("Options"); }); + + it("should use the signature Data type for digit-leading operationIds (#213)", () => { + const op: OperationInfo = { + ...mockOperation, + methodName: "_123NumericLead", + capitalizedMethodName: "_123NumericLead", + dataTypeName: "NumericLeadData", + }; + + const result = buildQueryKeyFn(op); + const initializer = result.declarations[0].initializer as string; + + expect(initializer).toContain( + "clientOptions: Options", + ); + expect(initializer).not.toContain("_123NumericLeadData"); + }); }); describe("buildMutationKeyFn", () => { diff --git a/tests/tsmorph/buildMutationHooks.test.ts b/tests/tsmorph/buildMutationHooks.test.ts index 1566a8b..9435159 100644 --- a/tests/tsmorph/buildMutationHooks.test.ts +++ b/tests/tsmorph/buildMutationHooks.test.ts @@ -6,6 +6,7 @@ import type { GenerationContext, OperationInfo } from "../../src/types.mjs"; const mockPostOperation: OperationInfo = { methodName: "addPet", capitalizedMethodName: "AddPet", + dataTypeName: "AddPetData", httpMethod: "POST", isDeprecated: false, parameters: [{ name: "body", typeName: "NewPet", optional: false }], @@ -16,6 +17,7 @@ const mockPostOperation: OperationInfo = { const mockDeleteOperation: OperationInfo = { methodName: "deletePet", capitalizedMethodName: "DeletePet", + dataTypeName: "DeletePetData", httpMethod: "DELETE", isDeprecated: false, parameters: [{ name: "id", typeName: "number", optional: false }], @@ -26,6 +28,7 @@ const mockDeleteOperation: OperationInfo = { const mockPutOperation: OperationInfo = { methodName: "updatePet", capitalizedMethodName: "UpdatePet", + dataTypeName: "UpdatePetData", httpMethod: "PUT", isDeprecated: false, parameters: [ @@ -39,6 +42,7 @@ const mockPutOperation: OperationInfo = { const mockPatchOperation: OperationInfo = { methodName: "patchPet", capitalizedMethodName: "PatchPet", + dataTypeName: "PatchPetData", httpMethod: "PATCH", isDeprecated: false, parameters: [{ name: "body", typeName: "Partial", optional: true }], @@ -174,23 +178,35 @@ describe("buildMutationHooks", () => { ); }); - it("should use unknown for missing data type", () => { + it("should fall back to unknown when the SDK signature exposes no Data type", () => { const opWithoutData: OperationInfo = { ...mockPostOperation, methodName: "unknownMutation", capitalizedMethodName: "UnknownMutation", - }; - const ctx: GenerationContext = { - ...mockFetchContext, - modelNames: ["Pet", "NewPet"], // no UnknownMutationData + dataTypeName: undefined, }; - const result = buildUseMutationHook(opWithoutData, ctx); + const result = buildUseMutationHook(opWithoutData, mockFetchContext); const initializer = result.declarations[0].initializer as string; expect(initializer).toContain("Options"); }); + it("should use the signature Data type for digit-leading operationIds (#213)", () => { + const op: OperationInfo = { + ...mockPostOperation, + methodName: "_123CreateThing", + capitalizedMethodName: "_123CreateThing", + dataTypeName: "CreateThingData", + }; + + const result = buildUseMutationHook(op, mockFetchContext); + const initializer = result.declarations[0].initializer as string; + + expect(initializer).toContain("Options"); + expect(initializer).not.toContain("_123CreateThingData"); + }); + it("should spread options at the end", () => { const result = buildUseMutationHook(mockPostOperation, mockFetchContext); const initializer = result.declarations[0].initializer as string; diff --git a/tests/tsmorph/buildQueryHooks.test.ts b/tests/tsmorph/buildQueryHooks.test.ts index a8afcb8..4eaf41c 100644 --- a/tests/tsmorph/buildQueryHooks.test.ts +++ b/tests/tsmorph/buildQueryHooks.test.ts @@ -17,6 +17,7 @@ import type { GenerationContext, OperationInfo } from "../../src/types.mjs"; const mockOperation: OperationInfo = { methodName: "findPets", capitalizedMethodName: "FindPets", + dataTypeName: "FindPetsData", httpMethod: "GET", isDeprecated: false, parameters: [{ name: "limit", typeName: "number", optional: true }], @@ -27,6 +28,7 @@ const mockOperation: OperationInfo = { const mockPaginatableOperation: OperationInfo = { methodName: "findPaginatedPets", capitalizedMethodName: "FindPaginatedPets", + dataTypeName: "FindPaginatedPetsData", httpMethod: "GET", isDeprecated: false, parameters: [{ name: "page", typeName: "number", optional: true }], @@ -45,6 +47,7 @@ const mockStringPaginatableOperation: OperationInfo = { const mockRequiredParamsOperation: OperationInfo = { methodName: "findPetById", capitalizedMethodName: "FindPetById", + dataTypeName: "FindPetByIdData", httpMethod: "GET", isDeprecated: false, parameters: [{ name: "id", typeName: "number", optional: false }], @@ -52,7 +55,8 @@ const mockRequiredParamsOperation: OperationInfo = { isPaginatable: false, }; -const mockNoParamsOperation: OperationInfo = { +// No dataTypeName: the SDK signature exposed no Options parameter +const mockNoDataTypeOperation: OperationInfo = { methodName: "getStatus", capitalizedMethodName: "GetStatus", httpMethod: "GET", @@ -62,6 +66,21 @@ const mockNoParamsOperation: OperationInfo = { isPaginatable: false, }; +// hey-api prefixes a digit-leading operationId in the function name but strips +// the digits from the Data type, so the two names diverge (#213) +const mockDigitLeadingOperation: OperationInfo = { + methodName: "_123NumericLead", + capitalizedMethodName: "_123NumericLead", + dataTypeName: "NumericLeadData", + httpMethod: "GET", + isDeprecated: false, + parameters: [{ name: "page", typeName: "number", optional: true }], + allParamsOptional: true, + isPaginatable: true, + pageParamType: "number", + pageParamTypeKind: "number", +}; + const mockFetchContext: GenerationContext = { client: "@hey-api/client-fetch", modelNames: [ @@ -85,11 +104,6 @@ const mockAxiosContext: GenerationContext = { client: "@hey-api/client-axios", }; -const mockUnknownDataContext: GenerationContext = { - ...mockFetchContext, - modelNames: [], -}; - describe("buildQueryHooks", () => { describe("infinite query helpers", () => { it("should omit the default value when a paginatable op has required params", () => { @@ -163,10 +177,10 @@ describe("buildQueryHooks", () => { expect(initializer).not.toContain("= {}"); }); - it("should handle operations without params and unknown data type", () => { + it("should fall back to unknown when the SDK signature exposes no Data type", () => { const result = buildUseQueryHook( - mockNoParamsOperation, - mockUnknownDataContext, + mockNoDataTypeOperation, + mockFetchContext, ); const initializer = result.declarations[0].initializer as string; @@ -177,6 +191,20 @@ describe("buildQueryHooks", () => { "getStatus({ ...clientOptions, signal, throwOnError: true })", ); }); + + it("should use the signature Data type for digit-leading operationIds (#213)", () => { + const result = buildUseQueryHook( + mockDigitLeadingOperation, + mockFetchContext, + ); + const initializer = result.declarations[0].initializer as string; + + expect(initializer).toContain( + "clientOptions: Options = {}", + ); + expect(initializer).not.toContain("Options"); + expect(initializer).not.toContain("_123NumericLeadData"); + }); }); describe("buildUseSuspenseQueryHook", () => { @@ -202,10 +230,10 @@ describe("buildQueryHooks", () => { ); }); - it("should handle operations without params and unknown data type", () => { + it("should fall back to unknown when the SDK signature exposes no Data type", () => { const result = buildUseSuspenseQueryHook( - mockNoParamsOperation, - mockUnknownDataContext, + mockNoDataTypeOperation, + mockFetchContext, ); const initializer = result.declarations[0].initializer as string; @@ -244,6 +272,18 @@ describe("buildQueryHooks", () => { expect(initializer).toContain("initialPageParam: 1"); }); + it("should cast to the signature Data type for digit-leading operationIds (#213)", () => { + const result = buildUseInfiniteQueryHook( + mockDigitLeadingOperation, + mockFetchContext, + ); + + expect(result).not.toBeNull(); + const initializer = result?.declarations[0].initializer as string; + expect(initializer).toContain("as Options"); + expect(initializer).not.toContain("_123NumericLeadData"); + }); + it("should make initialPageParam and getNextPageParam optional overrides", () => { const result = buildUseInfiniteQueryHook( mockPaginatableOperation, @@ -314,7 +354,7 @@ describe("buildQueryHooks", () => { describe("buildPrefetchFn", () => { it("should build prefetch function", () => { - const result = buildPrefetchFn(mockOperation, mockFetchContext); + const result = buildPrefetchFn(mockOperation); expect(result.declarations[0].name).toBe("prefetchUseFindPets"); @@ -333,17 +373,14 @@ describe("buildQueryHooks", () => { }); it("should include default value for optional params", () => { - const result = buildPrefetchFn(mockOperation, mockFetchContext); + const result = buildPrefetchFn(mockOperation); const initializer = result.declarations[0].initializer as string; expect(initializer).toContain("= {}"); }); it("should not include default value for required params", () => { - const result = buildPrefetchFn( - mockRequiredParamsOperation, - mockFetchContext, - ); + const result = buildPrefetchFn(mockRequiredParamsOperation); const initializer = result.declarations[0].initializer as string; expect(initializer).toContain( @@ -355,7 +392,7 @@ describe("buildQueryHooks", () => { }); it("should accept fetch query options (#157)", () => { - const result = buildPrefetchFn(mockOperation, mockFetchContext); + const result = buildPrefetchFn(mockOperation); const initializer = result.declarations[0].initializer as string; expect(initializer).toContain( @@ -364,11 +401,8 @@ describe("buildQueryHooks", () => { expect(initializer).toMatch(/\.\.\.options\s*\}\)/); }); - it("should handle operations without params and unknown data type", () => { - const result = buildPrefetchFn( - mockNoParamsOperation, - mockUnknownDataContext, - ); + it("should fall back to unknown when the SDK signature exposes no Data type", () => { + const result = buildPrefetchFn(mockNoDataTypeOperation); const initializer = result.declarations[0].initializer as string; expect(initializer).toContain( @@ -382,7 +416,7 @@ describe("buildQueryHooks", () => { describe("buildEnsureQueryDataFn", () => { it("should build ensureQueryData function", () => { - const result = buildEnsureQueryDataFn(mockOperation, mockFetchContext); + const result = buildEnsureQueryDataFn(mockOperation); expect(result.declarations[0].name).toBe("ensureUseFindPetsData"); @@ -393,11 +427,8 @@ describe("buildQueryHooks", () => { }); it("should be similar to prefetch but use ensureQueryData", () => { - const prefetchResult = buildPrefetchFn(mockOperation, mockFetchContext); - const ensureResult = buildEnsureQueryDataFn( - mockOperation, - mockFetchContext, - ); + const prefetchResult = buildPrefetchFn(mockOperation); + const ensureResult = buildEnsureQueryDataFn(mockOperation); const prefetchInit = prefetchResult.declarations[0].initializer as string; const ensureInit = ensureResult.declarations[0].initializer as string; @@ -407,11 +438,8 @@ describe("buildQueryHooks", () => { expect(ensureInit).not.toContain("prefetchQuery"); }); - it("should handle operations without params and unknown data type", () => { - const result = buildEnsureQueryDataFn( - mockNoParamsOperation, - mockUnknownDataContext, - ); + it("should fall back to unknown when the SDK signature exposes no Data type", () => { + const result = buildEnsureQueryDataFn(mockNoDataTypeOperation); const initializer = result.declarations[0].initializer as string; expect(initializer).toContain( @@ -423,7 +451,7 @@ describe("buildQueryHooks", () => { }); it("should accept ensure query data options (#157)", () => { - const result = buildEnsureQueryDataFn(mockOperation, mockFetchContext); + const result = buildEnsureQueryDataFn(mockOperation); const initializer = result.declarations[0].initializer as string; expect(initializer).toContain( diff --git a/tests/tsmorph/buildQueryOptions.test.ts b/tests/tsmorph/buildQueryOptions.test.ts index bc2a4e2..d2cb10e 100644 --- a/tests/tsmorph/buildQueryOptions.test.ts +++ b/tests/tsmorph/buildQueryOptions.test.ts @@ -9,6 +9,7 @@ import type { GenerationContext, OperationInfo } from "../../src/types.mjs"; const mockOperation: OperationInfo = { methodName: "findPets", capitalizedMethodName: "FindPets", + dataTypeName: "FindPetsData", httpMethod: "GET", isDeprecated: false, parameters: [{ name: "limit", typeName: "number", optional: true }], @@ -19,6 +20,7 @@ const mockOperation: OperationInfo = { const mockPaginatableOperation: OperationInfo = { methodName: "findPaginatedPets", capitalizedMethodName: "FindPaginatedPets", + dataTypeName: "FindPaginatedPetsData", httpMethod: "GET", isDeprecated: false, parameters: [{ name: "page", typeName: "number", optional: true }], @@ -37,6 +39,7 @@ const mockStringPaginatableOperation: OperationInfo = { const mockRequiredParamsOperation: OperationInfo = { methodName: "findPetById", capitalizedMethodName: "FindPetById", + dataTypeName: "FindPetByIdData", httpMethod: "GET", isDeprecated: false, parameters: [{ name: "id", typeName: "number", optional: false }], @@ -62,7 +65,7 @@ const mockContext: GenerationContext = { describe("buildQueryOptions", () => { describe("buildQueryOptionsFn", () => { it("should build a queryOptions factory", () => { - const result = buildQueryOptionsFn(mockOperation, mockContext); + const result = buildQueryOptionsFn(mockOperation); expect(result.kind).toBe(StructureKind.VariableStatement); expect(result.isExported).toBe(true); @@ -82,10 +85,7 @@ describe("buildQueryOptions", () => { }); it("should not add a default value when the operation has required params", () => { - const result = buildQueryOptionsFn( - mockRequiredParamsOperation, - mockContext, - ); + const result = buildQueryOptionsFn(mockRequiredParamsOperation); const initializer = result.declarations[0].initializer as string; expect(initializer).toContain( @@ -101,7 +101,7 @@ describe("buildQueryOptions", () => { ...mockOperation, jsDoc: "/**\n * Returns all pets\n */", }; - const result = buildQueryOptionsFn(op, mockContext); + const result = buildQueryOptionsFn(op); expect(result.leadingTrivia).toBe("/**\n * Returns all pets\n */\n"); }); From b2b2356b7d3da084a3161b3346e69d9425024817 Mon Sep 17 00:00:00 2001 From: Urata Daiki <7nohe@users.noreply.github.com> Date: Tue, 11 Aug 2026 23:00:49 +0900 Subject: [PATCH 2/5] refactor: consolidate operation type-name resolution after review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Apply cleanup findings from the parallel review pass: - Move getDataTypeName to a neutral operationNames.mts module and consolidate the two duplicated getErrorType copies there, removing the buildCommon -> buildQueryHooks import edge. - Derive the Error type stem from dataTypeName instead of the method name — the same #213 divergence applied to hey-api's Error types, silently degrading TError to unknown for digit-leading operationIds. - Pass the already-computed options parameter into getDataTypeNameFromSignature instead of re-walking the arrow function, and drop the inert getShortType call on written type-argument text. - State the naming rule once on OperationInfo.dataTypeName (hey-api-owned names are read from the signature; self-minted names may derive from the method name) and trim the comment blocks that restated it. - Tests: build the digit-leading mock via spread, drop modelNames Data entries and assertions made inert by the refactor, and extend the digit-leading fixture with an error response and a POST operation to pin the Error-type stem rule end to end. --- src/parseOperations.mts | 24 ++++++++------- src/tsmorph/buildCommon.mts | 8 +---- src/tsmorph/buildMutationHooks.mts | 18 ++---------- src/tsmorph/buildQueryHooks.mts | 30 +------------------ src/tsmorph/operationNames.mts | 35 ++++++++++++++++++++++ src/types.mts | 6 ++++ tests/inputs/digit-leading.yaml | 34 ++++++++++++++++++++++ tests/parseOperations.test.ts | 37 +++++++++++++++++------- tests/tsmorph/buildCommon.test.ts | 9 ++---- tests/tsmorph/buildMutationHooks.test.ts | 13 +++++---- tests/tsmorph/buildQueryHooks.test.ts | 25 +++++----------- tests/tsmorph/buildQueryOptions.test.ts | 7 +---- 12 files changed, 136 insertions(+), 110 deletions(-) create mode 100644 src/tsmorph/operationNames.mts diff --git a/src/parseOperations.mts b/src/parseOperations.mts index 89da40e..277842a 100644 --- a/src/parseOperations.mts +++ b/src/parseOperations.mts @@ -1,4 +1,9 @@ -import { Node, type Project, type VariableDeclaration } from "ts-morph"; +import { + Node, + type ParameterDeclaration, + type Project, + type VariableDeclaration, +} from "ts-morph"; import ts from "typescript"; import { capitalizeFirstLetter, @@ -68,22 +73,19 @@ function extractParameters( } /** - * Read the operation's Data type name from the SDK function signature. - * hey-api types every SDK function as `(options: Options) => ...`, - * and the first type argument of Options is the authoritative Data type name. - * Deriving it from the method name instead breaks for digit-leading - * operationIds, where hey-api prefixes the function (`_123NumericLead`) but - * strips the digits from the type (`NumericLeadData`) (#213). + * Read the operation's Data type name from the first type argument of the + * SDK options parameter (`Options`). + * See OperationInfo.dataTypeName for why the name is read from the signature + * instead of derived from the method name (#213). */ function getDataTypeNameFromSignature( - method: VariableDeclaration, + optionsParam: ParameterDeclaration | undefined, ): string | undefined { - const [optionsParam] = getVariableArrowFunctionParameters(method); const typeNode = optionsParam?.getTypeNode(); if (!typeNode || !Node.isTypeReference(typeNode)) return undefined; if (typeNode.getTypeName().getText() !== "Options") return undefined; const [dataTypeArg] = typeNode.getTypeArguments(); - return dataTypeArg ? getShortType(dataTypeArg.getText()) : undefined; + return dataTypeArg?.getText(); } /** @@ -173,7 +175,7 @@ export async function parseOperations( const sdkParams = getVariableArrowFunctionParameters(desc.method); const allParamsOptional = sdkParams.length === 0 || sdkParams[0].isOptional(); - const dataTypeName = getDataTypeNameFromSignature(desc.method); + const dataTypeName = getDataTypeNameFromSignature(sdkParams[0]); const pageParamInfo = dataTypeName ? paginatableMethods.get(dataTypeName) : undefined; diff --git a/src/tsmorph/buildCommon.mts b/src/tsmorph/buildCommon.mts index 787291a..2539f8f 100644 --- a/src/tsmorph/buildCommon.mts +++ b/src/tsmorph/buildCommon.mts @@ -5,7 +5,7 @@ import { type VariableStatementStructure, } from "ts-morph"; import type { GenerationContext, OperationInfo } from "../types.mjs"; -import { getDataTypeName } from "./buildQueryHooks.mjs"; +import { getDataTypeName } from "./operationNames.mjs"; /** * Build the default response type alias. @@ -152,12 +152,6 @@ export function buildMutationKeyFn( * Example: * export type FindPaginatedPetsInfiniteClientOptions = Omit, "query"> & * { query?: Omit, "page"> }; - * - * The Data type is always in scope here: `parseOperations` only marks an - * operation paginatable when it resolved `dataTypeName` from the SDK signature - * and found the page parameter inside that very type, and it discovers it - * through the same exported declarations that become `ctx.modelNames`. So - * there is no missing-Data case to fall back on. */ export function buildInfiniteClientOptionsType( op: OperationInfo, diff --git a/src/tsmorph/buildMutationHooks.mts b/src/tsmorph/buildMutationHooks.mts index 9b2bfd5..b867e8c 100644 --- a/src/tsmorph/buildMutationHooks.mts +++ b/src/tsmorph/buildMutationHooks.mts @@ -4,22 +4,8 @@ import { type VariableStatementStructure, } from "ts-morph"; import type { GenerationContext, OperationInfo } from "../types.mjs"; -import { getDataTypeName, SDK_CALL_ARGS } from "./buildQueryHooks.mjs"; - -/** - * Get the error type string based on client type. - */ -function getErrorType(op: OperationInfo, ctx: GenerationContext): string { - const errorTypeName = `${op.capitalizedMethodName}Error`; - // Operations without error responses have no generated Error type - const errorType = ctx.modelNames.includes(errorTypeName) - ? errorTypeName - : "unknown"; - if (ctx.client === "@hey-api/client-axios") { - return `AxiosError<${errorType}>`; - } - return errorType; -} +import { SDK_CALL_ARGS } from "./buildQueryHooks.mjs"; +import { getDataTypeName, getErrorType } from "./operationNames.mjs"; /** * Build useMutation hook. diff --git a/src/tsmorph/buildQueryHooks.mts b/src/tsmorph/buildQueryHooks.mts index f0f4d71..65f96e2 100644 --- a/src/tsmorph/buildQueryHooks.mts +++ b/src/tsmorph/buildQueryHooks.mts @@ -4,32 +4,7 @@ import { type VariableStatementStructure, } from "ts-morph"; import type { GenerationContext, OperationInfo } from "../types.mjs"; - -/** - * Get the error type string based on client type. - */ -function getErrorType(op: OperationInfo, ctx: GenerationContext): string { - const errorTypeName = `${op.capitalizedMethodName}Error`; - // Operations without error responses have no generated Error type - const errorType = ctx.modelNames.includes(errorTypeName) - ? errorTypeName - : "unknown"; - if (ctx.client === "@hey-api/client-axios") { - return `AxiosError<${errorType}>`; - } - return errorType; -} - -/** - * Resolve the generated Data type name for an operation, falling back to - * unknown when the SDK signature exposes no Data type. The name is read from - * the SDK function's own `Options` parameter rather than - * derived from the method name, which diverges for digit-leading - * operationIds (#213). - */ -export function getDataTypeName(op: OperationInfo): string { - return op.dataTypeName ?? "unknown"; -} +import { getDataTypeName, getErrorType } from "./operationNames.mjs"; /** * SDK call arguments shared by every generated queryFn/mutationFn. @@ -89,9 +64,6 @@ export function getPageType(op: OperationInfo): string { * TQueryFnData the infinite options are instantiated with (#203). Spelled as * a cast rather than `!` because generated code is linted downstream, and a * non-null assertion trips biome's noNonNullAssertion. - * - * Only ever called for paginatable operations, so the Data type is - * guaranteed to exist — see buildInfiniteClientOptionsType for why. */ export function buildPagedQueryFn( op: OperationInfo, diff --git a/src/tsmorph/operationNames.mts b/src/tsmorph/operationNames.mts new file mode 100644 index 0000000..5325ff9 --- /dev/null +++ b/src/tsmorph/operationNames.mts @@ -0,0 +1,35 @@ +import type { GenerationContext, OperationInfo } from "../types.mjs"; + +/** + * Resolve the generated Data type name for an operation, falling back to + * unknown when the SDK signature exposes no Data type. + * See OperationInfo.dataTypeName for why the name is read from the signature + * instead of derived from the method name (#213). + */ +export function getDataTypeName(op: OperationInfo): string { + return op.dataTypeName ?? "unknown"; +} + +/** + * Get the error type string based on client type. + * The Error type shares its stem with the Data type — hey-api mints both from + * the operationId (see OperationInfo.dataTypeName, #213) — so the stem comes + * from `dataTypeName` rather than the method name. The modelNames probe stays + * because operations without error responses have no generated Error type. + */ +export function getErrorType( + op: OperationInfo, + ctx: GenerationContext, +): string { + const stem = op.dataTypeName + ? op.dataTypeName.replace(/Data$/, "") + : op.capitalizedMethodName; + const errorTypeName = `${stem}Error`; + const errorType = ctx.modelNames.includes(errorTypeName) + ? errorTypeName + : "unknown"; + if (ctx.client === "@hey-api/client-axios") { + return `AxiosError<${errorType}>`; + } + return errorType; +} diff --git a/src/types.mts b/src/types.mts index 805a3d8..acda82c 100644 --- a/src/types.mts +++ b/src/types.mts @@ -13,6 +13,12 @@ export interface OperationInfo { * Not derivable from the method name: for digit-leading operationIds * hey-api prefixes the function but strips the digits from the type (#213). * Undefined when the SDK signature exposes no Data type. + * + * The rule this encodes: names hey-api owns (Data, Error) must be read from + * the SDK signature or share this field's stem; names this codegen mints + * itself (DefaultResponse, MutationResult, key fns) may be derived from + * capitalizedMethodName because they anchor back to the SDK via + * `typeof methodName`. */ dataTypeName?: string; /** HTTP method (e.g., "GET", "POST", "PUT", "PATCH", "DELETE") */ diff --git a/tests/inputs/digit-leading.yaml b/tests/inputs/digit-leading.yaml index ca1de1a..374005c 100644 --- a/tests/inputs/digit-leading.yaml +++ b/tests/inputs/digit-leading.yaml @@ -24,3 +24,37 @@ paths: type: array items: type: string + "400": + description: bad request + content: + application/json: + schema: + $ref: "#/components/schemas/ApiError" + post: + operationId: 456createThing + requestBody: + content: + application/json: + schema: + type: object + properties: + name: + type: string + responses: + "201": + description: created + "400": + description: bad request + content: + application/json: + schema: + $ref: "#/components/schemas/ApiError" +components: + schemas: + ApiError: + type: object + required: + - message + properties: + message: + type: string diff --git a/tests/parseOperations.test.ts b/tests/parseOperations.test.ts index a1a8283..7ea2c15 100644 --- a/tests/parseOperations.test.ts +++ b/tests/parseOperations.test.ts @@ -72,14 +72,8 @@ describe("parseOperations", () => { expect(findPaginatedPets?.pageParamTypeKind).toBe("number"); }); - // The infinite-query builders spell the Data type as `op.dataTypeName` with no - // fallback, because an operation is only marked paginatable after the name read - // from its SDK signature matched a `*Data` type whose query property contains the - // page parameter — and that type comes from the same exported declarations that - // become `modelNames`. That coupling is what makes the fallback unnecessary, so it - // is pinned here rather than left implicit: resolving Data types by any other - // route must keep this invariant or the generated code will reference a type that - // does not exist. + // Pins the invariant the infinite-query builders rely on: every paginatable + // operation has a dataTypeName that exists among the exported model names. it("should expose a Data type in modelNames for every paginatable operation", async () => { const project = new Project({ skipAddingFilesFromTsConfig: true }); project.addSourceFilesAtPaths(`${outputPath(fileName)}/**/*`); @@ -99,7 +93,6 @@ describe("parseOperations", () => { expect(paginatable.length).toBeGreaterThan(0); for (const op of paginatable) { - expect(op.dataTypeName).toBeDefined(); expect(ctx.modelNames).toContain(op.dataTypeName); } }); @@ -274,10 +267,32 @@ describe("parseOperations - digit-leading operationId (#213)", () => { expect(op).toBeDefined(); expect(op?.methodName).toBe("_123NumericLead"); expect(op?.dataTypeName).toBe("NumericLeadData"); - // The two names diverge — this is exactly what #213 is about - expect(op?.dataTypeName).not.toBe(`${op?.capitalizedMethodName}Data`); expect(op?.isPaginatable).toBe(true); expect(op?.pageParamTypeKind).toBe("number"); + + const postOp = operations.find((o) => o.httpMethod === "POST"); + expect(postOp?.methodName).toBe("_456CreateThing"); + expect(postOp?.dataTypeName).toBe("CreateThingData"); + }); + + // Pins the naming assumption getErrorType's stem derivation relies on: + // hey-api mints the Error type from the same stem as the Data type. + it("should generate Error types sharing the Data type stem", async () => { + const project = new Project({ skipAddingFilesFromTsConfig: true }); + project.addSourceFilesAtPaths(`${outputPath(digitFileName)}/**/*`); + + const ctx = buildGenerationContext( + project, + "@hey-api/client-fetch", + "page", + "nextPage", + "1", + false, + "1.0.0", + ); + + expect(ctx.modelNames).toContain("NumericLeadError"); + expect(ctx.modelNames).toContain("CreateThingError"); }); }); diff --git a/tests/tsmorph/buildCommon.test.ts b/tests/tsmorph/buildCommon.test.ts index 740831c..d33f975 100644 --- a/tests/tsmorph/buildCommon.test.ts +++ b/tests/tsmorph/buildCommon.test.ts @@ -49,7 +49,7 @@ const mockPaginatableOperation: OperationInfo = { const mockContext: GenerationContext = { client: "@hey-api/client-fetch", - modelNames: ["Pet", "NewPet", "FindPetsData", "AddPetData"], + modelNames: ["Pet", "NewPet"], serviceNames: ["findPets", "addPet"], pageParam: "page", nextPageParam: "nextPage", @@ -202,7 +202,6 @@ describe("buildCommon", () => { expect(initializer).toContain( "clientOptions: Options", ); - expect(initializer).not.toContain("_123NumericLeadData"); }); }); @@ -222,13 +221,9 @@ describe("buildCommon", () => { }); describe("buildInfiniteClientOptionsType", () => { it("should exclude the page param from the query type", () => { - const ctx: GenerationContext = { - ...mockContext, - modelNames: [...mockContext.modelNames, "FindPaginatedPetsData"], - }; const result = buildInfiniteClientOptionsType( mockPaginatableOperation, - ctx, + mockContext, ); expect(result.isExported).toBe(true); diff --git a/tests/tsmorph/buildMutationHooks.test.ts b/tests/tsmorph/buildMutationHooks.test.ts index 9435159..7d4beef 100644 --- a/tests/tsmorph/buildMutationHooks.test.ts +++ b/tests/tsmorph/buildMutationHooks.test.ts @@ -55,13 +55,9 @@ const mockFetchContext: GenerationContext = { modelNames: [ "Pet", "NewPet", - "AddPetData", "AddPetError", - "DeletePetData", "DeletePetError", - "UpdatePetData", "UpdatePetError", - "PatchPetData", "PatchPetError", ], serviceNames: ["addPet", "deletePet", "updatePet", "patchPet"], @@ -199,12 +195,17 @@ describe("buildMutationHooks", () => { capitalizedMethodName: "_123CreateThing", dataTypeName: "CreateThingData", }; + const ctx: GenerationContext = { + ...mockFetchContext, + modelNames: [...mockFetchContext.modelNames, "CreateThingError"], + }; - const result = buildUseMutationHook(op, mockFetchContext); + const result = buildUseMutationHook(op, ctx); const initializer = result.declarations[0].initializer as string; expect(initializer).toContain("Options"); - expect(initializer).not.toContain("_123CreateThingData"); + // The Error type shares the Data type stem, not the method name + expect(initializer).toContain("TError = CreateThingError"); }); it("should spread options at the end", () => { diff --git a/tests/tsmorph/buildQueryHooks.test.ts b/tests/tsmorph/buildQueryHooks.test.ts index 4eaf41c..cfeaedf 100644 --- a/tests/tsmorph/buildQueryHooks.test.ts +++ b/tests/tsmorph/buildQueryHooks.test.ts @@ -69,27 +69,18 @@ const mockNoDataTypeOperation: OperationInfo = { // hey-api prefixes a digit-leading operationId in the function name but strips // the digits from the Data type, so the two names diverge (#213) const mockDigitLeadingOperation: OperationInfo = { + ...mockPaginatableOperation, methodName: "_123NumericLead", capitalizedMethodName: "_123NumericLead", dataTypeName: "NumericLeadData", - httpMethod: "GET", - isDeprecated: false, - parameters: [{ name: "page", typeName: "number", optional: true }], - allParamsOptional: true, - isPaginatable: true, - pageParamType: "number", - pageParamTypeKind: "number", }; const mockFetchContext: GenerationContext = { client: "@hey-api/client-fetch", modelNames: [ "Pet", - "FindPetsData", "FindPetsError", - "FindPaginatedPetsData", "FindPaginatedPetsError", - "FindPetByIdData", "FindPetByIdError", ], serviceNames: ["findPets", "findPaginatedPets", "findPetById"], @@ -193,17 +184,18 @@ describe("buildQueryHooks", () => { }); it("should use the signature Data type for digit-leading operationIds (#213)", () => { - const result = buildUseQueryHook( - mockDigitLeadingOperation, - mockFetchContext, - ); + const ctx: GenerationContext = { + ...mockFetchContext, + modelNames: [...mockFetchContext.modelNames, "NumericLeadError"], + }; + const result = buildUseQueryHook(mockDigitLeadingOperation, ctx); const initializer = result.declarations[0].initializer as string; expect(initializer).toContain( "clientOptions: Options = {}", ); - expect(initializer).not.toContain("Options"); - expect(initializer).not.toContain("_123NumericLeadData"); + // The Error type shares the Data type stem, not the method name + expect(initializer).toContain("TError = NumericLeadError"); }); }); @@ -281,7 +273,6 @@ describe("buildQueryHooks", () => { expect(result).not.toBeNull(); const initializer = result?.declarations[0].initializer as string; expect(initializer).toContain("as Options"); - expect(initializer).not.toContain("_123NumericLeadData"); }); it("should make initialPageParam and getNextPageParam optional overrides", () => { diff --git a/tests/tsmorph/buildQueryOptions.test.ts b/tests/tsmorph/buildQueryOptions.test.ts index d2cb10e..3cbe26f 100644 --- a/tests/tsmorph/buildQueryOptions.test.ts +++ b/tests/tsmorph/buildQueryOptions.test.ts @@ -49,12 +49,7 @@ const mockRequiredParamsOperation: OperationInfo = { const mockContext: GenerationContext = { client: "@hey-api/client-fetch", - modelNames: [ - "Pet", - "FindPetsData", - "FindPaginatedPetsData", - "FindPetByIdData", - ], + modelNames: ["Pet"], serviceNames: ["findPets", "findPaginatedPets", "findPetById"], pageParam: "page", nextPageParam: "nextPage", From b053bf19844df321f99e8f6ec1dd804e496f6f8b Mon Sep 17 00:00:00 2001 From: Urata Daiki <7nohe@users.noreply.github.com> Date: Tue, 11 Aug 2026 23:05:18 +0900 Subject: [PATCH 3/5] test: compile the digit-leading fixture end to end Address the review gap flagged by both the altitude and Codex passes: the digit-leading regression was pinned only at the OperationInfo level, so nothing proved the parser and generator together emit infinite hooks and output that typechecks. Run the full createSource pipeline over the digit-leading fixture, organize imports the way generate.mts does, and assert zero TypeScript diagnostics over the generated queries. --- tests/createSource.test.ts | 75 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/tests/createSource.test.ts b/tests/createSource.test.ts index 5b0e022..e12b487 100644 --- a/tests/createSource.test.ts +++ b/tests/createSource.test.ts @@ -1,5 +1,10 @@ +import { mkdir, writeFile } from "node:fs/promises"; +import path from "node:path"; +import { createClient } from "@hey-api/openapi-ts"; +import ts from "typescript"; import { afterAll, beforeAll, describe, expect, test } from "vitest"; import { createSource } from "../src/createSource.mjs"; +import { formatOutput } from "../src/format.mjs"; import { cleanOutputs, generateTSClients, outputPath } from "./utils"; const fileName = "createSource"; @@ -100,3 +105,73 @@ describe(fileName, () => { ); }); }); + +// End-to-end pin for #213: hey-api names the SDK function `_123NumericLead` +// but the Data type `NumericLeadData`. The full pipeline must still detect +// pagination and produce output that typechecks — the bug's two symptoms were +// a silently missing infinite hook and `Options` failing TS2344. +describe("createSource - digit-leading operationId (#213)", () => { + const prefix = "createSource-digit-leading"; + const dir = outputPath(prefix); + + beforeAll(async () => { + await createClient({ + input: path.join(__dirname, "inputs", "digit-leading.yaml"), + output: path.join(dir, "requests"), + plugins: ["@hey-api/client-fetch", "@hey-api/typescript", "@hey-api/sdk"], + }); + }); + afterAll(async () => await cleanOutputs(prefix)); + + test("emits infinite hooks and output that typechecks", async () => { + const source = await createSource({ + outputPath: path.join(dir, "requests"), + version: "1.0.0", + pageParam: "page", + nextPageParam: "nextPage", + initialPageParam: "1", + omitInitialPageParam: false, + client: "@hey-api/client-fetch", + }); + + const infiniteQueriesTs = source.find( + (s) => s.name === "infiniteQueries.ts", + ); + expect(infiniteQueriesTs?.content).toContain("use_123NumericLeadInfinite"); + expect(infiniteQueriesTs?.content).toContain( + "Options", + ); + + const queriesDir = path.join(dir, "queries"); + await mkdir(queriesDir, { recursive: true }); + await Promise.all( + source.map((file) => + writeFile(path.join(queriesDir, file.name), file.content), + ), + ); + // The real pipeline organizes imports after printing (generate.mts), + // which dedupes the Options import shared by the client and service + // import declarations — compile what users actually get. + await formatOutput(queriesDir); + + const program = ts.createProgram( + source.map((file) => path.join(queriesDir, file.name)), + { + strict: true, + noEmit: true, + skipLibCheck: true, + esModuleInterop: true, + target: ts.ScriptTarget.ES2020, + module: ts.ModuleKind.ESNext, + moduleResolution: ts.ModuleResolutionKind.Bundler, + }, + ); + const diagnostics = ts + .getPreEmitDiagnostics(program) + .map( + (d) => + `${d.file?.fileName ?? ""}: ${ts.flattenDiagnosticMessageText(d.messageText, "\n")}`, + ); + expect(diagnostics).toEqual([]); + }); +}); From 9f46a99bb539137936e8f627180e5f62b1827c21 Mon Sep 17 00:00:00 2001 From: Urata Daiki <7nohe@users.noreply.github.com> Date: Tue, 11 Aug 2026 23:09:35 +0900 Subject: [PATCH 4/5] test: raise the timeout on the digit-leading compile test Type-checking the generated output against the real TanStack Query types exceeds vitest's default 5s timeout on CI runners. --- tests/createSource.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/createSource.test.ts b/tests/createSource.test.ts index e12b487..b7a3910 100644 --- a/tests/createSource.test.ts +++ b/tests/createSource.test.ts @@ -123,7 +123,11 @@ describe("createSource - digit-leading operationId (#213)", () => { }); afterAll(async () => await cleanOutputs(prefix)); - test("emits infinite hooks and output that typechecks", async () => { + // Type-checking the generated output against the real @tanstack/react-query + // types takes longer than the default 5s timeout on CI runners + test("emits infinite hooks and output that typechecks", { + timeout: 60_000, + }, async () => { const source = await createSource({ outputPath: path.join(dir, "requests"), version: "1.0.0", From 3587031f03433b0c2444a41f35c17dfc4a1ebba2 Mon Sep 17 00:00:00 2001 From: Urata Daiki <7nohe@users.noreply.github.com> Date: Tue, 11 Aug 2026 23:13:39 +0900 Subject: [PATCH 5/5] test: set a 30s global testTimeout The suite runs real codegen (hey-api generation, TypeScript programs), which is multi-second work; under CI runner contention the 5s default flaked on the pre-existing bundler-resolution test as well. Replace the per-test override with a global timeout. --- tests/createSource.test.ts | 6 +----- vitest.config.ts | 3 +++ 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/createSource.test.ts b/tests/createSource.test.ts index b7a3910..e12b487 100644 --- a/tests/createSource.test.ts +++ b/tests/createSource.test.ts @@ -123,11 +123,7 @@ describe("createSource - digit-leading operationId (#213)", () => { }); afterAll(async () => await cleanOutputs(prefix)); - // Type-checking the generated output against the real @tanstack/react-query - // types takes longer than the default 5s timeout on CI runners - test("emits infinite hooks and output that typechecks", { - timeout: 60_000, - }, async () => { + test("emits infinite hooks and output that typechecks", async () => { const source = await createSource({ outputPath: path.join(dir, "requests"), version: "1.0.0", diff --git a/vitest.config.ts b/vitest.config.ts index 4c9fe78..55eb958 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -8,6 +8,9 @@ export default defineConfig({ // instead of 15). Note that a CLI `--exclude` appends to this list rather than // replacing it, so you cannot A/B this entry from the command line. exclude: [...defaultExclude, ".claude/**"], + // Tests run real codegen (hey-api generation, TypeScript programs), which is + // multi-second work; under CI runner contention the 5s default flakes. + testTimeout: 30_000, coverage: { // Scope coverage with `include`, not `exclude`. `coverage.exclude` is matched // against absolute paths with picomatch's `contains` option, so every relative