Signals when there is multiple layers do not get observed.

[AlbertSabate]

Describe the bug Having a considerable extensive depth, a signal does not get observed on the JSX

To Reproduce The depth of the signal is as follows: Page -> web-component loads a signal from store -> in the JSX add a function which has a switch which renders another JSX.

TODO: Do a reproduction repository.

Expected behavior Signals should have to always be triggered.

[aralroca]

I expect the reproducible example to create that failing test 💪🏽

[AlbertSabate]

Repo: https://github.com/zenet-technology/brisa-forms (private repo, @aralroca has access to it)

1. bun dev
2. The page is a form. Fill the form i18n-input as its required and click submit
3. No error will be displayed on the input group called object
4. Go to the code and uncomment this line https://github.com/zenet-technology/brisa-forms/blob/0ab50de87bebd7dbdbbf8d8a79adb01248c51879/src/web-components/form/controlled.tsx#L244
5. Repeat from point 2. And the error will appear.

[aralroca]

I understand that the correct behavior displays that error, right? This error comes from the Server Action:

I understand that without the console.log it stops being reactive for some reason, and with the console.log it becomes reactive again and the error is shown. Is this the expected behavior?

I don't know what it could be. I have done a diff of both files and the only change is the console.log, there is no bug in build-time. So it's probably a signals registry runtime bug.

[AlbertSabate]

I understand that without the console.log it stops being reactive for some reason, and with the console.log it becomes reactive again and the error is shown. Is this the expected behavior?

Yes. It should always be reactive as my understanding. It's very weird that if I put the console.log in front works. console.log should be harmless, but it make everything work. Should work even if you remove the console.log

[aralroca]

I need to investigate why it happens, so I have no idea for now.

[aralroca]

As you use the switchElements function inside the JSX, to the console.log when doing a getter of a signal, it makes that when the signal changes, it executes again the whole function switchElements, and therefore the reactivity affects all the JSX of the return of this function, not only to where the signal was registered. This is similar to return () => <div /> explained to Ryan Carniato here.

On the other hand, for some reason, the signal you have in the return (JSX) is not registered correctly (this is the issue).

At least now I understand why it works with the console.log. However, it is still necessary to determine why the signal is not registered inside the JSX. Let's see if I can generate a failing test with this.

[aralroca]

For some reason wrapping with a div solves the problem. Probably is something related to documentFragment... Let's continue investigating...

+<div>
  {issuesErrors.value?.[
    `${prefix ? `${prefix}.` : ""}${element.props.name}`
  ] && (
    <error-message class="-mt-8">
      {(
        issuesErrors.value?.[
          `${prefix ? `${prefix}.` : ""}${element.props.name}`
        ] ?? []
      )
        .map((e) => e.message)
        .toString()}
    </error-message>
  )}
+</div>

[aralroca]

@AlbertSabate I think I got the failing test 💪🏽

https://github.com/brisa-build/brisa/blob/798a3b7d7328695a3b11c036ab2f6f310dc5b763/packages/brisa/src/utils/brisa-element/index.test.ts#L3215-L3252

[aralroca]

I see that the reactivity is executed, it creates the node with the last changes, the problem is that it adds it in a DocumentFragment, without being visible in the DOM.

[aralroca]

Alternative 1

I have a change idea during rendering that will most likely fix it and reduce our code. The bad part is that it will perform more DOM operations instead of grouping them as we were doing so far.

To understand a bit of context, in some cases we are using a documentFragment to group DOM operations. The bad part is that in some cases, as we have seen, there is a signal registered to an element that as a parent has the documentFragment, therefore, although it is reactive and changes, it is not reflected in the DOM.

This is our current process (simplified, without slots, danger HTML, etc), to understand better how it works now the web component render.

So the idea is to remove the documentFragment and use the element as parent in this step:

I will try to make a POC of this idea to see if it solves the problem by continuing to pass all the tests we currently have.

Alternative 2

Another possible alternative would be: to maintain the documentFragment, but at the moment that the internal effect is executed after a reactivity that when accessing the parent it is the correct one and not the documentFragment.

This alternative would add more code instead of removing it, but perhaps it is better in terms of performance.

It will be great of testing these 2 alternatives and measure them in performance issues, at the same time by not using the documentFragment also the user will see the changes a few milliseconds earlier although it requires more blocking work.

[aralroca]

https://developer.mozilla.org/en-US/docs/Web/API/DocumentFragment#performance

"The performance benefit of DocumentFragment is often overstated. In fact, in some engines, using a DocumentFragment is slower than appending to the document in a loop as demonstrated in this benchmark. However, the difference between these examples is so marginal that it's better to optimize for readability than performance." — MDN

We've identified that our initial optimization attempt here, which relies on DocumentFragment, does not yield significant performance benefits and, in fact, adds unnecessary complexity to the codebase. This approach introduces maintenance challenges, especially in terms of keeping reactivity intact in certain scenarios. Moving forward, we should prioritize readability and focus on optimizations that offer a clear advantage rather than micro-optimizations that complicate the implementation without tangible improvements.

[aralroca]

Okay, it's not a rendering problem. It's a problem with the signal optimizations we do in build-time.

The call of the function foo() is considered that it is a signal and that's why it wraps it with a function:

const times = 10;
const Component = ({}, { store, derived }: WebContext) => {
  let count = 0;
  const show = derived(() => store.get('show'));

  const foo = (num: number) => {
    return [
      null,
      {},
      [
        ['div', {}, [null, {}, /* Here is the problem -> */ () => (num < times ? foo(num + 1) : '')]],
        [null, {}, () => show.value && 'show' + num],
      ],
    ];
  };

  return foo(++count);
};

Removing the arrow function () => (num < times ? foo(num + 1) : '') to num < times ? foo(num + 1) : '' solves the problem.

I think the problem is related with the markup generator support here:

https://github.com/brisa-build/brisa/blob/f1e9846f58083e95838b56d2a20d88638e41e548/packages/brisa/src/utils/client-build-plugin/transform-to-reactive-arrays/index.ts#L214-L239

[aralroca]

In the end, it was a rendering problem. The set of fragments and temporary containers sometimes had disconnected nodes. They were reactive, but they were not part of the DOM.

When this happens, it is always the shadowRoot. This change fixed all the failing tests while keeping the others passing fine.