solidjs / solid

A declarative, efficient, and flexible JavaScript library for building user interfaces.
https://solidjs.com
MIT License
32.43k stars 925 forks source link

Operation with && in event handlers via props causes the 'outside a createRoot' error #1715

Open timothyallan opened 1 year ago

timothyallan commented 1 year ago

Describe the bug

It was suggested that the Discord support thread I raised should be logged as an issue, so here goes:

Why does button 1 generate a warning? Doesn't it just evaluate to boolean anyways?

Your Example Website or App

https://playground.solidjs.com/anonymous/d5fe0e41-701e-4360-b991-867f9e57811f

Steps to Reproduce the Bug or Issue

Click button 1, check out the console warning.

Expected behavior

No warning.

Screenshots or Videos

No response

Platform

MacOS, all browsers

Additional context

No response

atk commented 1 year ago

Events, timeouts, promises are all out of the normal execution context and therefore, also out of the reactive root, which allows registering signals as dependencies.

A simple solution is to use an event signal to move the event into the reactive system:

const [ev, setEv] = createSignal();
createEffect(on(ev, () => {
  console.log(otherSignal());
}, { defer: true }));
return <div onClick={setEv}>...</div>;
timothyallan commented 1 year ago

Thanks for the suggestion. Using the Boolean wrapper (button 2 in the playground) seems to be a substantially less involved solution, I'll stick with that method I guess.

it still seems odd from a DX perspective that using one reactive signal 'works', but &&'ing 2 causes the issue.

Sending in a combined reactive function works as well, so you can combine them beforehand: https://playground.solidjs.com/anonymous/bb8d1861-28b1-4148-bb36-3fe1c81d44bd

ryansolid commented 1 year ago

Yeah thinking about this. This is a bug. I changed some stuff in the transform a few versions back to fix one problem but introduced this one. We will look at getting this fixed up.

ryansolid commented 1 year ago

Hmm.. I can't think of a way with how our current reactivity works to solve both issues. Either we process the prop too eagerly which isn't acceptable (and can break things like hydration) or we have this issues. We could just drop this optimization which has performance implications. We basically need a lazy memo that self releases to fix this. Putting this firmly in Solid 2.0 territory.