Skip to content
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

Update to new GraphQL ppx #117

Closed
huy-nguyen opened this issue Oct 30, 2019 · 4 comments · Fixed by #119
Closed

Update to new GraphQL ppx #117

huy-nguyen opened this issue Oct 30, 2019 · 4 comments · Fixed by #119

Comments

@huy-nguyen
Copy link
Contributor

huy-nguyen commented Oct 30, 2019

The ppx mentioned in the README is not maintained anymore (announcement). It also may not support BuckleScript 6 (source). Please consider using this one instead.

@huy-nguyen huy-nguyen changed the title Update link to GraphQL ppx Update to new GraphQL ppx Oct 30, 2019
@gugahoa
Copy link
Contributor

gugahoa commented Oct 30, 2019

I think it's worth mentioning both, and let users choose

@huy-nguyen
Copy link
Contributor Author

I see that the old ppx is specified explicitly in bsconfig.json:

https://github.com/FormidableLabs/reason-urql/blob/d6a2dbf6de3794cc218ba08828f068cf4445f2f4/bsconfig.json#L32

Does that require people to install the old one if they just want to use the new one?

@Schmavery
Copy link
Collaborator

I don't think the ppx is used anywhere in the library, I wouldn't be surprised if that option could be safely removed from the bsconfig.

That said, I think it makes sense to mention graphql_ppx_re in the README as a first option, maybe something like you can use [graphql_ppx_re](link) (which is based on [graphql_ppx](link)).

As things move towards bs-platform 6, it seems like it would be nice to have the support and to guide people towards the more active library.

@parkerziegler
Copy link
Contributor

@Schmavery is correct, I believe that flag can be safely removed. The only reason it was included was back when I was first prototyping the examples in repo, but we are long past that point. If anyone wants to make a PR to remove it and update the docs, I'll be happy to review – otherwise I can get to it tomorrow! I do think it'd be good to direct folks towards the more actively maintained lib, so if folks are interested in contributing you could also update the in-repo examples!

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 a pull request may close this issue.

4 participants