Skip to content
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.

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

[kbn/server-route-repository] Improve type inference when using IKibanaResponseFactory #198677

Open
miltonhultgren opened this issue Nov 1, 2024 · 2 comments
Labels
Team:obs-knowledge Observability Experience Knowledge team

Comments

@miltonhultgren
Copy link
Contributor

miltonhultgren commented Nov 1, 2024

We have added support for returning the result's from KibanaResponseFactory. This works well with our inference when using the ok function since we can unwrap the object we pass back.

But when using any of the error codes, our inference breaks down because they are typed as any, so which ends up overshadowing any other type information we try to provide.

We should find some way to alter these types or make them changeable so that we can better infer the shape of the error objects.
Perhaps if they are changed to unknown or never instead that could improve the situation.

Note: In general it is better for performance to define explicit return types on the handler since it means less inference work for the TypeScript compiler to do

@botelastic botelastic bot added the needs-team Issues missing a team label label Nov 1, 2024
@miltonhultgren miltonhultgren added the Team:obs-knowledge Observability Experience Knowledge team label Nov 1, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-knowledge-team (Team:obs-knowledge)

@botelastic botelastic bot removed the needs-team Issues missing a team label label Nov 1, 2024
@miltonhultgren
Copy link
Contributor Author

@awahab07 Just as an example, if we change this line:

conflict(options?: ErrorHttpResponseOptions): IKibanaResponse;

to

conflict(options?: ErrorHttpResponseOptions): IKibanaResponse<never>;

Then that seems to fix it but I don't know what the impact of this would be, or if it semantically makes sense.
I believe it does (there will never be a payload because the HTTP client will interpret those status codes as errors and throw instead), but we need to validate that with the Core team perhaps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:obs-knowledge Observability Experience Knowledge team
Projects
None yet
Development

No branches or pull requests

2 participants