Skip to content

fix(sdk-codegen): make run_inline_query response type generic #1730

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
marc-wilson wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(sdk-codegen): make run_inline_query response type generic #1730

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cf66aac
fix(sdk-codegen): make run_inline_query response type generic
marc-wilson Jul 2, 2026
f61843d
Merge branch 'main' into fix/run-inline-query-json-type
marc-wilson Jul 20, 2026
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
51 changes: 45 additions & 6 deletions packages/sdk-codegen/src/typescript.gen.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,9 +451,10 @@ let response = await sdk.ok(sdk.${method.name}(`;
params.length > 0 ? `\n${params.join(this.paramDelimiter)}` : '';
}
const callback = `callback: (response: Response) => Promise<${mapped.name}>,`;
const generics = streamer ? '' : this.methodGenericParams(method);
Comment on lines 453 to +454

@gemini-code-assist gemini-code-assist Bot Jul 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Currently, if streamer is true, the generic type parameter is omitted (generics is set to '') and the callback return type is hardcoded to Promise<${mapped.name}> (which resolves to Promise<string>).

To ensure that streaming versions of format-dependent methods also benefit from the generic response type, we should use this.responseTypeName(method) in the callback signature and allow generics to be generated for streaming methods as well.

Suggested change
const callback = `callback: (response: Response) => Promise<${mapped.name}>,`;
const generics = streamer ? '' : this.methodGenericParams(method);
const callback = 'callback: (response: Response) => Promise<' + this.responseTypeName(method) + '>,';
const generics = this.methodGenericParams(method);

@marc-wilson marc-wilson Jul 2, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, and agreed in principle — but I'd like to keep this specific to the non-streaming path for now.

The streaming variant's callback receives the raw Response/Readable (e.g. for piping a download to a file), so its return type is really "whatever the caller's callback resolves to," not the parsed response.value shape that issue #1703 is about. Hardcoding that callback's return type to string predates this PR and isn't something this change touches — I only added the generic to the non-streaming (streamer === false) branch.

That said, I think your suggestion is a reasonable extension: expose the same <T> on the streaming signature so callers of sdk.stream.run_inline_query<T>(...) aren't stuck with Promise<string> either. I'd rather do that as a deliberate follow-up (along with the other result_format-driven methods called out in the PR description — run_query, run_look, etc.) once the core generic-default pattern here is confirmed to be the right approach, rather than widen this PR's diff. Let me know if you'd prefer it bundled in now instead.

const header =
this.commentHeader(indent, headComment) +
`${indent}async ${method.name}(` +
`${indent}async ${method.name}${generics}(` +
(streamer ? `\n${bump}${callback}` : '');
const returns = streamer ? '' : `: ${this.returnType(indent, method)}`;

Expand Down Expand Up @@ -494,15 +495,53 @@ let response = await sdk.ok(sdk.${method.name}(`;
);
}

/**
* Methods whose declared response type is a generic `string` but whose
* actual runtime shape depends on a `result_format` request parameter
* (e.g. 'json' returns a parsed object/array, not a string). The Looker
* API spec does not document per-format response shapes, so these are
* special-cased here to expose a generic type parameter instead of the
* inaccurate hardcoded `string` return type.
*
* See https://github.com/looker-open-source/sdk-codegen/issues/1703
*/
private static readonly formatDependentStringMethods = new Set([
'run_inline_query',
]);

/**
* `true` if this method should expose a `<T = string>` generic instead of
* a hardcoded `string` return type
*/
hasGenericResponse(method: IMethod): boolean {
return TypescriptGen.formatDependentStringMethods.has(method.name);
}

/**
* The generic type parameter declaration to splice between a method's
* name and its parameter list, or '' if the method has no generic response
*/
methodGenericParams(method: IMethod): string {
return this.hasGenericResponse(method) ? '<T = string>' : '';
}

/**
* The name to use for the method's response type, honoring the generic
* response override for format-dependent methods
*/
responseTypeName(method: IMethod): string {
if (this.hasGenericResponse(method)) return 'T';
return this.typeMap(method.type).name;
}

/**
* Return type declaration for the method
* @param indent
* @param method
*/
returnType(indent: string, method: IMethod): string {
const mapped = this.typeMap(method.type);
const errors = this.errorResponses(indent, method);
return `Promise<SDKResponse<${mapped.name}, ${errors}>>`;
return `Promise<SDKResponse<${this.responseTypeName(method)}, ${errors}>>`;
}

customHeaderComment(term: string, method: IMethod, params: string[] = []) {
Expand Down Expand Up @@ -650,9 +689,10 @@ ${indent}})`;
fragment =
params.length > 0 ? `\n${params.join(this.paramDelimiter)}` : '';
}
const generics = this.methodGenericParams(method);
const header =
this.commentHeader(indent, headComment) +
`${indent}export const ${method.name} = async (`;
`${indent}export const ${method.name} = async ${generics}(`;
const returns = this.returnType(indent, method);

return (
Expand Down Expand Up @@ -836,13 +876,12 @@ ${indent}})`;

httpCall(indent: string, method: IMethod) {
const request = this.useRequest(method) ? 'request.' : '';
const mapped = this.typeMap(method.type);
const bump = this.bumper(indent);
const args = this.httpArgs(bump, method);
const errors = this.errorResponses(indent, method);
return (
`${indent}return ${this.it(method.httpMethod.toLowerCase())}` +
`<${mapped.name}, ${errors}>(` +
`<${this.responseTypeName(method)}, ${errors}>(` +
this.httpPath(method.endpoint, request) +
`${args ? ', ' + args : ''})`
);
Expand Down
6 changes: 3 additions & 3 deletions packages/sdk/src/4.0/funcs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10890,13 +10890,13 @@ export const run_query = async (
* @param options one-time API call overrides
*
*/
export const run_inline_query = async (
export const run_inline_query = async <T = string>(
sdk: IAPIMethods,
request: IRequestRunInlineQuery,
options?: Partial<ITransportSettings>
): Promise<SDKResponse<string, IError | IValidationError>> => {
): Promise<SDKResponse<T, IError | IValidationError>> => {
request.result_format = encodeParam(request.result_format);
return sdk.post<string, IError | IValidationError>(
return sdk.post<T, IError | IValidationError>(
`/queries/run/${request.result_format}`,
{
limit: request.limit,
Expand Down
6 changes: 3 additions & 3 deletions packages/sdk/src/4.0/methods.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10236,12 +10236,12 @@ export class Looker40SDK extends APIMethods implements ILooker40SDK {
* @param options one-time API call overrides
*
*/
async run_inline_query(
async run_inline_query<T = string>(
request: IRequestRunInlineQuery,
options?: Partial<ITransportSettings>
): Promise<SDKResponse<string, IError | IValidationError>> {
): Promise<SDKResponse<T, IError | IValidationError>> {
request.result_format = encodeParam(request.result_format);
return this.post<string, IError | IValidationError>(
return this.post<T, IError | IValidationError>(
`/queries/run/${request.result_format}`,
{
limit: request.limit,
Expand Down
4 changes: 2 additions & 2 deletions packages/sdk/src/4.0/methodsInterface.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7236,10 +7236,10 @@ export interface ILooker40SDK extends IAPIMethods {
* @param options one-time API call overrides
*
*/
run_inline_query(
run_inline_query<T = string>(
request: IRequestRunInlineQuery,
options?: Partial<ITransportSettings>
): Promise<SDKResponse<string, IError | IValidationError>>;
): Promise<SDKResponse<T, IError | IValidationError>>;

/**
* ### Run an URL encoded query.
Expand Down