janestreet / bonsai

A library for building dynamic webapps, using Js_of_ocaml
MIT License
367 stars 39 forks source link

Cannot build examples/hello_world #10

Closed nekketsuuu closed 4 years ago

nekketsuuu commented 4 years ago

I followed the "Getting Started" tutorial to build examples/hello_world, but the build failed with the following errors:

$ cd ./examples/hello_world
$ dune build ./main.bc.js
Entering directory '/mnt/c/Users/nek/dev/src/github.com/janestreet/bonsai'
File "src/environment.ml", line 1:
Error: The implementation src/environment.pp.ml
       does not match the interface src/.bonsai.objs/byte/bonsai__Environment.cmi:
       Values do not match:
         val add_exn : t -> 'a Key.t -> 'a data -> t
       is not included in
         val add_exn : t -> key:'a Key.t -> data:'a data -> t
       File "src/environment.mli", line 7, characters 0-64:
         Expected declaration
       File "src/univ_map_intf.ml", line 47, characters 2-45:
         Actual declaration
File "src/eval.ml", lines 26-31, characters 4-41:
26 | ....let%pattern_bind result, apply_action =
27 |       let%mapn input = Value.eval environment input
28 |       and model = model in
29 |       compute ~inject input model, apply_action ~inject input model
30 |     in
31 |     Snapshot.create ~result ~apply_action
Error: This expression has type
         'a Ui_incr.Incr.t = ('a, Ui_incr.state_witness) Incremental.t
       but an expression was expected of type
         (model, action, result) Snapshot.t
Done: 304/310 (jobs: 1)

Is this a wrong way to build, or is this a bug of HEAD of this repository? If this is a bug, is there any CIs to check builds?

Environment

TyOverby commented 4 years ago

You're using the Head of Bonsai, but some fairly old versions of its dependencies. You can use the version of Bonsai on opam without issue, but if you want to build the tip-of-tree on github, you'll need to use the latest versions of its dependencies (also on github). You can do that easily by adding the janestreet opam-repository to your opam switch: https://github.com/janestreet/opam-repository for your opam switch

nekketsuuu commented 4 years ago

Thanks for your explanation!

Then, how about adding version constraints to dependencies written in the opam file? https://github.com/janestreet/bonsai/blob/b6c1a0e7a2f1e9df829383a3cd708c950502856e/bonsai.opam#L12-L21

If so, we newcomers can notice that we are using the old dependencies when building the example according to https://github.com/janestreet/bonsai/blob/b6c1a0e7a2f1e9df829383a3cd708c950502856e/docs/getting_started/open_source/hello_world.mdx.

TyOverby commented 4 years ago

If version requirements were added, then opam would just say "no version found" for all the packages; which isn't much more helpful...

We're looking at ways to detect this and issue a more clear warning; thanks for bringing this up though!