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

Add [@js.dict], [@@@js.require], [@@js.capitalize] #174

Closed
wants to merge 3 commits into from
Closed

Conversation

nojb
Copy link
Member

@nojb nojb commented Jun 1, 2024

This PR adds the possibility of annotating an OCaml type expression (string * ty) list with [@js.dict] to indicate that values of this type should be mapped to JS objects in the obvious way (and vice-versa).

@nojb
Copy link
Member Author

nojb commented Jun 3, 2024

The second commit adds support for [@@@js.require "foo"], which generates in the implementation side a let-binding

let ___require = Ojs.apply (Ojs.variable "require") [|Ojs.string_to_js "foo"|]

and sets the global object to be __require. This simplifies binding Node libraries, eg

[@@@js.require "crypto"]
type hash
val create_hash: string -> hash [@@js.global]

would bind https://nodejs.org/api/crypto.html#cryptocreatehashalgorithm-options.

The generated javascript contains require("crypto") (both in the dev and release profiles) with a literal string argument (which seems important, see #66 (comment), cc @jchavarri), but only if the string "crypto" does not appear elsewhere in the file (because JSOO shares constants whenever it can), opened ocsigen/js_of_ocaml#1619 about this.

@nojb nojb requested a review from mlasson June 3, 2024 06:49
@nojb nojb changed the title Add support for [@js.dict] Add support for [@js.dict] and [@@@js.require] Jun 3, 2024
@nojb nojb removed the request for review from mlasson June 3, 2024 06:55
@nojb
Copy link
Member Author

nojb commented Jun 3, 2024

The third (and last!) commit adds the possibility of telling JSOO to use CamelCase instead of camelCase for the generated JS names, which is useful to bind certain APIs and much cleaner than using [@js "CamelCase"] everywhere. (The same could be achieved by naming the OCaml labels _camel_case, but it is not very nice...).

@nojb nojb changed the title Add support for [@js.dict] and [@@@js.require] Add [@js.dict], [@@@js.require], [@@js.capitalize] Jun 3, 2024
@nojb
Copy link
Member Author

nojb commented Jun 3, 2024

Prior art for the require part: #66 and #68

@nojb
Copy link
Member Author

nojb commented Jun 11, 2024

Friendly ping.

@mlasson
Copy link
Member

mlasson commented Jun 12, 2024

@js.dict

I am not sure we want to add the [@js.dict] mainly because I am not super fond of the "Ojs.iter_properties" (I would love to deprecate it actually).

To implement your dictionnaries, you should be able to do what you want by creating a module like:

module Dict = struct 
  type 'a t = (string * 'a) list 
  let t_of_js f t =
    let l = ref [] in
    iter_properties t (fun k -> l := (k, f (get_prop_ascii t k)) :: !l);
    !l

  let t_to_js f x =
    let t = empty_obj () in
    List.iter (fun (k, v) -> set_prop_ascii t k (f v)) x;
    t
end

Then you can replace ((string * t) list [@js.dict] by t Dict.t.
I've just realized that this example was mentioned in "LOW_LEVEL_BINDING.md .

@js.require

The current way to live without require is to add a custom JavaScript stub that enrich the global object with some object imported from modules (using one of the various module system found in the JavaScript ecosystem). I am not completely against providing an out-of-the box support a require annotation but it will be better to check if it works not only with node but also with bundling tools like browserify and webpack.

@js.capitialize

Do you think it would be interesting to support a module annotation (ie. @@@) for that ?

@nojb nojb closed this Jun 12, 2024
@nojb nojb deleted the nojebar_dict branch June 12, 2024 09:47
@nojb
Copy link
Member Author

nojb commented Jun 12, 2024

Thanks for the review! The changes here are not critical; I suggest we punt this and revisit it later if/as needed.

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 this pull request may close these issues.

2 participants