solid-primitives icon indicating copy to clipboard operation
solid-primitives copied to clipboard

createScriptLoader triggers a warning about disposed computations

Open sfnmk opened this issue 10 months ago • 4 comments

Describe the bug

When using createScriptLoader a warning is shown that reads:

computations created outside a createRoot or render will never be disposed

should I wrap the createScriptLoader around some other function? Currently I'm using it like this:

const MainLayout = (props: RouteSectionProps) => {
  createScriptLoader({
    src: '<insert-script-url>',
    crossOrigin: 'anonymous',
  })

  return (
    <MetaProvider>
      <Navbar />
      <div class="flex h-screen-nav">
        <SideNav />
        <main class="flex-grow h-full p-4 overflow-hidden">
          <Suspense>{props.children}</Suspense>
        </main>
      </div>
    </MetaProvider>
  )
}

export default MainLayout

Minimal Reproduction Link

N/A

sfnmk avatar Feb 03 '25 04:02 sfnmk

This is a known issue. If we want to use the internal spread (which minimies code and development effort), we can't do anything about it.

atk avatar Feb 03 '25 09:02 atk

I just ran across this issue. It is caused by the spread function being called in a timeout callback. A callback function is run out of the current context, so Solid cannot tie the computation to a concrete JSX component. A quick check reveals that spread calls createRenderEffect 2 to 3 times - so this is causing the issue.

Solution

After a quick glimpse I don't understand the logic behind calling spread, but assuming it is right, I believe the issue can be solved by using getOwner() and runWithOwner() in this manner. This would assign the right context for Solid.

import { getOwner, runWithOwner } from "solid-js";

...

export function createScriptLoader(props: ScriptProps): HTMLScriptElement | undefined {
    const currentOwner = getOwner();
    
    ...

    setTimeout(() => runWithOwner(currentOwner, () => spread(script, scriptProps, false, true)));

    ...
}

(I guess I'm missing some TS definitions, I'm not a TS guy.)

Tyg-g avatar Feb 04 '25 15:02 Tyg-g

Unfortunately, running with the owner creates the exact error in hydration that we aim to solve with using the timeout.

atk avatar Feb 05 '25 04:02 atk

Ok, I see. So it's an architectural problem. I hope it'll get resolved. If I understand correctly, in its current form if createScriptLoader is called in a component, that is mounted+unmounted more than once, it creates a small memory leak.

Tyg-g avatar Feb 06 '25 09:02 Tyg-g