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

Melange compatibility #79

Open
jchavarri opened this issue Aug 15, 2023 · 2 comments · May be fixed by #80
Open

Melange compatibility #79

jchavarri opened this issue Aug 15, 2023 · 2 comments · May be fixed by #80

Comments

@jchavarri
Copy link

jchavarri commented Aug 15, 2023

@aantron would you support making this library compatible with Melange? I don't think many changes would be needed, mostly a couple of changes on opam and dune files. cc @davesnx

Using Melange would unlock a fix for #52.

@jchavarri
Copy link
Author

One thing that comes to mind is that because melange also uses dune, a new library should be added, maybe they could be called promise.native and promise.melange or something like that.

@davesnx
Copy link

davesnx commented Aug 15, 2023

I have vendored this project under server-reason-react on https://github.com/ml-in-barcelona/server-reason-react/tree/main/packages/promise, and changes that I made was:

  • Add dune file for js/
  • Remove type Result and result dependency on native/ (was there for compatibility with 4.02)
  • Remove type result on js/ (was there for compatibility with BuckleScript < 6)

Just my 2 cents about ReScript/Melnage. If there's a need for a compatibility with ReScript (#73) maybe we could migrate either js/promise.re to ReScript and keep it under a bsconfig folder or migrate to OCaml and add the dune with the modes melange.

I have done this similar stuff for styled-ppx and it's a decent solution

@davesnx davesnx linked a pull request Oct 4, 2023 that will close this issue
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.

2 participants