-
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
feat: optionally provide LSP an instantiated GraphQLConfig #1432
Conversation
@zth awesome, thank you! can you allow passing in an instantiated instance to GraphQLLanguageService class as well? For monaco efforts this would allow us to modify the config object via user input/etc in React, with the current PR |
@acao Won't that already work...? |
@zth ah yes, it does work that way. we may be migrating GraphQLCache over to interface package anyways |
@divyenduz what do you think of this? |
@acao let me know if there's anything more you'd like me to do here. Would be awesome if this could be included in a release soon, I think this + the last progress you've made means I can drop my fork. |
@ardatan does this make sense? |
@zth i think ill merge it anyways, looks like it works the way it should |
Yes, this will be amazing and would ease the transition of VSCode extension from graphql-config v2 to v3. I was also thinking that LSP can itself have some defaults (like tsconfig has, making this zero-config), basically include all GraphQL files in the workspace etc. What do you think? |
@divyenduz agreed! so what should those defaults be? |
thanks so much @zth ! gonna kick out a release as soon as i merge and test the TS/TSX branch |
That's a great question. Maybe on the lines of:
OR
Relying on automatic filtering of documents vs definitions in There is a challenge though, To support a generic configuration like this. We will have to make the language server tolerant of duplicate types. |
@divyenduz great suggestions! personally, I think the best option would be the first - to default to the single source schema approach, and allow the user to configure in graphql-config, vscode, etc of choice. in the case of multiple schemas, the user can list multiple schema inputs for each project seperately, but then that way each project would define an entire schema without type collisions? is that something that will be an issue for users? |
I think that looks good too! Just one nit - my impression is that the common place for the schema file is in the root, not the src folder. At least that's my experience. Is it common to put it in src? |
@divyenduz In recent alpha, I added a support for legacy config. You have the same API like |
@kamilkisiela good to know! we are using things are looking great, i think i've almost got it working in web workers and the browser in a webpack setup? |
This allows supplying a pre-existing graphql-config to the LS. The idea is that external consumers, like a vscode extension, should be able to provide defaults and extensions of the existing config (or generate it programatically all together) in a way that can be supplied directly to the LS.
It's basically #1272 but rebased.