search

LexiFi / gen_js_api

Easy OCaml bindings for Javascript libraries
MIT License
177 stars 31 forks source link

Fix #104 #134

Closed mlasson closed 3 years ago

mlasson commented 3 years ago

This seems to be the more reasonable fix (proposed by @hhugo) .

Fix #104

alainfrisch commented 3 years ago

Does this force a dependency on a more recent of js_of_ocaml than what we require today?

mlasson commented 3 years ago

Does this force a dependency on a more recent of js_of_ocaml than what we require today?

So recent that it does not exist yet actually !

alainfrisch commented 3 years ago

Don't we want to keep this open, and merge when a release of js_of_ocaml with those primitives is available?

alainfrisch commented 3 years ago

Btw, the PR references from #104, namely https://github.com/ocsigen/js_of_ocaml/pull/997, has been merged on js_of_ocaml's master branch on May 6th, 2020, and I can see there has been several releases of js_of_ocaml since then (https://github.com/ocsigen/js_of_ocaml/blob/master/CHANGES.md). In particular, the changelog for release 3.7.0 mentions "Runtime: Change the semantic of MlBytes.toString, introduce MlBytes.toUtf16". Isn't it the one we need?

mlasson commented 3 years ago

The change in semantics happened when ocsigen/js_of_ocaml#997 was merged but they never added the primitive proposed in #104. It means that since 3.7.0 "Ojs.get/set/delete" is broken when applied to non ascii keys (which is not the end of the world; I am not aware of any API with non ascii characters...).

I propose a new solution to be able to close the issue: I deprecated these functions and introduced : 

external get_prop_ascii: t -> string -> t = "caml_js_get"
  (** Get the property from an object (only works if the property key is a plain ascii string). *)

external set_prop_ascii: t -> string -> t -> unit = "caml_js_set"
  (** Set an object property (only works if the property key is a plain ascii string). *)

external delete_prop_ascii: t -> string -> unit = "caml_js_delete"
  (** Delete an object property (only works if the property key is a plain ascii string). *)

external get_prop: t -> t -> t = "caml_js_get"
  (** Get the property from an object. *)

external set_prop: t -> t -> t -> unit = "caml_js_set"
  (** Set an object property. *)

external delete_prop: t -> t -> unit = "caml_js_delete"
  (** Delete an object property. *)

The ppx now use the ascii version only on ascii "properties" and fallback to "proper UTF-16 encoding" when needed (through string_to_js !).