Topics

SolidJS•8mo ago

computations created outside a `createRoot` or `render` will never be disposed

Escaping reactivity contexts is presumably usually because of async operations. Well, I create this effect inside an event handler. My component is attached to a specific part of the DOM, creating a small island of reactivity. However the event in question is a keydown event attached to the document.

onMount(() => {
    document.addEventListener('keydown', kbHandler)
})

onCleanup(() => {
    document.removeEventListener('keydown', kbHandler)
})

onMount(() => {
    document.addEventListener('keydown', kbHandler)
})

onCleanup(() => {
    document.removeEventListener('keydown', kbHandler)
})

a) Is this the cause of the warning or am I mistaken? b) What is a good way to do this? Docs talk about delegated events that are attached to the document, but dispatched to the element (uncessary in this case, but whatever). But I tried <div onKeyDown={kbHandler}> and it won't fire my handler.

38 Replies

peerreynders•8mo ago

Have a look at from

martixyOP•8mo ago

Tnx, I will. Am I right about what causes the warning?

peerreynders•8mo ago

To be honest I'm still dubious. from should only be necessary for event listeners outside of the DOM Example

Solid Playground

Quickly discover what the solid compiler will generate from your JSX template

Brendonovich•8mo ago

I create this effect inside an event handler.

to be clear, you're calling createEffect inside kbHandler yeah? if so the warning would be bc the event handler doesn't create a reactive root, so for the effect to dispose properly you need to call createRoot and dispose it manually

martixyOP•8mo ago

you're calling createEffect inside kbHandler yeah?

Correct.

peerreynders•8mo ago

To what end?

martixyOP•8mo ago

To, uh... use signals inside the handler?

peerreynders•8mo ago

What are you trying to accomplish?

martixyOP•8mo ago

Add a download progress bar to a page that doesn't have one. The download function has an onprogress callback, where I set the signal and I want to update UI as a response to that. @Brendonovich I'm not sure what to do with createRoot. What about getOwner/runWithOwner?

peerreynders•8mo ago

In from the produce function gets a setter from a signal in the originating reactive context—can you make that work without a createEffect inside the handler?

Brendonovich•8mo ago

const dispose = createRoot(() => {
  createEfffect(...)
})

const dispose = createRoot(() => {
  createEfffect(...)
})

If you want to dispose the effect when the component unmounts then yeah you could use getOwner & runWithOwner

martixyOP•8mo ago

@peerreynders I am very new at solid and trying to parse what's going on. Ah, I see. runWithOwner looks awfully similar to .bind() 🙂 Ah, it isn't...

peerreynders•8mo ago

I'm just trying to solve this problem without having to manually create and dispose of reactive contexts. The problem seems to be that you want to affect change inside an existing reactive context from the outside. To some extent that is from does for events. createResource/createAsync is the typical way for async value changes (via promises) to enter a reactive context.

martixyOP•8mo ago

That sounds good. Ok, let me try to parse the playground you gave first before trying the createRoot/runWithOwner route.

peerreynders•8mo ago

But I tried <div onKeyDown={kbHandler}> and it won't fire my handler.

A div needs a tabindex otherwise it can't take focus https://playground.solidjs.com/anonymous/05c45974-252a-4b79-bb26-3b3927493e57

Solid Playground

Quickly discover what the solid compiler will generate from your JSX template

martixyOP•8mo ago

Hm, that's good to know. But I need the event to be global.

peerreynders•8mo ago

Minimal from example which installs an event listener on the document body. https://playground.solidjs.com/anonymous/442a1e83-a1fd-4111-9511-f667b5bcacbf

Solid Playground

Quickly discover what the solid compiler will generate from your JSX template

martixyOP•8mo ago

Tnx. Sure to come in handy. I have to figure out a few other things first, but I'll try it.

peerreynders•8mo ago

It doesn't seem to complain about the mundane onMount/onCleanup setup either. https://playground.solidjs.com/anonymous/594e1b36-0737-4e02-b9ee-8ba8626d019b

Solid Playground

Quickly discover what the solid compiler will generate from your JSX template

martixyOP•8mo ago

Maybe it's in production mode?

peerreynders•8mo ago

I was under the impression that your listener had a createEffect() inside it. My assumption was that was causing the error. The goal is to have the listener "tell" the reactive context, rather than the listener trying to "do" any work. Tell, don't Ask

martixyOP•8mo ago

I mean it causes a warning, but the playground does not reproduce it.

Brendonovich•8mo ago

what it comes down to is that callbacks don't have an owner. if you want to get rid of the warning and dispose the effect properly you need to create an owner with createRoot and dispose of it properly, or reuse the owner from the component with getOwner and runWithOwner so the effect stops when the component unmounts

peerreynders•8mo ago

Think about it. If there was no warning (and it would work) then you would be creating an effect every time the listener runs. So each keydown yet another effect function is hanging around subscribed to the signals it queried, all of them ready to run the moment those signals change. What you really want is createEffect inside the app (once) accessing a signal that is updated by the listener.

martixyOP•8mo ago

Good point.

martixyOP•8mo ago

It took me a while but I think I'm starting to get it. I get no warnings now, and my code works as expected. However I would appreciate advice on how the code is structured. https://playground.solidjs.com/anonymous/ad7bb9a9-ccd8-496a-a680-15c32d5d9330 Mostly, is there anything wrong with how I create the signals and how I pass them around? And if there is, what is a better way to do this? (To make it easier to parse - the call chain is kbHandler -> collectDownloads -> startDownloads -> queueDownload)

Solid Playground

Quickly discover what the solid compiler will generate from your JSX template

peerreynders•8mo ago

Is this pure CSR or is SSR involved?

martixyOP•8mo ago

No. I mean CSR

peerreynders•8mo ago

I'm just asking because module globals that are used during SSR are shared across concurrent requests on the server. If you decided to go SSR and that download signal was initialized to support the SSR render you would have to put in the effort to isolate it. Any reason you are using Index rather than For?

martixyOP•8mo ago

Uh.... based on first reading of the docs, that seemed the most appropriate one.

peerreynders•8mo ago

For manipulates then entire DOM fragment that relates to the item which is what you typically want; rule of thumb is if the item is an object you want For. Index relates to the text nodes inside of the DOM, so it tends to leave the DOM in place but deals with everything on the text node level; rule of thumb is if the item is a primitive you want Index.

martixyOP•8mo ago

Oh, thank you! That's a nice rule 🙂

peerreynders•8mo ago

For a better editor experience you'll probably want to replace constructs like:

ts 
ev.target.tagName.toLowerCase() === "input"

ts 
ev.target.tagName.toLowerCase() === "input"

with

ev.target instanceof HTMLInputElement

ev.target instanceof HTMLInputElement

martixyOP•8mo ago

Oh, yea. That was some old code copied over. TBH I was expecting remarks around passing around the same object and mutating it along the way... If I didn't need to tunnel the signals from the bottom to the top it would have been much easier I think...

peerreynders•8mo ago

You don't need a map on Array.from

Array.from(images).map((el) => el.href)

Array.from(images).map((el) => el.href)

This should work

Array.from(image, (el) => el.href);

Array.from(image, (el) => el.href);

passing around the same object

which one; I'm still trying to orient myself

martixyOP•8mo ago

the downloads signal, which is at the top contains the other signals which are created at the bottom of the stack.

  let [progress, setProgress] = createSignal({ progressFraction: 0 });
  options.progress = progress;

  let [progress, setProgress] = createSignal({ progressFraction: 0 });
  options.progress = progress;

I put it as a property on the object so it can go in the template at the top. 🙂

peerreynders•8mo ago

Just noticed that. It's probably fine; personally I like to keep the reactive graph as static as possible, so given this situation I would choose a single store over an array of objects where the name and progress value is stored.

martixyOP•8mo ago

Hm, well, it does make sense, this component is effectively the root of the "application".

Solid is a declarative reactive Javascript library for creating user interfaces.

12KMembers

View on Discord

Want results from more Discord servers?

Add your server