feat: graphql-config@3 support in lsp server #1616
Conversation
| @@ -296,11 +292,7 @@ export class GraphQLCache implements GraphQLCacheInterface { | |||
| } | |||
| const filesFromInputDirs = await this._readFilesFromInputDirs( | |||
There was a problem hiding this comment.
@ardatan i had some questions about await project.loadDocuments(), as I think it could replace a lot of functionality we have in GraphQLCache. Our implementation has a different structure, where there is a collection item per file, and thus potentially multiple asts and other extracted data, whereas with Source[] it seems to be a collection of ASTs/strings with potentially entries with the same location file reference. I think we can at least get rid of our redundant logic to glob, read and parse files seperately here. There may be a bit of data missing for this to be accomplished, for which I can open a PR to graphql-config to add that data to these interfaces?
There was a problem hiding this comment.
@acao Sure! You can open a PR for that :)
There was a problem hiding this comment.
Looking into this further and I don't know if I'll need to actually!
paired with this PR, all of this is working:
https://github.com/prisma-labs/vscode-graphql/pull/184/files
however, there is some redunant logic between our GraphQLCache and MessageProcessor now, as of this PR, and I think I need to rewrite some of this. Either way, going to see if making it more graphql-config based in file loading will help! will definately make it more extensible!
There was a problem hiding this comment.
@ardatan the way that we cache some of the information about files, we need to store the range. it would be nice if there was a source.range in getDocuments() from which to compute offsets
There was a problem hiding this comment.
it seems i can address this issue with custom loader extensions as well. i need to be able to set specific parser configs as well. probably will be the solution in the long run
acao
force-pushed
the
1615-graphql-config
branch
2 times, most recently
from
514c37f to
19b47f2
Compare
acao
force-pushed
the
1615-graphql-config
branch
from
22f7c2e to
2d747b6
Compare
- load documents instead of 'includes" - use latest patch version
- pre-cache schema more frequently -fix messageProcessor test with config extensions - updated test to use real cache/config instances
acao
force-pushed
the
1615-graphql-config
branch
from
90dfb19 to
0b2e3c6
Compare
|
I still want to add tests to this, and whats more, it's had no peer reviews or testing, and has a major impact on most of the GraphQL IDE ecosystem! the greatest impact is on the server, but there are some wide-reaching and important bugfixes, some of which I turned into some small but critical PRs here that I have merged. So I may release these and a few other patches as hotfix patch releases across the ecosystem before merging this PR as a minor version bump to everything but |
acao
changed the title
acao
force-pushed
the
1615-graphql-config
branch
from
01188df to
b05ef54
Compare
move tmpdir default to logger
acao
force-pushed
the
1615-graphql-config
branch
from
b05ef54 to
a067257
Compare
acao
changed the title
- adopt `documents` instead of `includes`, and other `graphql-config@3` features - pre-cache schema on load and on `getDefinition` - load schema from `project.getSchema()` by default or from individual files with config - pre-cache all files in `documents` on initialization, which can be optionally disabled.
documentsinstead ofincludes[email protected]conversionBonus
WorkspaceSymbolDefinitionsTodo
This needs more tests!