-
Notifications
You must be signed in to change notification settings - Fork 42
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
Ast mapper version is not compat with bs-platform v6 #77
Comments
https://github.com/mhallin/graphql_ppx/blob/master/src/bucklescript/graphql_ppx.ml#L1-L2 We're doing this conversion to downgrade to |
Ok. It's not that simple 😄 |
It looks to me as though no code changes are necessary. The executable probably just needs to be compiled against OCaml 4.06. It's using OCaml-migrate-parsetree so it should just work. |
I just verified my assumption that no code changes are necessary. I built the PPX with OCaml 4.06 and copied the binary to my node_modules in a project that depends on BuckleScript 6.X. graphql_ppx works as expected in such scenario. |
@anmonteiro DO you think there is a sane way to have both 5.X and 6.X support? From what I tested when compiling with OCaml 4.06 it fails with 5.X. I assume we have to switch in CI and build two versions of every binary for both 5.X and 6.X. |
@baransu yeah, we are always going to need 2 binaries. The best alternative I see is to do what I recently did for the ReasonReact repo (reasonml/reason-react#392) and use 2 esy files, one for each version of the OCaml compiler |
@anmonteiro That's nice. Thanks! |
It's possible to have one binary. I am doing that in Bisect_ppx, using a patched ocaml-migrate-parsetree (which I should probably just release). |
Hi @mhallin,
I've tried to use new bs-platform and it's not compat with graphql_ppx
bsb output is
My package.json
The text was updated successfully, but these errors were encountered: