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

Abort controller support #3005

Open
alessiocancian opened this issue Sep 13, 2023 · 4 comments
Open

Abort controller support #3005

alessiocancian opened this issue Sep 13, 2023 · 4 comments

Comments

@alessiocancian
Copy link

Currently there's no way to abort a pending request using useLoader or similar hooks.
It's a big problem in use cases where large models are downloaded using slow connections, and currently the requests keep loading in background even when the component using useLoader gets unmounted.

Sadly it seems that two dependencies needs to add abort controller support before being able to use it on r3f...
three.js: mrdoob/three.js#23070
suspend-react: pmndrs/suspend-react#10

Currently because of the suspend call, a component using useLoader can't even detect the unmount with the useEffect hook, since the component doesn't get mounted during the loading.
This example will make it clearer:

function Example() {
  const model = useLoader(...);
  
  useEffect(() => {
    console.log("Mount"); // gets called only after loading ended
    return () => {
      console.log("Unmount"); // gets called only if the component gets unmounted after loading
    };
  }, []);

  return <></>;
}

It is probably an intended behavior but may be a problem when implementing abort controller support.

I hope that opening an issue here helps catching some more attention to this problem and to find the nicest and smoothest API on both dependencies.

@alessiocancian alessiocancian changed the title Abort controller Abort controller support Sep 13, 2023
@CodyJasonBennett
Copy link
Member

I'm not sure I understand why a code change is needed in suspend-react since that only resolves promises via React Suspense. For three.js loaders, that is indeed not actionable until that PR gets through. I see you've already updated there, since it was lost.

@alessiocancian
Copy link
Author

alessiocancian commented Sep 13, 2023

I'm not sure I understand why a code change is needed in suspend-react since that only resolves promises via React Suspense.

Maybe it isn't required to support an abortcontroller but could be useful to detect the unmount of the component to abort the request if the component gets dismissed during the loading screen, since it's hard to do it inside a Suspense (for the issue about useEffect I mentioned).

It is probably doable by wrapping the useLoader component with <Suspense> and passing an abort signal from there but that leaves the problem to the developer, imho it is a good idea to automatically stop the request on unmount by default.

@CodyJasonBennett
Copy link
Member

A suspended component is never mounted -- suspense interrupts rendering to the nearest suspense boundary. If you're trying to address memory leaks from three.js loaders, upstream support is the only way around that. You could maybe fork or patch three.js FileLoader to add in this behavior. Another API not entertained there would have this stem from the loading manager.

@alessiocancian
Copy link
Author

A suspended component is never mounted -- suspense interrupts rendering to the nearest suspense boundary.

Yes that's the problem, I think that the issue in suspend-react was asking for a way to automatically abort the loading on unmount from the same component or hook using the suspend function, but as I also mentioned inside that issue it doesn't seem to be possible.

But I think that by wrapping the loader with <Suspence> it may be possible, should be something like:

function LoadData({ abortController }) {
  const data = useLoader(..., abortController);
  return (...);
}

function LoadWrapper() {
  const abortController = new AbortController(); //probably needs to be useMemo-ed

  useEffect(() => {
    return () => {
      abortController.abort();
    };
  }, []);

  return (
    <Suspense>
      <LoadData  abortController={abortController} />
    </Suspense>
  )
}

Obviously this can't be included inside a library but only in the end implementation.

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

2 participants