-
-
Notifications
You must be signed in to change notification settings - Fork 38
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
Typescript performance issue with large queries in a single file #89
Comments
This issue is appropriately in the |
@JoviDeCroock I tried that, errors the same. While trying that I see different behavior worth mentioning:
https://github.com/deathemperor/gql.tada.sample/blob/main/src/queries.ts#L1266
|
Hm, so there's a couple of issues:
Past these two issues it comes down to the simple stuff: the file contains a lot of queries. It's basically irrelevant whether fragment masking is turned on or off. This is likely just a bad experience overall as all queries in one file are all checked at once. We can likely try to find bottlenecks here and optimise, but it's hard to imagine this being a supported case 🤔 since nothing is split up and separated properly. Currently, my tracing modifications to tsserver aren't good enough though to be granular enough to catch the bottlenecks I'll have to put in further work to try to narrow down which type inferences are the most expensive ones for us |
Renaming here, since I have a suspicion this is a general issue with queries of this size, especially if they're in the same file. Where I'm currently at is that some type inferences seem to take longer than I expect (trace showing some work suddenly being spent pushing minimum time in The other problem that's pretty visible is that Marking as “help wanted”We'll basically need more examples and reproductions that exhibit this behaviour, potentially with smaller schemas to narrow the problem down. |
We found a small piece of the puzzle and addressed this in: #107 |
I have to remedy this performance issue using codegen, most API like ResultOf/VariablesOf/graphql.scalar still works and the same graphql() usage is kind of the same so it's a good solution while waiting for this to improve. |
Another piece of the puzzle found and addressed in: #111 (Releasing as We refactored code in the parser that could cause bottlenecks but found that these code paths were not actually changing performance at all. But this basically narrowed the issues down to string literal performance. This doesn't seem to be a perfect solution just yet and there's some more things to do. But I didn't want to wait with releasing this, since it already helps quite a bit. |
I'd like to put a bit of a time-cap on this issue and close it for now. The performance over the last few versions should have improved, and — at least for now — I don't see many paths that could lead to improved type inference performance. Some situations are still worsened by TypeScript eagerly evaluating types it doesn't need to; either on type hints, which seems to re-evaluate several levels deep, or on making changes/editing, which causes TypeScript to partially disregard its type cache as well, it seems. For now, the remaining situations that I've seen that cause issues often have repeating patterns of:
However, over the last few versions we've improved performance substantially, and I think we've reached a point where I'd like to focus on improving the experience overall first.
We'd of course re-open this issue, or replace it, once we have more information, more input from TypeScript team members, or more ideas for a path forward to further improving this. |
Thanks @kitten , I value your efforts on the matter. I admit that our set up isn't the best one as I'm still on learning path. Having said that, I really want to adopt gql.tada for our projects as we move toward that path after working with codegen for almost 3 years. I frequently retry the performance and typings with every releases but it's still a blocking issue for us so it's a stay with codegen for now. Looking forward for more updates on the subject! |
This is a little unrelated to query-parsing performance in general, but seems to cause a general uplift in performance across the board. We just released This reduces memory usage and decreases start-up type checking performance, but due to decreased pressure on the type checker, also seems to help with general updates after the initial startup/checking process. The new output format can also be generated with the CLI ( |
Using When using direct graphql type declarations (without fragments), the issue goes away. |
@Maximaximum: There is a length limit to each document, and if you mean you're using fragments in the exact document you're defining, you may have reached the token limit of what can be parsed in the TypeScript checker efficiently. In short, there's an assumption to how large a single document can be, and that's now basically enforced through TypeScript's limits. If you have anything more concrete and do believe this is an issue and this isn't related to the length of your individual documents, I'm happy to take a look at a reproduction ❤️ Side-note: Locking this issue, since new replies won't be related to the milestone created, linked issue, or the original issue here |
Describe the bug
It takes 10+ seconds to show types (any type, not just types from graphql queries) and cause eslint the about the same amount to run fixes (I assume it's because eslint typescript waits for the type server to result). This means our DX at the moment basically reverts to Javascript world (we're using it on our production code base).
In the video, I tried to make a change on line 13 and then hover on
data
to see the type. once shown, data hasany
which is caused byType instantiation is excessively deep and possibly infinite.
Screen.Recording.2024-02-25.at.5.57.04.PM.mov
This issue is more about @0no-co/graphqlsp than gql.tada I suppose?
Reproduction
https://github.com/deathemperor/gql.tada.sample
gql.tada version
gql.tada 1.2.1
@0no-co/graphqlsp 1.4.1
Validations
The text was updated successfully, but these errors were encountered: