[Feedback] Preserve JSX mode

Similar one, but not for modules

This looks really promising. I will do some testing with solidjs.

6 Likes

I created a test app with npx create-rescript-app, switched to rescript v12 and replaced react with solidjs.
The first impression is very promising. The app compiled without problems and solidjs could handle the generated jsx files well :partying_face:.

So preserve mode seems to be working with solidjs, which is great!

But sadly, I immediately ran into a problem with reactivity. This is not related to JSX preserve mode, but would still require an additional transformation step to make the code work.

In the example code there is a button component:

let make = props =>
  <button
    {...
    props}
    className="bg-blue-600 text-white"
  />

that is compiled to:

function make(props) {
  let newrecord = {...props};
  return <button {...newrecord}  className={"bg-blue-600 text-white"}/>;
}

The line let newrecord = {...props}; breaks reactivity since it basically copies all properties.
The line only appears when additional classes are added to the button component.

This button:

let make = props =>
  <button {...props} />

would compile to:

function make(props) {
  return <button {...props} />;
}

and works just fine.

I have to do further tests to see if there are any other problems, but still, getting rid of the “js-back-to-jsx” babel-transform, that is currently necessary, is a great step forward.

4 Likes

And one other thing. For some reason the generated res.jsx files still contain the JSX include:

import * as JsxRuntime from "react/jsx-runtime";

even though the include is not used anywhere.

1 Like

Glad to hear some progress is made! Thanks for diving back into it.

Is spreading props common in Solid? I guess what’s going on here in terms of ReScript is that JSX props are represented as records, and record updates are immutable. So, it needs to create a new record to be consistent with the expectations.

EDIT: Actually, I don’t see the reason for that happening either. That immutable update shouldn’t happen in preserve mode I guess.

Thanks for playing with this @Fattafatta!

We are tracking JSX preserve mode: unused import of "react/jsx-runtime" · Issue #7433 · rescript-lang/rescript · GitHub already.

4 Likes

Were you able to verify if the other issues you’ve had now works?

I just checked and it looks like the problem with the lower case component names is gone (custom components would result in different code than a standard component created with jsx.component). All components have upper case names now.

There was another problem with switch statements and solidjs signals. Here, the resulting code is slightly different (the curry._1 call is gone, which is to be expected since the change to uncurried), but the resulting code still contains an additional variable assignment that breaks reactivity:

@react.component
let make = () => {
  let maybe = createSignal(Some("option"))
  
  <div>
  	{
      switch maybe() {
  	  | Some(m) => m
      | _ => ""
  	  }->React.string
  	}
  </div>
}

compiled code:

function Broken(props) {
  let maybe = SolidJs.createSignal("option");
  let m = maybe();
  return <div>
  {m !== undefined ? m : ""}
  </div>;
}

The problematic part: let m = maybe();.

But I am not sure this can be “fixed” (from a solidjs perspective), since, from the compilers perspective, it is a meaningful optimization and it totally makes sense to do it.

And, as you already pointed out in another post: There is a perfectly usable workaround available, by using the <For>, <If>, <Show> components provided by solid.

So all in all I would say, we only need to solve the very first problem with let newrecord = {...props}; and everything should be fine.

2 Likes

Great! But to confirm - that same code in TS (as in - assign the maybe() call to a variable, do type assertion on that variable, and then use the refined result) would have the same issue, no?

1 Like

Yes that would be problematic too. For these cases, solidjs provides the createMemo function that can be used to create a derived signal that keeps reactivity.

You can theoretically do hat in ReScript too. The difference is that in ReScript, the problem is much less obvious, since the source code looks completely normal, and you wouldn’t get an error. The component would simply never update. So it’s much harder to spot the problem. But once you understand it, there are multiple solutions available that all work well in ReScript.

1 Like

Just out of curiosity, could you show an example of ReScript vs TS code where this is the case (much harder to spot the problem in ReScript vs TS)? Would be interesting to see if we can do something more about it.

Unlike React, Solid’s component functions only execute once on creation. Reactivity is done through a tracking context:

const [count, setCount] = createSignal(0)

<span>{count()}</span>

By calling count() inside of span, solid figures out that it needs to re-render <span> anytime that count changes

In @Fattafatta’s example, in the rescript code they put maybe() in between the <div>, which is correct, but the compiler put the maybe() function call outside of it in the output, thus calling the getter outside of the tracking scope

let m = maybe()

// not tracked
<div>{m}</div>

Right, thanks, and I follow all of that. My question is about contrasting TS and ReScript code, and specifically where the ReScript code is much less clear than the equivalent TS code. I’d like to see how the TS code looks when it does the same thing (take a value, refine the type of it so you can use it, and then use it).

Ah got it. Solid provides a <Show> component that will act as a guard against truthy values in TS:

<div>
  <Show when={maybe()}>
    {m => <div>m.foo</div>}
  </Show>
</div>

For ADTs they have a switch / match component, but it doesn’t provide refinement

Right, thanks! But this would look identical in ReScript, no? We could also bind to <Show /> in the same way?

When working with SolidJS, you always have to keep the basic reactivity concepts in mind, the interactions between signals, subscribers and tracking scope.

Signals provide getter and setter functions to work with data. If a getter is used within a tracking scope, all code within that scope will be rerun if the signal updates.

A subscriber basically defines a tracking scope. The most simple example would be the createEffect function. Any getter used within createEffect will “subscribe” to the corresponding signal and will automatically be updated (rerun) whenever the signal changes.

Any JSX component will also create its own tracking scope.

This has some interesting implications when working with SolidJS. For example the following code would not work:

function App() {
  const [maybe] = createSignal("hi");

  return (
      maybe() ? <div>{maybe()}</div> : null
  );
}

Even though the getter (maybe()) is used correctly (used twice), The first one is outside of any tracking scope. So the component will never update.

Just adding a fragment (<>) would fix this, because the fragment would create its own tracking scope.

function App() {
  const [maybe] = createSignal("hi");

  return (
      <>maybe() ? <div>{maybe()}</div> : null</>
  );
}

This is of course somewhat hacky, and therefore it is recommended to instead use the <Show> component.

function App() {
  const [maybe] = createSignal("hi");

  return (
    <Show when={maybe()}>
      <div>maybe()</div>
    </Show>)
}

The Show component will automatically generate a tracking scope and subscribe to all relevant signals.

While at first, this seems complicated, it is something that every experienced SolidJS programmer will know well, and they would easily spot errors like the one above.

The same would be true for this snippet (that would not work either):

function App() {
  const [maybe] = createSignal("hi");
  const m = maybe();

  return (
      <> m ? <div>{m}</div> : null</>
  );
}

It is easy to spot, that the getter is called outside a tracking scope. So no updates will be triggered.

The point I’m trying to make is that, although SolidJS can be complicated, it is also very explicit. In many cases the error is obvious and easy to spot, just by looking at the code.

Basically this line const m = maybe() would be a huge red flag.

Now let’s have a look at ReScript.

@react.component
let make = () => {
  let maybe = createSignal(Some("hi"))

  switch maybe() {
    | Some(m) => <div>{m->React.string}</div>
    | _ => React.null
  }
}

I would say that for the average ReScript programmer this would look like valid code. A SolidJS programmer, coming to ReScript, would probably think the same. (Although they might be irritated by the use of a getter inside the switch statement.)

But in order to get the error right away, you need a decent understanding of how the compiler transforms the code to JavaScript.

The problem only becomes obvious when looking at the generated JS code. The getter is clearly called outside a tracking scope:

function App() {
  let maybe = SolidJs.createSignal("hi");
  let m = maybe();
  if (m !== undefined) {
    return <div>
    {m}
    </div>;
  } else {
    return null;
  }
}

The problem also persists if using the “fragment trick” in the TypeScript example. Adding a fragment would result in this code:

function Broken(props) {
  let maybe = SolidJs.createSignal("hi");
  let m = maybe();
  return <>
  {Primitive_option.some(m !== undefined ? <div>
    {m}
    </div> : null)}
  </>;
}

That’s why the problem is much more relevant for ReScript than it is for TypeScript. Those transformations and small optimizations that the ReScript compiler does, can lead to problems that would not happen in TypeScript.
And those changes are hidden when only looking at the ReScript code.
You have to know how a ReScript switch statement is represented in JavaScript, in order to be able to spot the problem.

So what I’m trying to say is, that neither a ReScript programmer coming to SolidJS nor a SolidJS programmer coming to ReScript, will have the necessary understanding and intuitions about the code. Because it is necessary to have a good understanding of the ReScript compiler and of reactivity in SolidJS.
This can easily lead to frustration and the believe that SolidJS and ReScript do not work together.

(Honestly it would be interesting to know, how many ReScript programmers actually check the resulting JS code. My assumption is that most people do, but I’m not sure about it).

But those small optimizations are one of the great features of ReScript (at least for me), that make it so much easier to write good code without having to think too much about it.
Only in this very specific case, when using SolidJS, it sadly makes it more complicated.

So, from my perspective, solving those problems should not be done by changing the way the ReScript compiler behaves. It is more about finding a good way to make those interactions more explicit and easier to spot.

My first step would be to just add a little more documentation and simply explain those interactions. That would make it much easier for a newcomer, to avoid those mistakes.

Thinking about it. When I update rescript-solidjs for ReScript V12, I will probably just add some explanations in the documentation. That should already be a great improvement.

2 Likes

Thank you for the thorough write up! It would be interesting to think about whether there’s anything additional we can do to make the clearer/easier to work with. I look forward to seeing how things look with v12 and rescript-solidjs!

1 Like

While updating the bindigs to v12, I ran into another complication.

SolidJS uses slightly different DOM attributes. For example in SolidJS class is allowed, and there are also special ones like classList that exist only in SolidJS.

With v11 I was using the generic JSX transform feature to modify the domProps type by providing my own definition of the Elements module (as described in the docs):

module Elements = {
  type props = {
    ...JsxDOM.domProps,
    class?: string /* Solid also allows class */,
    classList?: classList,
    textContent?: string,
  }
}

With v12 it seems generic JSX transform and preserve mode are mutually exclusive. (At least in my tests I could not get them to work together). Which seems intuitive at first glance, since defining your own function names is not necessary if those functions are never created.
But now I can no longer use it to modify domProps.

I found a solution that works, by simply providing my own JsxDOM module:

module JsxDOM = {
  type style = JsxDOMStyle.t
  type domRef

  type popover = | @as("auto") Auto | @as("manual") Manual | @as("hint") Hint

  type popoverTargetAction = | @as("toggle") Toggle | @as("show") Show | @as("hide") Hide
  type domProps = {
    ...JsxDOM.domProps,
    class?: string /* Solid also allows class */,
    classList?: string,
    textContent?: string
  }
}

But this seems much less elegant than the previous solution.

Would it be possible to have JSX transform and preserve mode active at the same time (with preserve mode taking precedence)?

Or do you have another idea on how to solve this in a cleaner way than just overriding JsxDOM?

With v12 it seems generic JSX transform and preserve mode are mutually exclusive. (At least in my tests I could not get them to work together).

What exactly are you testing? Can you give a playground example?

JSX transform needs to be active for JSX preserve to work.
So, I’m a little lost about what you are trying.

Sorry, I don’t know how I can set the necessary compiler flags in the playground.

But I just tried to build a simplified example and something strange happend.

I have this simple component:

@jsx.component
let make = () => {
 <div className="text"></div>
}

When using these settings in my rescript.json:

{
  "bsc-flags": [ "-bs-jsx-preserve" ],
  "jsx": {
    "version": 4,
    "module": "MyOwnJsx"
  }
}

it actually works and the component gets compiled as expected.

function Btn(props) {
  return <div className={"text"}/>;
}

But when I change the code in my MyOwnJsx.res:

module Elements = {
  type props = JsxDOM.domProps

  @module("myownjsx")
  external jsx: (string, props) => Jsx.element = "jsx"

and remove the @module("myownjsx") part:

module Elements = {
  type props = JsxDOM.domProps

  external jsx: (string, props) => Jsx.element = "jsx"

the code will be compiled to:

function Btn(props) {
  return jsx("div", {
    className: "text"
  });
}

So I think the solution would be to simply provide the dummy @module entry. But the bahaviour was a bit surprising for me.