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

Some strict TS narrowing is limiting parameter reuse #145

Closed
thelinuxlich opened this issue Sep 20, 2022 · 9 comments
Closed

Some strict TS narrowing is limiting parameter reuse #145

thelinuxlich opened this issue Sep 20, 2022 · 9 comments

Comments

@thelinuxlich
Copy link
Contributor

If we define the parameters array outside of the parameter definition like this:

apiBuilder({
  path: '/marketplaces/:id/transactions',
  method: 'get',
  alias: 'getTransactions',
  parameters: queryStringStandardParameters,
  response: transactionsSchema,
})

The typechecker complains with:
image

@ecyrbe
Copy link
Owner

ecyrbe commented Sep 20, 2022

did you use the asParameters helper to create the parameters ?
if so can you provide full definition so i can reproduce the issue ?

@thelinuxlich
Copy link
Contributor Author

Ohhh, I didn't know it was required to reuse params. Looking at this method description, I was actually wondering its purpose: https://www.zodios.org/docs/api/helpers#asparameters

Maybe it should have a minor change to emphasize it's required for params composition in asApi/apiBuilder

@thelinuxlich
Copy link
Contributor Author

But also Iooking at NarrowRaw definition, I don't see at first glance why parameter type couldn't be a standard union(Query | Header | Body) so it doesn't need the asParameter wrapper

@ecyrbe
Copy link
Owner

ecyrbe commented Sep 20, 2022

Yes unfortunately, yes you need to wrap into asParameters to infer the type correctly. Else typescript will complain the string are not literals.
Before, zodios was using the as const, but this restricted usage of zodios to only typescript.
So for zodios to be compatible with javacript, zodios now need to resort to this narrowing trick.
So everywhere you need to compose your definitions into splitted ones, you need to use these helpers.

I'll improve the documentation to make this clearer.

@ecyrbe
Copy link
Owner

ecyrbe commented Sep 21, 2022

Fixed in version 9.1 :

  • Improved docs
  • deprecated asApi, asParameters, asErrors and asCrudApi (they are still there)
  • added makeApi, makeParameters, makeErrors, makeCrudApi as these names are more suggestive that you need to use them to create the underlying definitions

@ecyrbe ecyrbe closed this as completed Sep 21, 2022
@dejay-at-upstart
Copy link

but this restricted usage of zodios to only typescript.

I'm confused. Aren't Zod and Zodios both TypeScript-first libraries?

I'm not a TypeScript expert, and I'm trying to read through the source code to figure out how to get even simple, simple scenarios to work properly in Zodios. The way TypeScript narrowing works here is making it really difficult to wrap, extend, or abstract Zodios into additional layers.

E.g.:

// Works:
const workingApi = makeApi([{
  method: 'get',
  path: '/api/users',
  alias: 'getUsers',
  response: z.string(),
  parameters: [],
}]);

const endpoints = [{
  method: 'get',
  path: '/api/users',
  alias: 'getUsers',
  response: z.string(),
  parameters: [],
}];

// Broken:
const brokenApi = makeApi(endpoints);
/*
Error: Argument of type '{ method: string; path: string; alias: string; response:
ZodString; parameters: never[]; }[]' is not assignable to parameter of type
'{ method: NarrowRaw<Method>; path: string; alias?: NarrowRaw<string | undefined>;
description?: string | undefined; requestFormat?: NarrowRaw<RequestFormat | undefined>;
... 5 more ...; errors?: NarrowRaw<...>; }[]'.
*/

I hate to be the old man shouting at clouds here, but if a developer is using a programming language, IDE, framework, or package that make it harder to perform the very simplest type of abstraction that a developer could possibly perform, I fail to see how that ecosystem could be appropriate for the developer. Maybe this just comes down to me not being a TypeScript expert, but I feel I shouldn't have to be an expert to pass a single variable into a single function.

Here's another example:

const workingApi = makeApi([{
  method: 'get',
  path: '/api/users',
  alias: 'getUsers',
  response: z.string(),
  parameters: [],
}]);

const workingClient = new Zodios('/', workingApi);

// Works:
workingClient.getUsers();

const endpoints = [{
  method: 'get',
  path: '/api/users',
  alias: 'getUsers',
  response: z.string(),
  parameters: [],
}];

const brokenApi = makeApi(endpoints as Parameters<typeof makeApi>[0]);
const brokenClient = new Zodios('/', brokenApi);

// Broken:
brokenClient.getUsers();
/*
Property 'getUsers' does not exist on type 'ZodiosInstance<{ method: NarrowRaw<Method>;
path: string; alias?: NarrowRaw<string | undefined>; description?: string | undefined;
requestFormat?: NarrowRaw<RequestFormat | undefined>; ... 5 more ...;
errors?: NarrowRaw<...>; }[]>'.ts(2339)
*/

I'm now searching through the Zodios source code to discover how I could possibly get around these issues.

TypeScript is making me cynical. Now, whenever I see a claim like "Foo, a simple library for TypeScript users", I automatically translate that to "Foo, a simple library for TypeScript users with Ph.Ds in type casting, up casting, type guarding, type inference, type helpers, type narrowing, type widening, typing context, type literals, type symbols, inline typing, generic types, static type inference, dynamic type inference, compile-time type validation, run-time type validation, schema validation, covariance, contravariance, AST traversal, etc. etc. etc.

Anyways, I've just come across other documented issues that recommend using the builders and internal client types, etc., so I'm going to give all of those things a shot on top of the hundred other things I've already tried.

Thanks, TypeScript, you've made my life easier now that I don't have untyped variables.

@ecyrbe
Copy link
Owner

ecyrbe commented Oct 2, 2023

You need to use the helper makeApi or apiBuilder with parameters inlined. Else typescript can't infer literals and convert them to just a type of string and tuples into arrays.
Don't fight the library, it's explicitely stated everywhere in the docs how to declare APIs.
It's not ideal, but there is no way around typescript limitations here.
If you still need a typesafe REST api client but don't want to bother about this (writing déclarations by hand)
Take a look at @astahmer projects, there are really good code generated alternatives he maintain.

@dejay-at-upstart
Copy link

You need to use the helper makeApi or apiBuilder with parameters inlined. Else typescript can't infer literals and convert them to just a type of string and tuples into arrays.

I kind of wonder whether having static type checking of API endpoints is really worth all the complication it introduces into the framework in terms of restrictions and workarounds.

@ecyrbe
Copy link
Owner

ecyrbe commented Oct 8, 2023

if it wasn't, tRPC and Zodios would not exist.
The restrictions are a simple function call that are part of the framework, and using them also provide autocompletion to write the endpoint and call them.
nestjs or angular for exemple uses decorators that are way more verbose and still don't provide type safety, no autocompletion, while tRPC and Zodios with a simple function call to wrap your endpoints provide it.
i think it's pretty mind blowing it can do it for such a little cost actually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants