PPX-less API for creating DOM elements and props.

[glennsl]

As discussed in https://github.com/ml-in-barcelona/jsoo-react/discussions/113. This adds a PPX-less API using plain functions to create DOM elements and props.

I didn't manage to actually get started using jsoo-react this week, so none of this is actually tested in the real world. And there's a few essential parts missing still, see the TODO list below. This is a still good point to critique the general approach though.

TODO:

• [x] Flesh out Event.Pointer
• [x] Implement ref prop
• [x] Implement style prop
• [x] Implement dangerouslySetInnerHTML prop
• [x] Add SVG elements and props
• [x] Add proper interface
• [x] tests
• [x] Context API?
• [x] forwardRef?

Closes #105

[glennsl]

A couple of issues have been surfaced from rewriting the tests:

1. Namespace pollution causing naming collisions

Importing all these names into the namespace is bound to cause collisions. There are a couple ways we can address this:

a. Import the names in a larger scope. This pollutes even more, of course, but at least allows local bindings to shadow imports. And if done at the top of the file will always allow shadowed names to be accessed by fully qualifying them.

b. Selective imports. Not very idiomatic OCaml, and quite verbose, but this, along with point a above, is how it's done in Elm etc.

c. Qualify each invocation, perhaps using an aliased short-name, e.g. module H = React.Dom.Html, then H.div [|H.className "foo"|] [].

d. Avoid binding all these names by using an API like h "div" [|p "className" "foo"|] [] for example.

e. Bow down to the deity that is PPX.

2. Conditional props

There's currently no direct support for conditionally passing props. With the current API this is done using optional labeled arguments, and in JSX is done by borrowing the same semantics, but this isn't possible when creating an array directly.

A few options to address this:

a. Offer something like Prop.none as an alias to any "" "", effectively offering a no-op that can be used with if-expressions.

b. Offer some primitives for conveniently concatenating arrays, e.g. Prop.(concat [ [|className "foo"|]; if bar then [|href "..."|] else [||] ]).

c. Offer some kind of builder-style API to build a props array.

d. Bow down to the deity that is PPX.

[jchavarri]

Thanks for the work on this and detailed explanation on issues :)

1. Namespace pollution causing naming collisions

I feel inclined to follow either your suggestion in a. (import the names at the top of the module where the component implementation is being defined). or, using the approach you already applied in existing code (just rename prop to append some underscore, or something renaming solution). Alternatively, selective imports sound great as well, but as you say it can be quite cumbersome in this case, as there are tens of elements and props that are used in a single component tree.

2. Conditional props

Hm, this is an inconvenient issue indeed. Maybe an alternative option besides the ones you mention (kind of building on option b.) is to add a helper to produce either empty array or array with some prop, then combine this with Array.concat, e.g. for the example in PR:

(* In Dom_html.Prop *)
  let option f value = match value with
  | Some v -> [|f v|]
  | None -> [||]

(* In tests *)
let%component make ~href:href_ =
  React.Dom.Html.(a (Prop.option href href_) [])

Then can be used in combination with Array.concat when other props are involved:

let%component make ~href:href_ =
React.Dom.Html.(
  a
    (Array.concat
       [ Prop.option href href_
       ; [|className "value"; id "some-large-id"; hidden true; lang "es"|]
       ] )
    [])

---

Bow down to the deity that is PPX.

I hope with the work you're showcasing in this PR we can get to a point where PPX-less is decent enough 💪 😄 Then we can build PPX on top of it, for those that like adventure and want more thrill in their lives.

[glennsl]

Re: namespace pollution, I agree with your inclination and definitely don't think this is a deal-breaker, just a bit awkward. The error messages currently emitted when this happens are also not very nice, but should improve when the prop type is abstracted.

(* In Dom_html.Prop *)
let option f value = match value with
  | Some v -> [|f v|]
  | None -> [||]

Good idea! And inspires me to make a further refinement! What if instead we define option to be a prop modifier that hides the no-op:

(* In Dom_html.Prop *)
let option prop = function
  | Some value -> prop value
  | None -> any "" ""
(* val option : ('a -> prop) -> 'a option -> prop *)

(* In tests *)
let%component make ~href:href_ =
  React.Dom.Html.(a [|option href href_|] [])

[jchavarri]

And inspires me to make a further refinement! What if instead we define option to be a prop modifier that hides the no-op

Looks great! And it avoids the need to create empty arrays just for this purpose ✨

[glennsl]

What do you think about option vs maybe for the name?

let%component make ~href:href_ =
  React.Dom.Html.(a [|maybe href href_|] [])

I think maybe reads better, and often use it to name optional variables too since maybe_value reads better than value_opt or something. Then again, option might be more suggestive of this modifying the prop to take an option value.

[glennsl]

Or optional...

[jchavarri]

I like maybe! 🙂 option is really overloaded already...

[glennsl]

Made some further refinements, including a breaking change to the Context API to align it with rescript-react and allow both Reason and OCaml-style components to be created.

The last thing puzzling me is the forwardRef API, which I can't even understand the purpose of.

Apart from that, I think the only thing that remains is a proper interface.

[glennsl]

Interfaces done

[glennsl]

Make ppx changes so that instead of translating lowercase components to calls to createElement we just leave calls to div, p and such (user should put them in scope)

What's the reason for this? To aid migration?

Migrate types somehow from Html module in ppx to new Dom_html

What are these really used for? Outside of the PPX itself I mean.

[jchavarri]

What's the reason for this? To aid migration?

The main reason is that, if we do:

• the above (make Reason ppx use the new DSL)
• fix locations so they make sense to merlin (this is complex task, but as soon as I can find some time I'd like to tackle it because it's crucial)

then we get all docs and editor tooling upsides for free. You can hover over <div ..> and merlin will show the docs of the div function in the dsl, etc etc. I am not sure yet we can make everything work (props etc), but I am pretty sure it's a much clearer path than that we were going down before (using ppx to do this stuff).

What are these really used for? Outside of the PPX itself I mean.

They are used for the ppx transformation only, yes. But that plays a fundamental role at the moment on the lowercase component type checking. The ppx will generate type annotations for each prop, so you get compiler tell you about props with wrong types. But again, we get all this "for free" when we generate calls to DSL functions 🎉

[glennsl]

...then we get all docs and editor tooling upsides for free

I see. Yeah I think that makes sense. But we'd also get that even if we fully qualify the functions, won't we, and that would hopefully make it a seamless transition?

But again, we get all this "for free" when we generate calls to DSL functions :tada:

:tada:

[jchavarri]

Thanks for the work on this PR @glennsl. As there were conflicts with main I solved them and merged manually.

But we'd also get that even if we fully qualify the functions, won't we, and that would hopefully make it a seamless transition?

I am not sure I get this part, could you elaborate?

---

re: the tests (keeping both Reason and OCaml) I am not sure it makes a lot of sense once we move to using the dsl for everything. Reason syntax should already be covered by ppx tests, so there's not much advantage on keeping 2 files for the "end to end" tests.

[glennsl]

I am not sure I get this part, could you elaborate?

To quote what you said originally:

Make ppx changes so that instead of translating lowercase components to calls to createElement we just leave calls to div, p and such (user should put them in scope)

You've explained well the reason for using the new DSL instead of createElementdirectly, but not why the user should put the DSL in scope.

It seemed to me that fully qualifying all the DSL calls (e.g. React.Dom.Dsl.Html.div) would make the transition seamless. But now that I've thought it through a bit more, I realize this will only work for known HTML elements. It wouldn't work for SVG or anything else, unless we have some convention for "escaping" the HTML scope.

I also realize another problem with using the DSL instead of createElement directly, that it would no longer support custom elements and custom props. Although JSX doesn't support actual custom elements anyway, since they should contain a dash (-) which isn't a valid character in an identifier. But not supporting custom props is a bit of a bummer.

[glennsl]

Another question on your suggestions for next steps:

Add docs and make sure they work with ppx locations 😬

What kind of docs, and level of detail, do you imagine this should be?

[jchavarri]

Continuing discussion about namespacing in https://github.com/ml-in-barcelona/jsoo-react/pull/128#issuecomment-1024908940.

---

What kind of docs, and level of detail, do you imagine this should be?

I am not sure. For initial version I think just showing the allowed props for each element would be awesome, but I am not sure yet how this would work, as the props is just a list, not a function with labelled args.

I have to think more about this 🤔