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

Excessive stack depth comparing types #505

Open
soulchild opened this issue Aug 9, 2023 · 29 comments
Open

Excessive stack depth comparing types #505

soulchild opened this issue Aug 9, 2023 · 29 comments
Labels
wontfix This will not be worked on

Comments

@soulchild
Copy link

soulchild commented Aug 9, 2023

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
Copy link
Owner

ecyrbe commented Aug 9, 2023

Hello,

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

@soulchild
Copy link
Author

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
Copy link
Owner

ecyrbe commented Aug 14, 2023

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.

@soulchild
Copy link
Author

soulchild commented Aug 15, 2023

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!

@soulchild
Copy link
Author

soulchild commented Aug 15, 2023

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. ❤️

@soulchild
Copy link
Author

soulchild commented Aug 18, 2023

@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.

@konstantinkreft
Copy link

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

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

@soulchild
Copy link
Author

soulchild commented Aug 18, 2023

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
Copy link
Owner

ecyrbe commented Aug 18, 2023

thank you, will take a look

@soulchild
Copy link
Author

soulchild commented Aug 24, 2023

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.

@soulchild
Copy link
Author

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

@soulchild
Copy link
Author

Any news on this one, @ecyrbe? Thanks!

@ecyrbe
Copy link
Owner

ecyrbe commented Oct 9, 2023

No solution at the moment.

@soulchild
Copy link
Author

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?

@ecyrbe
Copy link
Owner

ecyrbe commented Oct 10, 2023

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

@soulchild
Copy link
Author

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! 🙏

Copy link

stale bot commented Nov 29, 2023

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.

@stale stale bot added the wontfix This will not be worked on label Nov 29, 2023
@soulchild
Copy link
Author

Please reopen, @ecyrbe. Thanks!

@soulchild
Copy link
Author

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

Copy link

stale bot commented Jan 10, 2024

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.

@stale stale bot added the wontfix This will not be worked on label Jan 10, 2024
@soulchild
Copy link
Author

Bump

@stale stale bot removed the wontfix This will not be worked on label Jan 10, 2024
Copy link

stale bot commented Feb 9, 2024

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.

@stale stale bot added the wontfix This will not be worked on label Feb 9, 2024
@soulchild
Copy link
Author

Bump

@stale stale bot removed the wontfix This will not be worked on label Feb 10, 2024
@toteto
Copy link

toteto commented Feb 22, 2024

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).

Copy link

stale bot commented Mar 23, 2024

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.

@stale stale bot added the wontfix This will not be worked on label Mar 23, 2024
@soulchild
Copy link
Author

bump

Copy link

stale bot commented Apr 22, 2024

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.

@stale stale bot added the wontfix This will not be worked on label Apr 22, 2024
@soulchild
Copy link
Author

bump

@stale stale bot removed the wontfix This will not be worked on label Apr 23, 2024
Copy link

stale bot commented May 26, 2024

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.

@stale stale bot added the wontfix This will not be worked on label May 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

4 participants