brisk-reconciler.ppx prevents usage of other standard JSX apis

[kyldvs]

I've noticed that the brisk-reconciler.ppx prevents using other JSX apis. I believe the ppx expects there to be a make function but the standard JSX api built in to reason uses a createElement function.

This comes up when trying to use something like Pastel.

print_endline(<Pastel color=Red> "Hello world" </Pastel>);

It also seems like the brisk reconciler ppx doesn't support list elements like normal Reason JSX:

<View> ...someListOfElements </View>

Is it possible to have the ppx translate let%component to the standard createElement form of JSX? Or is there some technical limitations around this?

[wokalski]

Good point... There are no technical limitations. In fact, instead of:

let%componet make = ...

People could type:

let%component createElement = ...

I like make because it mirrors reason-react but if the people wouldn't mind createElement, I wouldn't mind it either. The issue with children is much more serious. Special handling of children allows for:

1. Optional children
2. Explicitly not accepting children
3. Non-optional children
4. Special handling of JSX lists so that they are reconciled differently than dynamic lists.

The design is quite limiting. I wish there was a way to have different transforms. One (crude) solution would be to add [@brisk.nojsx] attribute that you can attach to arbitrary expression and the brisk jsx ppx won't be applied inside it.

[kyldvs]

I would strongly prefer let%component createElement = ... so that we don't restrict the set of features Reason offers when using this ppx.

---

Regarding children, I think 1-3 all have passable solutions with the standard treatment of children as a list.

1. I think this is the default, you can optionally pass or not pass children, (or if I misunderstand it can probably be solved the same as (3)).
2. When I want to design a component like this I mark the children as unit: ~children: list(unit). Now it is very hard to pass anything meaningful as children.
3. Here I would just use a prop rather than accepting the required element as a child. Not as nice syntactically I guess, but I prefer the slightly worse syntax over messing with the semantics of Reason's built in features.
4. I don't understand this enough to offer an alternative/determine importance, so maybe this one would warrant diverging :)

[wokalski]

1. You can. But it's restricted ~children=[] is passed by default. You cannot control the default value for instance.
2. ~children: list(unit) before we had any PPX ~children as _: list(unit) was omnipresent. It felt good to get rid of it.
3. I disagree it's messing with the semantics of Reason's built in features. [@JSX] is there for a reason and Reason-React has been leveraging it for a long time. I'd say that if anything, the builtin Reason's handling of children is suboptimal.

The API currently used by brisk-reconciler is the most natural one to use. Especially with things like optional "render props". It just works intuitively.

[kyldvs]

Yeah I do really like the API we have with this PPX, and it is natural to use! The unfortunate aspect is that it splits the Reason language. We can either use "Brisk JSX" or "Reason JSX". I don't think these conveniences warrant that split, but I understand other may disagree, which is fine.

My ideal scenario would be to bring these improvements to Reason's JSX where possible, but that is probably a lot of work.

In the meantime do you have any ideas how we could let the two kinds of JSX live side-by-side? [@brisk.nojsx] seems like a good place to start, not sure if file-level or expression level is better though

[wokalski]

[@brisk.nojsx] could work at any level. The mapper is defined here. You could use Ast_traverse.map_with_context objects to implement a mapping which contains some additional context. The context would contain information whether the attribute in a parent expression/structure.

[wokalski]

@kyldvs I asked Revery users for feedback regarding the JSX ppx. We'll see what they say. The patch to change JSX in refmt would be very simple. I once implemented a related change quickly. What's more @IwanKaramazow and Antonio are very helpful with those changes.

[glennsl]

A scoped @brisk.nojsx might be nice. Something like this perhaps:

[@brisk.nojsx Pastel.(
  print_endline(<Pastel color=Red> "Hello world" </Pastel>)
)];

Mixing several different kinds of JSX worries me, so it would be nice if "external" JSX code was clearly marked.