Wasm_of_ocaml support

[vouillon]

@hhugo @rgrinberg This is an alternative design. What do you think about it?

This adds a wasm mode. All the js_of_ocaml options are duplicated as wasm_of_ocaml options. There is a (js_of_ocaml (enabled_if <blang expression>)) and (wasm_of_ocaml (enabled_if <blang expression>)) option in env and executable stanzas to control whether the js and wasm modes are enabled.

I have not updated the documentation yet.

[samoht]

Why do I need to specify the submodes here? Dune could find out what to build for this wasm.js target on its own, isn't it?

[vouillon]

The documentation has not been updated. That should be the following above:

(executable (name foo) (modes wasm))

But yes, you can just use dune build.

Not that this was just copied from the Js_of_ocaml part of the doc:

dune/doc/jsoo.rst

Lines 38 to 44 in b409963

	And then request the ``.js`` target:
	.. code:: console
	$ dune build ./foo.bc.js
	$ node _build/default/foo.bc.js
	hello from js

[hhugo]

From a quick look, I think I prefer this approach better.

It seems you have not implemented the generation of a bc.js when js_of_ocaml is disabled, do you still want it ?

[hhugo]

L.Map.exists

[hhugo]

Don't we have an issue if byte_for_jsoo appears twice (once for js and once for wasm)

[hhugo]

Suggested change

	let equal = Poly.equal
	let equal (a : t) b = Poly.equal a b

[hhugo]

Suggested change

	let to_list (x : t) =
	let to_list ({ wasm; js}: t) =

[hhugo]

Suggested change

	match mode with
	| Mode.JS -> "js"
	match (mode : Mode.t) with
	| JS -> "js"

[hhugo]

Why Some and not None ?

[vouillon]

This is so that we don't need a case for None there:

dune/src/dune_rules/jsoo/jsoo_rules.ml

Lines 18 to 20 in ec321a5

	(match parent.enabled_if with
	| Some (Const default) -> default
	| _ -> assert false)

dune/src/dune_rules/jsoo/jsoo_rules.ml

Lines 603 to 605 in ec321a5

	(match js_of_ocaml.enabled_if with
	| Some (Const default) -> Memo.return default
	| _ -> assert false)

[hhugo]

I think it would be either to read if there were less hidden invariants. What do you think of the following ?

diff --git a/src/dune_rules/jsoo/js_of_ocaml.ml b/src/dune_rules/jsoo/js_of_ocaml.ml
index dc2fad30a..2c8c8e185 100644
--- a/src/dune_rules/jsoo/js_of_ocaml.ml
+++ b/src/dune_rules/jsoo/js_of_ocaml.ml
@@ -358,7 +358,7 @@ module Env = struct
     ; sourcemap = None
     ; runtest_alias = None
     ; flags = Flags.default ~profile
-    ; enabled_if = Some Blang.true_
+    ; enabled_if = None
     }
   ;;
 end
diff --git a/src/dune_rules/jsoo/jsoo_rules.ml b/src/dune_rules/jsoo/jsoo_rules.ml
index b10ac8f27..98b29afeb 100644
--- a/src/dune_rules/jsoo/jsoo_rules.ml
+++ b/src/dune_rules/jsoo/jsoo_rules.ml
@@ -12,18 +12,16 @@ let compute_env ~mode =
         let local = Js_of_ocaml.Mode.select ~mode local.js_of_ocaml local.wasm_of_ocaml in
         let* parent = parent in
         let+ enabled_if =
-          match local.enabled_if with
-          | None ->
-            Memo.return
-              (match parent.enabled_if with
-               | Some (Const default) -> default
-               | _ -> assert false)
-          | Some enabled_if -> Expander.eval_blang expander enabled_if
+          match Option.first_some local.enabled_if parent.enabled_if with
+          | Some enabled_if ->
+            let+ v = Expander.eval_blang expander enabled_if in
+            Some (Blang.Const v)
+          | None -> Memo.return None
         in
         { Js_of_ocaml.Env.compilation_mode =
             Option.first_some local.compilation_mode parent.compilation_mode
         ; sourcemap = Option.first_some local.sourcemap parent.sourcemap
-        ; enabled_if = Some (Const enabled_if)
+        ; enabled_if
         ; runtest_alias = Option.first_some local.runtest_alias parent.runtest_alias
         ; flags =
             Js_of_ocaml.Flags.make
@@ -601,8 +599,8 @@ let jsoo_enabled
   | None ->
     let* js_of_ocaml = jsoo_env ~dir ~mode in
     (match js_of_ocaml.enabled_if with
-     | Some (Const default) -> Memo.return default
-     | _ -> assert false)
+     | Some enabled_if -> eval enabled_if
+     | None -> Memo.return true)
 ;;
 
 let jsoo_enabled_modes ~expander ~dir ~in_context =

[hhugo]

This looks weird. Is it the case that wasm_files is always empty when mode is js ?

[vouillon]

Yes, that's the case:

dune/src/dune_rules/jsoo/js_of_ocaml.ml

Lines 227 to 234 in ec321a5

	and+ wasm_files =
	match mode with
	| Mode.JS -> return []
	| Wasm ->
	field
	"wasm_files"
	(Dune_lang.Syntax.since Stanza.syntax (3, 17) >>> repeat string)
	~default:[]

[hhugo]

Maybe we could add an assertion.

[hhugo]

@vouillon could you submit a version of ocsigen/js_of_ocaml#1724 using this PR ?

[vouillon]

It seems you have not implemented the generation of a bc.js when js_of_ocaml is disabled, do you still want it ?

It does not fit well. For the wasm_of_ocaml test suite, I have switched to using dune test stanzas and only had to add a few rules to copy a bc.wasm.js file to bc.js.

[vouillon]

See ocsigen/js_of_ocaml#1726

[hhugo]

This looks good, I left one last suggestion. The documentation still needs to be updated

[hhugo]

Does that comment still make sense now that jsoo and wasm options are different ?

[vouillon]

Yes, it still makes sense. In particular, one may want to pass a JavaScript file to wasm_of_ocaml to get some primitive purity information. Or one need to include some JavaScript code both when generating JavaScript and Wasm code. For instance, I have this for the virtual_dom package:

 (js_of_ocaml
  (javascript_files ../lib/virtualdom.compiled.js ./thunk.js ./hooks.js))
 (wasm_of_ocaml
  (javascript_files ../lib/virtualdom.compiled.js ./thunk.js ./hooks.js))

[hhugo]

Should this be Js_of_ocaml.In_context.default ?

[vouillon]

This is your code...
But yes, that should work.

[hhugo]

This is your code...

I know

[hhugo]

this could be lifted up and shared with the one inside js_of_ocaml above

[hhugo]

@hhugo I'm not completely sure what to do about the ```(wasm_of_ocaml (sourcemap ))option. The distinction betweenfile` and `inline` does not really make sense: there is no inline source maps in Wasm and the source maps are going to be in the `.bc.wasm.asset` directory. I can keep the three options `no`, file or ``inline`` to stay in line with Js_of_ocaml. Or I can have just two options ``yes`` / ``no``.

I don't know either. We could also let dune complain if one tries to use inline with wasm.

[vouillon]

I don't know either. We could also let dune complain if one tries to use inline with wasm.

Or rather it should complain for file since one does not generate a separate .map target file?

[hhugo]

This should be only_in_executable

[hhugo]

Should the documentation mention the external dependencies that are required for the build ?

[vouillon]

I now explicitly point to the github repository above.

[hhugo]

Approved. One will need to squash commits before merging

[vouillon]

I have made another clean-up pass.
I have also disallowed the enable_if option in libraries, like other build-related options.

[hhugo]

Opam depext is unused in setup-ocaml v3

[rgrinberg]

LGTM. Feel free to send the docs in another PR