-
Notifications
You must be signed in to change notification settings - Fork 53
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
Add ReScript support #115
Add ReScript support #115
Conversation
Great stuff! Let me review/merge this in the coming days! 🙂 |
@sorenhoyer let me know if you want to discuss adding tests etc. |
@zth I'd like that. I Added one test for parsing of a query with a variable with ReScript syntax, but before I add more, I'd like to hear what you had in mind. :-) |
No that looks great! I think maybe extending them to making sure the can handle multiple tags in the same file, and also that what it picks up is valid GraphQL (so just parsing it via graphql-js without doing anything with the results) would be a good idea too, if you don't mind. |
…g it + Moved functions from languagePlugin-tests.ts to test-utils/* for reuse in FindGraphQLTags-tests.ts
@zth added multiple tags to the test and also parsing it with the relay-compiler to verify that it is valid graphql that is picked up. Not sure what else is necessary to test here, other than perhaps the default reason syntax, but maybe that is a little out of scope for this PR, since this is focussing on adding ReScript support. Let me know if you have any suggestions regarding the tests. I'm not so "creative" when it comes to finding test cases heh. ;-) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! 😄 Just one comment and this is good to go 👍
…." when parsing a file
@sorenhoyer one final thing 😅 Could you add an entry to the changelog? Please tag and thank yourself for the work in there too! ;) EDIT: And merge master? That way CI should start working for this branch as well. |
The error "reserveCache failed: Cache already exists" in the pibeline is probably related to this actions/cache#144 (comment) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉 Awesome, thank you so much for doing this!
Working example: https://github.com/sorenhoyer/react-relay-rescript-boilerplate
Todos: