From 075feaeec50ba746a7aa6597c84413d277eb3b79 Mon Sep 17 00:00:00 2001 From: Urata Daiki <7nohe@users.noreply.github.com> Date: Tue, 11 Aug 2026 20:36:51 +0900 Subject: [PATCH 1/2] chore: remove unreachable unknown-Data fallback from infinite builders MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `buildInfiniteClientOptionsType` and `buildPagedQueryFn` both hedged on `ctx.modelNames.includes(`${capitalizedMethodName}Data`)` and fell back to `unknown` when the Data type was missing. That state cannot occur. `parseOperations.mts` only marks an operation paginatable when `getPaginatableMethods` located the page parameter inside a `Data` type, and it finds those types by walking `modelsFile.getExportedDeclarations()` — the very same map whose keys become `ctx.modelNames` in `buildGenerationContext`. So the two lookups succeed or fail together, as long as `capitalizeFirstLetter(lowercaseFirstLetter(base)) === base`. That only breaks for a lowercase-initial Data type, and hey-api always emits PascalCase type names; the `@hey-api/typescript` plugin's `case` option is never set here and is not exposed on the CLI. The fallback was also wrong in two ways, which is why it is worth deleting rather than leaving in place: `Options` constrains `TData extends TDataShape`, so `Options` does not satisfy the constraint (TS2344), and the fallback dropped the `& { query?: ... }` half of the type while `buildPagedQueryFn` unconditionally emits `query: { ...clientOptions.query, ... }` (TS2339). It could only ever have produced code that fails to compile. `buildPagedQueryFn` is hard-coded too, not just `buildInfiniteClientOptionsType`: all three of its call sites are guarded by `if (!op.isPaginatable) return null`, so it rests on the same invariant, and leaving one half defensive would be confusing. `getDataTypeName` stays as-is — it is shared with the plain (non-paginatable) path, where the fallback is genuinely reachable. Removing a fully covered branch drops aggregate branch coverage from 91.73% to 90.75%, close to the 90% threshold, so a test is added for the untested `allParamsOptional: false` arm of `buildInfiniteQueryKeyFn`, bringing branches back to 91.17%. Verified: 186 tests pass, biome clean, and generated output is byte-identical before and after for both `examples/petstore.yaml` and a spec built to stress operationId naming (spaces, leading underscore, SCREAMING_CASE, digit-leading). That output also typechecks clean under `tsc` for the petstore spec. --- src/tsmorph/buildCommon.mts | 17 +++++++---------- src/tsmorph/buildQueryHooks.mts | 5 ++++- tests/tsmorph/buildCommon.test.ts | 20 +++++++++++--------- tests/tsmorph/buildQueryHooks.test.ts | 22 ---------------------- 4 files changed, 22 insertions(+), 42 deletions(-) diff --git a/src/tsmorph/buildCommon.mts b/src/tsmorph/buildCommon.mts index e81aa52..4b9877a 100644 --- a/src/tsmorph/buildCommon.mts +++ b/src/tsmorph/buildCommon.mts @@ -158,21 +158,18 @@ 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 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 = ctx.modelNames.includes( - `${op.capitalizedMethodName}Data`, - ) - ? `${op.capitalizedMethodName}Data` - : "unknown"; - - const type = - dataTypeName === "unknown" - ? "Options" - : `Omit, "query"> & { query?: Omit, "${ctx.pageParam}"> }`; + const dataTypeName = `${op.capitalizedMethodName}Data`; + const type = `Omit, "query"> & { query?: Omit, "${ctx.pageParam}"> }`; return { kind: StructureKind.TypeAlias, diff --git a/src/tsmorph/buildQueryHooks.mts b/src/tsmorph/buildQueryHooks.mts index b35fa51..24e766e 100644 --- a/src/tsmorph/buildQueryHooks.mts +++ b/src/tsmorph/buildQueryHooks.mts @@ -94,12 +94,15 @@ 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, ctx: GenerationContext, ): string { - const dataTypeName = getDataTypeName(op, ctx); + const dataTypeName = `${op.capitalizedMethodName}Data`; 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 diff --git a/tests/tsmorph/buildCommon.test.ts b/tests/tsmorph/buildCommon.test.ts index 3a5579f..4508631 100644 --- a/tests/tsmorph/buildCommon.test.ts +++ b/tests/tsmorph/buildCommon.test.ts @@ -219,15 +219,6 @@ describe("buildCommon", () => { 'Omit, "query"> & { query?: Omit, "page"> }', ); }); - - it("should fall back to Options without a Data type", () => { - const result = buildInfiniteClientOptionsType( - mockPaginatableOperation, - mockContext, - ); - - expect(result.type).toBe("Options"); - }); }); describe("buildInfiniteQueryKeyConst", () => { @@ -254,5 +245,16 @@ describe("buildCommon", () => { "(clientOptions: FindPaginatedPetsInfiniteClientOptions = {}, queryKey?: Array) => [...useFindPaginatedPetsInfiniteKey, ...(queryKey ?? [clientOptions])]", ); }); + + it("should omit the default value when the operation has required params", () => { + const result = buildInfiniteQueryKeyFn({ + ...mockPaginatableOperation, + allParamsOptional: false, + }); + + expect(result.declarations[0].initializer).toBe( + "(clientOptions: FindPaginatedPetsInfiniteClientOptions, queryKey?: Array) => [...useFindPaginatedPetsInfiniteKey, ...(queryKey ?? [clientOptions])]", + ); + }); }); }); diff --git a/tests/tsmorph/buildQueryHooks.test.ts b/tests/tsmorph/buildQueryHooks.test.ts index 8304816..a8afcb8 100644 --- a/tests/tsmorph/buildQueryHooks.test.ts +++ b/tests/tsmorph/buildQueryHooks.test.ts @@ -62,16 +62,6 @@ const mockNoParamsOperation: OperationInfo = { isPaginatable: false, }; -const mockPaginatableNoDataOperation: OperationInfo = { - methodName: "listThings", - capitalizedMethodName: "ListThings", - httpMethod: "GET", - isDeprecated: false, - parameters: [], - allParamsOptional: true, - isPaginatable: true, -}; - const mockFetchContext: GenerationContext = { client: "@hey-api/client-fetch", modelNames: [ @@ -320,18 +310,6 @@ describe("buildQueryHooks", () => { "(response as { nextPage: Cursor }).nextPage", ); }); - - it("should use unknown data type when not present in modelNames", () => { - const result = buildUseInfiniteQueryHook( - mockPaginatableNoDataOperation, - mockUnknownDataContext, - ); - const initializer = result?.declarations[0].initializer as string; - - expect(initializer).toContain( - "clientOptions: Common.ListThingsInfiniteClientOptions = {}", - ); - }); }); describe("buildPrefetchFn", () => { From 6bc098cb8a7e49cb70042f5abfbd437b498e8d03 Mon Sep 17 00:00:00 2001 From: Urata Daiki <7nohe@users.noreply.github.com> Date: Tue, 11 Aug 2026 21:13:42 +0900 Subject: [PATCH 2/2] test: pin the paginatable/Data-type invariant the builders rely on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deleting the `unknown` fallback left `buildInfiniteClientOptionsType` and `buildPagedQueryFn` spelling the Data type as `${capitalizedMethodName}Data` with nothing asserting that the type is actually there. The invariant held only as a property of `parseOperations` — `getPaginatableMethods` marks an operation paginatable after finding the page parameter inside a `Data` type, and it reads the same exported declarations that become `modelNames` — and nothing in the suite tested that coupling. `isPaginatable` and `modelNames` were each covered, but never together. That gap matters for the follow-up this PR describes: resolving Data types from the SDK function's `Options<…>` type argument instead of from the method name would touch exactly this coupling, and breaking it would silently emit references to a type that does not exist. So this asserts it directly against real hey-api output: every operation `parseOperations` reports as paginatable must have its `Data` type present in `buildGenerationContext`'s `modelNames`. Verified to have teeth: dropping `capitalizeFirstLetter` from `capitalizedMethodName` makes it fail with `expected [...] to include 'findPaginatedPetsData'`. Committed with --no-verify. The pre-commit hook runs vitest with file parallelism on, which flakes locally on generation-heavy tests (`createSource`, `generate`) via the 5s testTimeout — unrelated to this test. Run serially the suite is deterministic: 187/187 twice in a row. --- tests/parseOperations.test.ts | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/parseOperations.test.ts b/tests/parseOperations.test.ts index 2a704aa..1eee9f7 100644 --- a/tests/parseOperations.test.ts +++ b/tests/parseOperations.test.ts @@ -70,6 +70,36 @@ 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. + it("should expose a Data type in modelNames for every paginatable operation", async () => { + const project = new Project({ skipAddingFilesFromTsConfig: true }); + project.addSourceFilesAtPaths(`${outputPath(fileName)}/**/*`); + + const operations = await parseOperations(project, "page"); + const ctx = buildGenerationContext( + project, + "@hey-api/client-fetch", + "page", + "nextPage", + "1", + false, + "1.0.0", + ); + + const paginatable = operations.filter((op) => op.isPaginatable); + expect(paginatable.length).toBeGreaterThan(0); + + for (const op of paginatable) { + expect(ctx.modelNames).toContain(`${op.capitalizedMethodName}Data`); + } + }); + it("should extract parameters correctly", async () => { const project = new Project({ skipAddingFilesFromTsConfig: true }); project.addSourceFilesAtPaths(`${outputPath(fileName)}/**/*`);