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

Add low-level suspense primitive #1877

Open
thetarnav opened this issue Sep 9, 2023 · 6 comments
Open

Add low-level suspense primitive #1877

thetarnav opened this issue Sep 9, 2023 · 6 comments
Labels
enhancement New feature or request
Milestone

Comments

@thetarnav
Copy link
Contributor

thetarnav commented Sep 9, 2023

A low-level suspense primitive would be useful for libraries to pause effects in "offscreen" branches.

Currently Suspense is not only a component-only primitive, but it is tied to resources, and by extend to routing, SSR, transitions, etc. It also assumes needing a fallback branch, which is not always needed.
Because of that it cannot be freely used in libraries, without affecting the rest of the app as a side effect.

For example in transition libraries like motionone and solid-transition-group, when we use <Transition> with mode="out-in", we need to wait for the previous element finish his exit animation, before the newly rendered element can be added to the DOM. But solid doesn't know that the new element is only kept in memory, and not yet appeared on the page, so the updates queue will proceed normally, calling all onMount callbacks, where we expect to deal with elements connected to the DOM.
An issue with more details: solidjs-community/solid-transition-group#34

Also if we wish to keep some roots in memory—e.g. to avoid recreating the same elements when filtering a large array, or displaying search highlights, or implementing a root pool primitive—there is no way to simply prevent then from running some side effects.

If we tried to use <Suspense> to suspend those branches, any resource read under it will also trigger it, possibly breaking the intended behavior of the app—by not showing a fallback in an expected place, or causing a transition (transaction) to be exited sooner (<Transition> is commonly used for wrapping rendered routes).

Code

example createSuspense using Suspense:
https://playground.solidjs.com/anonymous/21cef751-8f37-4354-8a63-0aa475d54e64

function createSuspense<T>(when: Accessor<boolean>, fn: () => T): T {
  let value: T,
    resolve = noop;

  const [resource] = createResource(
    () => when() || resolve(),
    () => new Promise<void>((r) => (resolve = r)),
  );

  Suspense({
    // @ts-expect-error children don't have to return anything
    get children() {
      createMemo(resource);
      value = fn();
    },
  });

  return value!;
}

In @fabiospampinato's oby this is solved by having a low-level suspense primitive that simply takes a function to suspend and a boolean signal to inform if the branch should be suspended or not, and returns the value directly, similar to createRoot.
When the condition signal is true, all effects will be suspended, while resources ignore it and keep the lookup for Suspense they can trigger.

Code

Something like this could maybe be implemented currently as this: (although I'm not sure if resources won't trigger it anyway)

function suspense<T>(when: Accessor<boolean>, fn: () => T): T {
  const SuspenseContext = getSuspenseContext(),
    store = {
      effects: [] as Computation<unknown>[],
      resolved: false,
      inFallback: when
    };

  let result!: T;

  SuspenseContext.Provider({
    value: store,
    get children() {
      result = fn();

      createMemo(() => {
        if (!when()) {
          store.resolved = true;
          resumeEffects(store.effects);
        }

        return result;
      });

      return undefined;
    }
  }) as any;

  return result;
}
@fabiospampinato
Copy link

fabiospampinato commented Sep 9, 2023

Definitely worth adding, imo.

There's another related primitive that looks like it's kinda necessary, suspended, which basically tells you when entering and exiting suspense. That's useful because under an active suspense boundary side effects should be paused, but side effects and "createEffect" instances are not the same thing, "createEffect" merely wraps a side effect, it's not one.

For example in the code below the actual side effect is the "fn" function, suspending the "createEffect" will do nothing to the actual side effect, this requires some manual pausing/resuming of the interval.

createEffect ( () => {
  const intervalId = createInterval ( fn, 100 );
  onCleanup ( () => {
    clearInterval ( intervalId );
  });
});

@ryansolid
Copy link
Member

Yeah Ive written about this a bit here: https://hackmd.io/NDrB5kP2QjGUrwYo2DwQ0w

I don't think Suspense is the lowest primitive here. It has very specific behaviors. Behaviors I don't think should change really, but it isn't the only problem of its kind.

That being said transition group issue sounds like I made a poor implementation. Likely the only one available to me at the time. I do like that bounded flushing also seems like the solution here.

Ultimately I think we solve this, but as something independent of Suspense. More fundamental.

@katywings
Copy link
Contributor

I think this issue might be related, since its also about the timing of Suspense vs transition-group:
solidjs-community/solid-transition-group#38

@ryansolid ryansolid added the enhancement New feature or request label Sep 11, 2023
@ryansolid ryansolid added this to the 2.0.0 milestone Sep 11, 2023
@fabiospampinato
Copy link

There seems to be another interesting component that this would unlock: one that doesn't render a fallback branch at all, it just suspends children, which makes the unsuspension cheaper, potentially by a lot.

@Tronikelis
Copy link

I am porting react's swr library to solid solid-swr and without createResource I can't add suspense support to my hooks.
Just want to mention that this would help me add suspense support to solid-swr 👍

If anybody is interested https://github.com/Tronikelis/solid-swr

@Tronikelis
Copy link

This is how I am currently abusing the createResource hook to enable suspense for my library, it works but feels like such a hack though:

/**
 * monkey-patches the `createResource` solid hook to work with `useSWR`
 */
const useSWRSuspense: typeof useSWR = (key, options) => {
    const swr = useSWR(key, options);

    let resolveResource: undefined | ((val?: never) => void);

    const [resource] = createResource(() => {
        return new Promise<undefined>(r => {
            resolveResource = r;
        });
    });

    // not in an createEffect, because suspense disables them
    (async () => {
        await swr._effect(); // this never throws
        resolveResource?.();
    })().catch(noop);

    const dataWithResource = () => {
        resource();
        return swr.data();
    };

    return {
        ...swr,
        data: dataWithResource,
    };
};

export default useSWRSuspense;

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

No branches or pull requests

5 participants