search

graphql / vscode-graphql

MIGRATED: VSCode GraphQL extension (autocompletion, go-to definition, syntax highlighting)
https://marketplace.visualstudio.com/items?itemName=Prisma.vscode-graphql
MIT License
557 stars 71 forks source link

0.3.0 Release #181

Closed acao closed 4 years ago

acao commented 4 years ago

for 3.0.0 milestone

Note: This requires some upstream PRs to the lsp monorepo itself: https://github.com/graphql/graphiql/issues/1615

acao commented 4 years ago

updated the list, PR should be ready by tomorrow i might have a companion PR for graphiql repo where we need to make LSP tweaks as well

acao commented 4 years ago

part of my goal here is to re-architect schema lookup/loading across the LSP/extensions/etc. graphql-config@3.0.0 should be a bit more performant, and provides some new interfaces that will allow us to delete a lot of redundant, low performance code from both the LSP and this extension.

schema lookups happen with almost every event the language server handles, so the more efficient it can be, the faster the LSP server will be.

acao commented 4 years ago

if folks are interested in end to end testing this, there are two active branches you'd need to yarn link together. PR #184 and graphql/graphiql#1615 are the ones. i will provide steps later. just clone both, checkout branches, yarn watch in graphiql and run the vscode extension debugger for vscode-graphql, and open a project with a graphql-config@3 setup, see readme changes for migration instructions! any and all feedback is welcome. it's been quite an effort fully replacing watchman for LSP client file watcher pattern, however, now pre-loading the schema and even other documents (fragments, operations) is working in a few helpful ways following the convention.

the only thing left is testing, and testing with larger schemas and more complex projects.

acao commented 4 years ago

If anyone can manually confirm this works for them locally today, I would feel confident to release as is.

"works on my machine" is not good enough for 300K user installs, in addition to all the other extensions for vscode and other IDEs that use our underlying LSP and LSP server, etc.

the changes we have to make to the LSP server to recover full-definition availability after removing server-side file watching are quite substantial, and without testing on some much larger schemas and codebases, I can't confirm that it won't incur greater performance issues than watchman already was presenting!

what's more, I can't guarantee that the pre-loading will also work as well as watchman itself, despite it's many drawbacks. for example, i found that on a fresh load, i still had to ctrl-click twice on an input type for it to jump to the schema, even though the schema was already cached according to the console

homoky commented 4 years ago

Release it as is. Users will provide feedback for quick fixes.

acao commented 4 years ago

ok, i just need to make over a dozen NPM releases first to the projects this one depends on and ones related to the required changes, and then I can update this PR with the latest release versions and then release here!

acao commented 4 years ago

https://marketplace.visualstudio.com/items?itemName=GraphQL.graphql-vscode-insiders

hey good news! this is an "insiders" version extension, if you want to give this a try. again, make sure to use graphql-config 3. it is working for me locally when i install from the marketplace. be sure to disable this 0.2.x extenasion first before enabling the "insiders" edition, and then you'll see "reload required" as well.

there are a few outstanding issues:

for this release, I want to make sure these features work:

divyenduz commented 4 years ago

Thanks for all the awesome work. I tried it on a very old project and observed this: https://www.loom.com/share/83ffbc3c5c404a7c961291b000510072

Jump to definition seems to work only in the current file. Unless the other file is opened once and save is pressed: https://www.loom.com/share/74bbca815ed34595bfe600b12507ae22

Probably an LSP issue though. Most other things seem to work for me 👍 🎉

acao commented 4 years ago

@divyenduz thanks for testing it! ah well thats the main issue in this effort, the schema preloading and thus definition cache warming. what happened when you ctrl-click an input type or fragment twice? usually it still loads in the cached schema file for me. maybe your cached schema file isn't working?

also, do you see the bugfix where you can see variables completion? or the new feature for variable name completion?

acao commented 4 years ago

@jgoux moving discussion to here: re config paths in your testing in #145 - I see! so nested graphql config isn't working. good catch. so, I think we have a few things to address here.

the extension is configured to activate only when it detects graphql config files from the workspace root path. however, in the extension settings you are able to supply baseDir to even change the directory where graphql config files can be found, and this technically works... if the extension manifest was configured to activate for this case! so i just need to change a file blob in the manifest in package.json, and custom, non-workspace-root config paths should work!

if you have multiple packages in a monorepo that use the config, the most ideal is to have one root config with multiple projects specified, with seperate schema/documents configs if need be. now with common js config you use process.env directly and can require sub configs for execution headers, etc even if you want from there.

but if you only have one package in a monorepo using graphql config, then I can understand why this would be frustrating. with the underlying implementation, it loads schema config for each file based on its path. if no schema is available in the config for that file path, then the extension isn't active beyond highlighting.

acao commented 4 years ago

in your case, @jgoux, if you have just one nested config file, and want to open the the monorepo root as the workspace, I think this should actually work: "vscode-graphql.config.baseDir": "./packages/web"

acao commented 4 years ago

@divyenduz i think I reproduced your issue! I had foolishly forgotten to clear the cache directory and forgot some changes where we use fs.mkdir to create the new nested schema cache directories! should just use mkdirp at this point, no need to manually do that when we can just use something recursive. so the schema file doesn't cache, thus no uri to jump to, no definition cache entries, etc. have a few tweaks to make for another insiders release today, I think!

acao commented 4 years ago

ohh, and it will require another graphiql LSP alpha release as well. no big deal though!

jgoux commented 4 years ago

@acao I just tried putting the setting in my .vscode/settings.json file : 

{
  "vscode-graphql.config.baseDir": "./packages/web"
}

But the config is still not detected.

acao commented 4 years ago

@jgoux strange!

seeing as this feature of having configs at subdirectories wasn't offered before, we might have to revisit it at a later date.

the latest release will fix the schema pre-cacheing, sorry i almost released it yesterday but I'm trying to recouperate more, and will be taking vacation very soon!

jgoux commented 4 years ago

@acao No worries, all of this is very promising!

Have a nice holiday! 🎉

acao commented 4 years ago

@jgoux thank you! looking forward to full vacation! just taking it easy this week and it's been nice.

this morning i've issued a 0.3.5 release that should fix some of the bugs. vsce packager is very strict about npm resolutions!

jump to definition should work shortly after restart (works fine when i install the extension from the marketplace on a fresh vscode instance!). for the first definition lookup, you may have to ctrl/cmd-click twice, or 'jump to definition' twice, where it fails the first time and pops up the error panel.

if you're more patient and wait til all ts/js inference is complete (loader in status bar) it sometimes works on the first try. I'm hoping to improve this, and get to the bottom of whatever is causing this issue. i need to address whatever is launching the output panel on the first definition lookup - perhaps because it's an error. with the older version that used watchman, was there an issue like that with the first definition lookup? if you, say, peek definition within 60s of reloading vscode?

i think it may have to do with the use of sync fs methods, though I did include an alpha release to the filesystem server that at least ensures all write methods are async again. maybe i should make another pass to ensure they all are async read. i'm considering just adopting fs-extra, for this and instead of using mkdirp

jgoux commented 4 years ago

@acao I can confirm that as you said, the jump to definition is working on the second try! Is there anything else I should look closely?

acao commented 4 years ago

fabulous news! thanks for checking. maybe see how schema changes take with definition lookup, and try workspace symbol lookup if you can? maybe also make sure variables type completion and named variable completion are working for operations?

also make sure to delete /tmp/graphql-language-service on your local when trying now, to simulate a fresh install, before schema has been cached

also, you can try operation execution if you want, with validated variables input, etc

acao commented 4 years ago

we may be very close to a stable release here!

jgoux commented 4 years ago

This is the ouput I got when booting VSCode with no files open : 

8/14/2020, 5:42:42 PM [4] (pid: 19285) graphql-language-service-usage-logs: {"type":"usage","messageType":"initialize"}

{
  fsPath: '/var/folders/vr/b32yycl13ylg3_sjwy88sp980000gn/T/graphql-language-service/clovis/projects/default/schema.graphql'
}
8/14/2020, 5:42:42 PM [1] (pid: 19285) graphql-language-service-usage-logs: invalid/unknown file in graphql config documents entry:
 './packages/web/src/{components,pages}/**/*.tsx'

8/14/2020, 5:42:42 PM [1] (pid: 19285) graphql-language-service-usage-logs: Error: 
      Unable to find any GraphQL type definitions for the following pointers:

          - ./packages/web/src/{components,pages}/**/*.tsx

Then I open two files, and hover a type in one of my template string : 

8/14/2020, 5:43:56 PM [4] (pid: 19285) graphql-language-service-usage-logs: {"type":"usage","messageType":"textDocument/didOpen","projectName":"default","fileName":"file:///Users/admin/Documents/Code/Clovis-team/clovis-labs/clovis/packages/web/src/components/Task.page.tsx"}

8/14/2020, 5:44:10 PM [4] (pid: 19285) graphql-language-service-usage-logs: {"type":"usage","messageType":"textDocument/didOpen","projectName":"default","fileName":"file:///Users/admin/Documents/Code/Clovis-team/clovis-labs/clovis/packages/web/src/components/Task.tsx"}

[Error - 5:44:21 PM] Request textDocument/definition failed.
  Message: Request textDocument/definition failed with message: Definition not found for GraphQL type task
  Code: -32603

Then I hover the type a second time, at this point I can go to the definition : 

8/14/2020, 5:46:07 PM [4] (pid: 19285) graphql-language-service-usage-logs: {"type":"usage","messageType":"textDocument/definition","projectName":"default","fileName":"file:///Users/admin/Documents/Code/Clovis-team/clovis-labs/clovis/packages/web/src/components/Task.tsx"}

8/14/2020, 5:46:07 PM [4] (pid: 19285) graphql-language-service-usage-logs: {"type":"usage","messageType":"textDocument/didOpen","projectName":"default","fileName":"file:///var/folders/vr/b32yycl13ylg3_sjwy88sp980000gn/T/graphql-language-service/clovis/projects/default/schema.graphql"}

8/14/2020, 5:46:07 PM [4] (pid: 19285) graphql-language-service-usage-logs: {"type":"usage","messageType":"textDocument/didClose","projectName":"default","fileName":"file:///var/folders/vr/b32yycl13ylg3_sjwy88sp980000gn/T/graphql-language-service/clovis/projects/default/schema.graphql"}

On subsequent hovers, I have the fsPath intruction prepended : 

{
  fsPath: '/var/folders/vr/b32yycl13ylg3_sjwy88sp980000gn/T/graphql-language-service/clovis/projects/default/schema.graphql'
}
8/14/2020, 5:46:58 PM [4] (pid: 19285) graphql-language-service-usage-logs: {"type":"usage","messageType":"textDocument/definition","projectName":"default","fileName":"file:///Users/admin/Documents/Code/Clovis-team/clovis-labs/clovis/packages/web/src/components/Task.tsx"}

8/14/2020, 5:46:58 PM [4] (pid: 19285) graphql-language-service-usage-logs: {"type":"usage","messageType":"textDocument/didOpen","projectName":"default","fileName":"file:///var/folders/vr/b32yycl13ylg3_sjwy88sp980000gn/T/graphql-language-service/clovis/projects/default/schema.graphql"}

8/14/2020, 5:47:09 PM [4] (pid: 19285) graphql-language-service-usage-logs: {"type":"usage","messageType":"textDocument/definition","projectName":"default","fileName":"file:///Users/admin/Documents/Code/Clovis-team/clovis-labs/clovis/packages/web/src/components/Task.tsx"}

Things that still don't work for me :

I hope this helps! 😄

jgoux commented 4 years ago

It seems like it's the first go to definition hovering that triggers the schema file dumping? It would explain why on the second try it works. The expected behaviour would be to dump the file as soon as the GraphQL service is running?

acao commented 4 years ago

@jgoux the behavior is triggered on the initialization event, actually! see here. thats why its confusing that it doesn't work on first click. my guess is that it's not done loading, or a race condition

acao commented 4 years ago

awesome, thanks @jgoux ! all of your own issues are known!

1st is a feature that can wait until a patch release. I'll remove it from the readme for now.

2nd is a feature i'm preparing to add in another round of iteraton once we make a stable release (it's behavior that will impact codemirror-graphql and graphiql as well, so it will take some time.).

the 3rd is a bugfix that I was hoping could wait til another patch release as well, but it's pretty annoying. you'll notice that it jumps to the graphql template string line - so if you jump to something that is 10 lines down in the template string, it jumps to 10 lines down from the top of the file instead. just need to add an offset like we do for diagnostics and completion :)

acao commented 4 years ago

ah! the baseDir option is not being handled by the server like I'd thought, that and maybe the offset fix I will try to target for another LSP server release to graphiql monorepo here, and then we can cut a release

huv1k commented 4 years ago

I am using 0.3.5 and still getting cache document errors:

[Error - 2:59:39 PM] Request textDocument/documentSymbol failed.
  Message: Request textDocument/documentSymbol failed with message: A cached document cannot be found.
  Code: -32603
acao commented 4 years ago

@huv1k yes, you'll see those eventually, but the initial cache builds by the second ctrl/cmd-click or jump to definition attempt, and then works from there.

so @jgoux - as per baseDir I have spent some time chasing down how to get extension config loading on the server side, and it's not gonna be easy because of our implementation. the original implementation from years ago manually generates connection, rather than using createConnection in vscode-languageserver, we use vscode-jsonrpc directly. so while the docs for this look pretty straighforward, our server architecture doesn't permit that approach, without re-inventing much of what createConnection does

I'm very used to this manual approach, however without it, we are missing some key features. so, introducing workspace settings to the language server will probably need to wait until a successive release

so until then, you can just put a graphql.config.js file in the root that requires the downstream config somehow? or whatever approach works?

jgoux commented 4 years ago

@acao No problem, I have two graphql.config.js, one for the codegen living in my package, and the other at the root for this extension 👍

acao commented 4 years ago

https://github.com/graphql/graphiql/pull/1651 < just need to tune this PR up a bit and most of the outstanding issues should be resolved! will get a chance after next week

acao commented 4 years ago

0.3.1 has been released! thanks everyone for helping

0.3.2 will include the edge case option for SDL first schemas. it's built in, just need to pass the config from server

huv1k commented 4 years ago

Thanks for hard work, I was testing it in the morning and it works flawlessly. Great job!

josh18 commented 4 years ago

Trying this out, looking pretty good.

A couple of things I noticed:

In the milestone notes it has use endpoints extension headers so there might be another way to set headers?

acao commented 4 years ago

@josh18 thanks for the validation note! what is the scalar type you are using for that variable

for the second point, you need to configure endpoints extension as before for headers, the readme has an example!

acao commented 4 years ago

@josh18 did it work when you configured the endpoint and headers in the graphql config?

acao commented 4 years ago

https://github.com/prisma-labs/vscode-graphql#usage

josh18 commented 4 years ago

Sorry was sick yesterday so didn't get a chance to test.

did it work when you configured the endpoint and headers in the graphql config?

Yup that works! I think I assumed the readme was stilled aimed at v2 and based on the issue from graphql-config I thought there was a different way of doing things in v3.

what is the scalar type you are using for that variable

Scalar type is ID. Looking into it a bit more it appears GraphQL treats IDs as strings but allows numbers as inputs as well which are then converted to strings. https://graphql.org/learn/schema/#scalar-types https://stackoverflow.com/a/47888604/1661462

acao commented 4 years ago

@josh18 hey thanks for looking into this! i knew i was overlooking a scalar type, haha. i remember writing a response to this while camping but I don't think it went through

I'm going to close this ticket since 0.3.1 is released. can you open a new ticket for this particular bug? will target this easy fix for 0.3.2 :)