-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
[lsp-server] Support JS/TS files as schema files #3587
Comments
hey I hope to make a relase of the phase 1 rewrite today to solve this problem! it will be a new major release |
@acao that's amazing!! could you clarify a bit more about how it will be solved? I noticed it's sort of possible on the pre-release branch, but it's pretty slow, and the click-through doesn't link to the correct line (it seems like it calculates the definition's line within the gql tag, then links to that line within the file, so it's off by the number of lines between the start of the file and the start of the gql tag). |
@addiebarron oh dear, well that is not good news haha! i mean, it's great news in that it's the first unexpected bug report for the pre-release, as I have a few new test cases for what should be this scenario, so this is very very good to know something is missing or else I would have made a broken release! Though that pre-release I uploaded is a bit out of date, the issue is likely still present. Can you provide a few example file snippets of the mix of config/schema/code files etc to reproduce the jump to definition offset bug? Also, I'm sad to know it's much slower for you, I've been testing with a small group of maybe 12 files, so I was hoping to open a work project today as well, as I feared this could also be the case. A likely explanation for this (i will need to profile) is that many of the bugs I fix in this first stage of the rewrite involve properly updating and even writing to the cache when we were not before, due to bugs that just weren't caught in the unit test because the Thus why I added the new high-level integration suite at essentially the LSP protocol level, and thus revealed all these cache misses and issues that should not be happening (and tracking down the root and fixing, etc)! What's more, these caches and the parsing are in duplicate because of some flaws I inherited and haven't taken up to reconcile or worsened accidentally (until rewrite phase 2, where I consolidate the file and schema caches, and consolidate the parsing to ) As you may agree with your expertise, increased parsing and writes to and duplications of the various in memory caches means both way more in memory cacheing. Also, a benchmark suite should be added to measure performance before this release. Some memory profiling with a larger project might help me track down some issues to target with the benchmarking. Initial project load (even with 12 files) and the definition lookups seemed slower to me, what else feels slower to you? Just everything? How many code files with/without graphql tags approximately? is there anything special about the schema, for example custom schema extensions? (also btw this means i am not releasing! and it is soon monday here anyways 😆 ) I need to take more time with this, and if the second stage of the rewrite proves to be more performant measurably than the 1st stage, then perhaps we will join them together, perhaps as separate PR merges but the same major release. if both prove to be significantly worse off than what's currently stable, then first I timebox targeting the perf issues, and if they persist without major changes then I go back to the drawing board, because performance is already an issue, I can't bring myself to make it worse! If you wanted to try the 2nd stage rewrite out with vscode, and/or tinker with it (PRs against my PRs or main are welcome ofc!), you could take these steps:
|
Also thank you so much for taking the time to try out the pre-release, you saved us all so much pain haha |
I left a related issue on the graphql-config repository: kamilkisiela/graphql-config#1425
Current Behavior (if applicable)
I work on a project that defines its GraphQL schema in
gql
tags across a number of.ts
files in the server directory. We generate aschema.graphql
file from those code files in the pre-commit step using graphql-codegen, but during development the most up-to-date information about the schema is in thosegql
tags, not in theschema.graphql
file.In VSCode, clicking through any client-side definitions (e.g. command-clicking a query name in an operation) sends me to the generated schema rather than the typescript (which is what we'd consider the "source of truth").
Desired Behavior
If JS/TS files are passed to the graphql-config
schema
field, VSCode should construct a schema from thegql
tags in those files and use that as the source of truth.This could be accomplished using the
CodeFileLoader
included ingraphql-tools
.The text was updated successfully, but these errors were encountered: