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

Investigate removing dependency on result #274

Closed
hongchangwu opened this issue Nov 20, 2019 · 10 comments
Closed

Investigate removing dependency on result #274

hongchangwu opened this issue Nov 20, 2019 · 10 comments
Assignees

Comments

@hongchangwu
Copy link
Contributor

Is there really good reason to keep this compatibility module now that OCaml 4.03 is 6 (soon 7) versions ago.

The result package doesn't play well with the Result module shipped with 4.08 (see janestreet/result#8). The fact that container uses result means users of container (even transitively) have to use Stdlib.Result to access the Result module from the standard library.

@c-cube
Copy link
Owner

c-cube commented Nov 20, 2019

I imagine we could bump the minimal version to 4.03 and get rid of result and uchar.
Not sure if it's even worth waiting for 3.0.

@c-cube c-cube self-assigned this Nov 20, 2019
@c-cube c-cube mentioned this issue Nov 20, 2019
3 tasks
@pmetzger
Copy link

I thought Bucklescript still targeted 4.02.3 for the moment?

@c-cube
Copy link
Owner

c-cube commented Nov 23, 2019

Oh, didn't they release their version targetting 4.06? I'm not following closely…

@pmetzger
Copy link

They apparently are supporting both for the moment; BS 7 is apparently in beta and will target only 4.06. I'm not clear on how long it will be until BS 5 is considered deprecated.

@c-cube
Copy link
Owner

c-cube commented Nov 25, 2019

In any case it's a bit of a moot question since they don't want to use containers, but rather Belt/tablecloth/… right?

@ozanmakes
Copy link

My 2cents as a user of both Containers and BuckleScript:

  • I think I'm one of the few people using these together. I love using Containers as my standard library everywhere, but for BuckleScript I went for a quick & dirty fork. This allowed me to use some Js and Belt functions to (prematurely) reduce some overhead, and drop things that used Format module which affects the final bundle size a lot and not recommended when compiling with BuckleScript.

  • BuckleScript versions that target OCaml 4.06 (6.x.x and 7.x.x) seem to be on their way to become the main version. I see fewer and fewer releases of 5.x.x, and flagship tools such as genType dropped support for BS 5.x.x altogether without a period of supporting both.

There are still some work that needs to be done by the community, but all in all I think you should feel free to drop OCaml 4.02 support.

@c-cube
Copy link
Owner

c-cube commented Nov 25, 2019

@osener thanks for the feedback.
I wonder, if there'd be some kind of attribute ([@@no_bs]?) that one could put on definitions, so that BS would just ignore these when compiling? I'm not sure it's possible (the attribute might be lost by the time BS gets the AST, I'm not sure), but it would be neat to be able to support BS users and still keep the Format functions for the rest of us.

@ozanmakes
Copy link

Looking at my changes again, I see that I kept a lot of the pretty printing functions without having the same bundle size problems I had with Tablecloth (it's been a while). The rules of BuckleScript's dead code elimination is a bit opaque to me, so I mostly do it by trial and error (maybe it was harder with Tablecloth because it uses a single ml file?).

I also remember that I had trouble getting BuckleScript to compile Containers code that defined non-labelled interfaces over labelled definitions, so I deleted a lot of code and settled on *Labels modules because of my personal preferences.

Regarding [@@no_bs], I'm not aware of such attribute, but it would be fantastic to have Containers support BuckleScript out of the box with a quality JS output that's competitive with Belt. I sense that it was already an inspiration for Belt, so I'm hoping @bobzhang can provide some insight.

@bobzhang
Copy link

bobzhang commented Dec 2, 2019

@c-cube do you use cppo for conditional compilation?

@c-cube
Copy link
Owner

c-cube commented Dec 2, 2019

No, I removed cppo a while ago in favor of dune-driven code generation. It makes development more pleasant since I can use merlin.

@c-cube c-cube closed this as completed in 30251e9 Dec 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants