solid icon indicating copy to clipboard operation
solid copied to clipboard

External source will rerun disposed inTransition computation function if there is a suspense context

Open rprend opened this issue 1 year ago • 1 comments

Describe the bug

We setup a simplified repro here: https://playground.solidjs.com/anonymous/c8c3f9c0-505f-43d2-a4ec-16bb8dbc9ed7. Setup solid with an external source. A memo returns a signal value. We call an inTransition trigger to simulate an external dependency being invalidated while we are in a transition. This will dispose the inTransition computation. After this, we start another transition, and update the signal which invalidates the memo. Since we are in a transition, we attempt to track the disposed autorun, throwing an error.

For this to repro, there needs to be a suspense context. startTransition() checks for the existence of a suspense before marking the transition as running.

tagging @milomg who wrote the repro case.

Your Example Website or App

https://playground.solidjs.com/anonymous/c8c3f9c0-505f-43d2-a4ec-16bb8dbc9ed7

Steps to Reproduce the Bug or Issue

Go to https://playground.solidjs.com/anonymous/c8c3f9c0-505f-43d2-a4ec-16bb8dbc9ed7 Run the playground example Notice that we throw an error when we rerun a disposed transition

Expected behavior

I would expect one of two solutions. First, make inTransition not autodispose. Currently it disposes immediately after the transition promise resolves: const triggerInTransition = () => startTransition(trigger).then(() => inTransition.dispose()). Second, if it does autodispose, we call the external source factory with the transition trigger to replace the disposed one.

We work around this on our end by ignoring the dispose() calls for inTransition computations, but this is not ideal because it leaks memory.

Screenshots or Videos

No response

Platform

  • OS: macOS
  • Browser: Chrome Version 127.0.6533.120
  • Version: [e.g. 91.1]

Additional context

No response

rprend avatar Sep 06 '24 19:09 rprend

I believe something like this should help (although ideally we'd also dispose when transitions that are created elsewhere are merged)

  if (ExternalSourceConfig && c.fn) {
    const [track, trigger] = createSignal<void>(undefined, { equals: false });
    const ordinary = ExternalSourceConfig.factory(c.fn, trigger);
    onCleanup(() => ordinary.dispose());
    const triggerInTransition: () => void = () =>
      startTransition(trigger).then(() => {
        inTransition.dispose();
        inTransition = undefined
      });
    let inTransition = undefined;
    c.fn = x => {
      track();
      return Transition && Transition.running ? (inTransition || inTransition = ExternalSourceConfig.factory(c.fn, triggerInTransition)).track(x) : ordinary.track(x);
    };
  }

milomg avatar Sep 06 '24 21:09 milomg