ecyrbe/zodios

Excessive stack depth comparing types

Opened this issue ยท 32 comments

I have a middleware in which I'm trying to pass the Zodios express request object to a function:

const healthApi = makeApi([
  {
    method: 'get',
    path: '/',
    description: 'Returns health status and version information of API',
    response: z.object({
      status: z.literal('ok'),
      version: z.string(),
    }),
    alias: 'getHealth',
  },
]);

export const api = mergeApis({
  '/healthz': healthApi,
  // ... other API endpoints created with makeApi()
});

const ctxSchema = z.object({
  user: z.instanceof(User),
});

type Api = typeof api;
type Context = typeof ctxSchema;
type Method = ZodiosMethod;
type ZodiosPaths<M extends Method> = ZodiosPathsByMethod<Api, M>;
type Middleware<M extends Method, Path extends ZodiosPaths<M>> = ZodiosRequestHandler<
  Api,
  Context,
  M,
  Path
>;

type ZodiosRequest = WithZodiosContext<Request, Context>;

const findEndpointForRequest = (req: ZodiosRequest) =>
  api.find(
    (endpoint) => endpoint.method === req.method.toLowerCase() && endpoint.path === req.route.path,
  );

const auditMiddleware =
  <M extends Method, Path extends ZodiosPaths<M>>(
    requestHandler: Middleware<M, Path>,
  ): Middleware<M, Path> =>
  async (req, res, next) => {
    const endpoint = findEndpointForRequest(req);
  };

TypeScript is giving me an error though:

Excessive stack depth comparing types 'Request<IfEquals<{ [K in PathParamNames<Path, never>]: MapSchemaParameters<FilterArrayByValue<({ method: "get"; path: "/healthz"; description: "Returns health status and version information of API"; response: ZodObject<...>; alias: "getHealth"; } extends { ...; } ? { ...; } extends { ...; } ? { ...; } extends { ...;...' and 'Request<ParamsDictionary, any, any, ParsedQs, Record<string, any>> & { user: User; }'.

I have the feeling that something's generally off with my API declaration or middleware because everything seems to work as expected, but while editing code I'm seeing intermittent type errors as well which go away when restarting the TS server in VSCode:

'api' implicitly has type 'any' because it does not have a type annotation and is referenced directly or indirectly in its own initializer.

Type arguments for 'ZodiosRequestHandler' circularly reference themselves.

The second error points to ZodiosRequestHandler in my type Middleware declaration in the example above.

Thanks a lot!

ecyrbe commented

Hello,

I'll need the full api declaration to trouble shoot what is hapenning

Hi ecyrbe,

I need some time to look over the API spec and see if there's anything we don't want to make public.

The error is so sporadic that I'm unable to reliably reproduce it. ๐Ÿ˜•

General question: My API has "only" 55 endpoints and VSCode is feeling quite sluggish already on my M1 Macbook Pro. Is it normal that TS checking takes about 2-3 seconds now with Zodios? Typechecking was almost instantly before introducing it.
Maybe there is indeed something wrong with my code which also slows down the TS checking in general? The circular reference error mentioned above could maybe explain why everything is getting slow and why the error shows up only sporadically. Just a wild guess: Maybe depending on in what order some cyclic reference is resolved? Could that be a possibility?

Thanks a lot for any help and this awesome library!

ecyrbe commented

Can you try to update to latest version, i also found ts perf issues with 10.9.2 , 10.9.4 got it back to better perfs.
And next release v11 handles even more endpoints and will be out by the end of the month.

The new version caused even longer delays (10+ seconds) during autocompletion and type hovering with TS eventually just giving up, sometimes showing sporadic and indeterministic errors (excessively deep types, circular references and such).

Because I tested my API declaration in isolation and couldn't find any problems regarding type checking (it's in a separate package), I went ahead and replaced the middleware types in my backend package with any:

// export type Method = ZodiosMethod;
// export type ZodiosPaths<M extends Method> = ZodiosPathsByMethod<Api, M>;
// export type Middleware<M extends Method, Path extends ZodiosPaths<M>> = ZodiosRequestHandler<
//   Api,
//   Context,
//   M,
//   Path
// >;

export type Middleware = any;
export type MyAppRequestHandler = any; // ZodiosRouterContextRequestHandler<Context>;
export type MyAppRequest = any; // WithZodiosContext<Request, Context>;

Although my middlewares are now screaming with type errors, this seems to have fixed the problem and TS started working again. Even the autocomplete performance went back to normal territory. So I guess something inside the middleware types must be causing a circular reference or similar thing.

Any other ideas?

Thanks again for debugging this with me!

For now, I any-typed my (route-specific) middlewares to get my application up and running again:

export const myMiddleware = () => (req: any, res: any, next: any) => {
   // ...
}

This fixes all TS hickups and performance issues. It's not a big deal at the moment, because I have only a handful of (route-specific) middlewares which are quite small. All global (app-specific) middlewares are correctly typed as follows and don't seem to cause any trouble:

export type MyAppRequestHandler = ZodiosRouterContextRequestHandler<MyAppContext>;
export const checkAuth = (): MyAppRequestHandler => async (req, res, next) => {
   // ...
}

Nevertheless, it'd still be cool if we figured out what causes the TS compiler to run in circles when properly typing route-specific middlewares. โค๏ธ

@ecyrbe We're now seeing (probably) similar problems in the frontend since introducing getKeyByAlias:

zodios.getKeyByAlias('user', { params: { uuid: userUUID } }),

Adding just this single line to our code base wreaks havoc on TS Intellisense. It now takes 10+ seconds and type-checking our frontend React code with tsc --noEmit eventually times out with: Type instantiation is excessively deep and possibly infinite.. Remove the line and the problem goes away.

The backend and API declaration package type-check just fine. I have put together a minimal reproduction repository and invited you to collaborate.

Interestingly, the problem is non-existent when using getKeyByPath instead:

zodios.getKeyByPath('get', '/users/:uuid', { params: { uuid: userUUID } }),

I was able to reproduce the excessive type instantiation problem we were having in our express backend as well. I added a simple reproduction in the repository I shared with you.

ecyrbe commented

thank you, will take a look

FWIW, even after getting rid of all route-specific middlewares I'm still seeing this sporadically:

'router' implicitly has type 'any' because it does not have a type annotation and is referenced directly or indirectly in its own initializer.

Just ran into this again immediately after opening my project in VS Code. Restarting TS server made it go away.

@ecyrbe May I kindly ask if you've had a chance to take a look at this yet?

Any news on this one, @ecyrbe? Thanks!

No solution at the moment.

Thank you! Were you able to reproduce the issue and we can consider it a bug? Or do you need more time to investigate this further?

i think it's a bug, so i'll let it open until fixed

Hey @ecyrbe,

please don't take the following the wrong way: You've been cranking out release after release over the year and now there hasn't been much activity on the Zodios repository for almost two months. If my memory serves correctly, you've had plans to release v11 at the end of August, but for whatever reasons that didn't work out (no offense, I'm absolutely cool with v10, maybe except for the bug discussed in this issue).

After having just migrated a rather big internal application to Zodios, I'm getting somewhat nervous that you're thinking of abandoning the project. Maybe you could be so kind to let us know what the next milestones for the project are going to be? That'd definitely give me (and maybe others) some peace of mind. If you're just taking some time out that's perfectly understandable! ๐Ÿ˜‰

Again, I'm not complaining! I know that it takes a lot of time and hard work to maintain a library like Zodios and I'm absolutely thankful for all the work you've put into it so far! It has made my life as an app developer much easier. Merci beaucoup! ๐Ÿ™

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Please reopen, @ecyrbe. Thanks!

Maybe you could be so kind to let us know what the next milestones for the project are going to be? That'd definitely give me (and maybe others) some peace of mind. If you're just taking some time out that's perfectly understandable! ๐Ÿ˜‰

๐ŸŽ‰ https://twitter.com/zodios_api/status/1733533770706223575

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Bump

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Bump

Not sure if related, but also facing performance issues when using mock<ZodiosClient> from vitest-mock-extended.

import { mock, type MockProxy } from 'vitest-mock-extended';

const mockAccountsClient: MockProxy<AccountsClient> = mock();
//           ^ TypeScript. Type instantiation is excessively deep and possibly infinite

The mockAccountsClient is resolved to any with the error `TypeScript. Type instantiation is excessively deep and possibly infinite.

Here is the API definition for this client: https://gist.github.com/toteto/61d651541f8e93e27f4891ebbac33ea1

Previously was OK, but adding the removeMfaAuthEndpoint caused the TS to timeout. Removing it, bring things to normal (but slow).

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

bump

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

bump

stale commented

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Hope dies last. Please let us know what's gonna happen to Zodios, @ecyrbe! ๐Ÿ™

stale commented

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Bump