Unregister JSX effects

[aralroca]

All the effects are registered when the signal get is dispatched. However, we don't have a way to unregister it. An example is this:

export default function BugIssue({ }, { state }: any) {
  let user = state({ name: 'Aral' })

  return (
    <div>
      {user.value && <b>{`Name: ${user.value.name}`}</b>}
    </div>
  )
}

This is the transformed JSX in the return statement after the build:

return h('div', {}, () => user.value && ['b', {}, () => `Name: ${user.value.name}`]);

In runtime, each function is wrapped with an effect and is generating the DOM elements. This is why each time that a signal changes, the effect is running and updating the DOM elements.

However, now if the user changes to null, this effect that is already registered is fired again:

() => `Name: ${user.value.name}`

[aralroca]

This is the faling test (marked as todo): https://github.com/aralroca/brisa/blob/480576d2d28689af4cccea55d76041d98e0fb8ff/src/utils/transform-jsx-to-reactive/integration.test.ts#L2331-L2375

[aralroca]

@amatiasq I think this could be a solution, although before implementing it, I will think about it in case we find a better one. Tell me please what do you think about this proposal.

Possible solution (proposal)

We can upgrade the build from:

return h('div', {}, () => user.value && ['b', {}, () => `Name: ${user.value.name}`]);

to something like this:

return h('div', {}, (r) => user.value && ['b', {}, r(() => `Name: ${user.value.name}`)]);

and then in runtime register all the subeffects of each effect, and whenever it is executed in effect that first eliminates all its subeffects.

In the same way as if the developers use the effects of this style (rare case):

effect(() => {
   if(someSignal.value) {
     effect(() => {
        console.log(someSignal.value, anotherSignal.value);
     })
   }
})

could be transformed during build to:

effect((r) => {
   if(someSignal.value) {
     effect(r(() => {
        console.log(someSignal.value, anotherSignal.value);
     }))
   }
})

correcting later in runtime.

There will never be many cases of nesting in JSX since each web component is isolated from the others.

However, if there are more than 2 nesting levels:

effect((r) => {
   if(someSignal.value) {
     effect(r((r2) => {
         if(anotherSignal.value) {
            effect(r2(r(() => {
             console.log(nested.value)
            })))
         }
     }))
   }
})

One change I've made that would help with implementation

Currently, the effects are registered within each state:

https://github.com/aralroca/brisa/blob/d36c50b097ea39081975a5509950071bf122e5cc/src/utils/signals/index.ts#L18-L35

This makes it impossible to delete subeffects from the execution of an effect.

Besides, just I noticed an issue that the cleanAll is not deleting the effects. And the cleanAll is called when the component is unmounted.

To fix this I did this PR #10. I have now moved the effects to a higher layer to have control of them, and this way I have been able to solve the issue of unregistering all the effects when doing unmount. So after this change, it will make it easier to implement this proposal.

https://github.com/aralroca/brisa/blob/0eb25e7e6e4e6ac675d43f61c193f7bfbe3b4ac4/src/utils/signals/index.ts#L7-L9

[amatiasq]

It is a good idea, I'm not grasping it full since I can't see what r would be here.

But yeah, moving the effects to the parent closure opens a whole new way of addressing this problem.

[aralroca]

I realized that at JSX level it is not necessary to make any change at build level, because the effect control is inside the brisa element file, I have tested and it works perfectly.

Brisa element

r param

https://github.com/aralroca/brisa/blob/d61a1298b8fff8d163390e9513421d541a55d3f1/src/utils/brisa-element/index.ts#L116-L123

wrap r in effect

https://github.com/aralroca/brisa/blob/d61a1298b8fff8d163390e9513421d541a55d3f1/src/utils/brisa-element/index.ts#L173-L174

recursive r subwrap

https://github.com/aralroca/brisa/blob/d61a1298b8fff8d163390e9513421d541a55d3f1/src/utils/brisa-element/index.ts#L201-L205

However, at build level it is also necessary to solve the use of nested effects in the component.

Signal test about nested effects

https://github.com/aralroca/brisa/blob/d61a1298b8fff8d163390e9513421d541a55d3f1/src/utils/signals/index.test.ts#L192-L219

The tests are working fine now, just need to make this small adjustment in build-time so that the effects can be written without adding this r thing.

But it's almost solved. I will soon do the PR.