Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

createScriptLoader triggers a warning about disposed computations #746

Open
sfnmk opened this issue Feb 3, 2025 · 4 comments
Open

createScriptLoader triggers a warning about disposed computations #746

sfnmk opened this issue Feb 3, 2025 · 4 comments

Comments

@sfnmk
Copy link

sfnmk commented Feb 3, 2025

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

@atk
Copy link
Member

atk commented Feb 3, 2025

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.

@Tyg-g
Copy link

Tyg-g commented Feb 4, 2025

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.)

@atk
Copy link
Member

atk commented Feb 5, 2025

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

@Tyg-g
Copy link

Tyg-g commented Feb 6, 2025

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants