Skip to content

ReasonML syntax support and language server integration#1488

Merged
JakeDawkins merged 9 commits intoapollographql:masterfrom
zth:vscode-extension-add-reason-relay-support
Dec 13, 2019
Merged

ReasonML syntax support and language server integration#1488
JakeDawkins merged 9 commits intoapollographql:masterfrom
zth:vscode-extension-add-reason-relay-support

Conversation

@zth
Copy link
Copy Markdown
Contributor

@zth zth commented Aug 26, 2019

Hi!

This is a draft PR of adding ReasonML support to the vscode apollo extension. It does not work yet from what I can tell, although I'm not quite sure if I'm testing it the right way.

I've tried to mimic how support was added for python/Ruby, and I'm looking for guidance on how to proceed here. Maybe someone from the team could have a look?

It can be tested by cloning https://github.com/zth/reason-relay, and opening the example folder. example/src/BookOverview.re contains something that looks roughly like this:

module Query = [%relay.query {|
  query SomeQuery {
   ...
  }
|}];

...where {| <graphql code here> |} is what should be syntax highlighted and sent to the language server.

I've added tests to make sure that the right code block is successfully extracted from the document, so that should be working, but I haven't managed to get the extension working locally here. Or something's not right with my changes.

There's a bunch of changes I've made, primarily to the graphql.re.json syntax file, that I've just copied and search replaced from the Python/Ruby, and that I honestly don't know whether they are right or if I'm missing something.

Anyway, I'd be very helpful for any help and guidance I can get. Thanks in advance!

…gration to vscode extension. Work in progress, not quite there yet.
@apollo-cla
Copy link
Copy Markdown

@zth: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

@JakeDawkins
Copy link
Copy Markdown
Contributor

@zth for some reason, it looks like the extension is having trouble starting up. When clicking the apollo logo at the bottom right, the showStats command isn't recognized by the extension. I'll look into it :)

@JakeDawkins
Copy link
Copy Markdown
Contributor

@zth Okay, from what I can tell, there are a couple issues, but I'm not 100% sure the cause of either.

The first is that there may be something wrong with the extractGraphQLDocumentsFromReasonStrings function, but I'm not sure what. It can't process files in the __generated__ folder. Adding **/__generated__/** to the excludes config option sidesteps those files, and from what I can tell it works better.

The second is something config-related. It errors and says it can't get the schema file, because it doesn't exist (although it does). It looks like it's trying to load the schema from the root of the repo (??). If I change the localSchemaFile path to ./example/schema.graphql it loads the schema properly

@zth
Copy link
Copy Markdown
Contributor Author

zth commented Sep 8, 2019

@JakeDawkins thank you for testing! I was able to get syntax highlighting working with the [%graphql ... syntax, but not [%relay.query ..., which is weird because the regexp I use seems to work fine when I test it manually. Didn't get the language server integration working though, so something's definitively up.

Anyway, I'll look into this a bit more in due time and get back here when I have any updates. Thanks!

@JakeDawkins
Copy link
Copy Markdown
Contributor

Your tagName option in the config may be messing up that extraction! Not sure, but it'd be something to check out, maybe leave that out of the config and try?

@zth zth changed the title [WIP] ReasonML syntax support and language server integration ReasonML syntax support and language server integration Nov 23, 2019
@zth
Copy link
Copy Markdown
Contributor Author

zth commented Nov 23, 2019

@JakeDawkins I finally got this working! 😅 I think this should be good then, I've verified locally and added a pretty simple test. However, something unrelated seems to fail in CI, and I can't think of anything I did that should affect that test. Any idea?

Thanks!

@zth
Copy link
Copy Markdown
Contributor Author

zth commented Dec 5, 2019

@JakeDawkins sorry to ping you, just very excited about this 😀 Anything I can do to help move this one along?

Thanks!

@JakeDawkins JakeDawkins merged commit f404758 into apollographql:master Dec 13, 2019
@JakeDawkins
Copy link
Copy Markdown
Contributor

@zth Sorry I forgot all about this!! I'll get this out in the next release :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants