Allow passing loader instances to `useLoader`

[mattrossman]

Problem

Consider this sandbox: https://codesandbox.io/p/sandbox/use-loader-extend-loader-issue-4w5dtq?file=%2Fsrc%2FApp.tsx%3A11%2C32

function ModelA() {
  // Just a regular useLoader call without extensions
  const gltf = useLoader(GLTFLoader, URL_A);
  ...
}

function ModelB() {
  const gltf = useLoader(GLTFLoader, URL_B,
    (loader) => {
      // Load materials as red basic material
      loader.register((parser) => ({
        async loadMaterial() {
          return new THREE.MeshBasicMaterial({ color: "red" });
        },
      }));
    }
  );

  ...
}

You might expect that <ModelA> loads normally, while <ModelB> loads with a plugin that turns materials red. However, both models actually appear red. Imagine that <ModelA> and <ModelB> live in separate files, or even separate libraries—it becomes quite difficult to debug why this occurs.

Or consider this code:

const URL = "..."
const PATH_A = "..."
const PATH_B = "..."

useLoader.preload(TextureLoader, URL, (loader) => {
   loader.setPath(PATH_A)
})
  
function MyComponent() {
  const texture = useLoader(TextureLoader, URL, (loader) => {
     loader.setPath(PATH_B)
  });
  ...
}

Do you know what <MyComponent> code will display without seeing the .preload() call? The component logic suggests that it loads a resource from PATH_B, but the .preload() call causes it to load the cached resource from PATH_A. The fact that the API for these two invocations even allow different extensions is confusing.

IMO the global side effects of useLoader extensions are an anti-pattern that breaks from the immutable, unidirectional flow that I expect in React. The userland code doesn't communicates that one component is mutating some global object that affects other components, which makes it hard to understand runtime behavior from reading the code.

Readability aside, this API doesn't allow you to use loaders like you could in vanilla Three, where you might have different loader configurations for different assets of the same type.

Potential Solution

I think it'd make more sense to pass loader instances to useLoader rather than a constructor. This makes the shared nature of loaders more obvious in user code, and allows you to use different loader configurations for different resources.

It also makes the .preload API easier to understand—the current API allows you to pass different configurations across .preload and useLoader invocations even though these are intended to share a loader instance.

This sandbox illustrates a possible implementation: https://codesandbox.io/p/sandbox/use-loader-extend-loader-issue-forked-c7kw3z?file=%2Fsrc%2FApp.tsx%3A8%2C1-16%2C5

const gltfLoaderA = new GLTFLoader();  // maybe need to store these in state?
const gltfLoaderB = new GLTFLoader();

gltfLoaderB.register((parser) => ({
  async loadMaterial() {
    return new THREE.MeshBasicMaterial({ color: "red" });
  },
}));

useLoader.preload(gltfLoaderA, URL_A);

function ModelA() {
  const gltf = useLoader(gltfLoaderA, URL_A);
  ...
}

function ModelB() {
  const gltf = useLoader(gltfLoaderB, URL_B);
  ...
}

Notice how here, even if I accidentally pass a different loader instance to .preload and the useLoader hook, it simply causes a cache miss instead of loading from the wrong path in my component.

const URL = "..."
const PATH_A = "..."
const PATH_B = "..."

const textureLoaderA = new TextureLoader().setPath(PATH_A)
const textureLoaderB = new TextureLoader().setPath(PATH_B)

useLoader.preload(textureLoaderA, URL)
  
function MyComponent() {
  const texture = useLoader(textureLoaderB, URL);
  ...
}

Maybe there's some way to make this compatible with the existing API too.

[CodyJasonBennett]

Does #3131 satisfy this?

[mattrossman]

@CodyJasonBennett That looks almost exactly like what I want!

The only difference I see is the caching behavior. For example, in this case:

useLoader.preload(gltfLoader, [URL_A, URL_B])

function ComponentA() {
  const gltf = useLoader(gltfLoader, URL_A)
}

In the current code, the suspend function sees these as different cache keys ([loader, URL_A] instead of [loader, URL_A, URL_B]) and would re-run the Loader.load function instead of returning the preloaded object.

This sandbox shows one way to address this, memoizing load results for each URL per-loader instead of just memoizing loader instances.

Something like like:

                    // memoizedResults[<loader>][<url>] = <result> | undefined
const memoizedResults = new WeakMap<Loader<unknown>, Map<string, unknown>>()

That way even though suspend re-runs, it skips loading a new object.

[CodyJasonBennett]

I'll be implementing your suggestion in our v9 branch. Need to wrangle with types for v8, but should be possible to backport.

[CodyJasonBennett]

Unfortunately, I haven't been able to backport this since TypeScript struggles to infer types correctly without upgrading three types (breaking change). This is in our v9 release candidate, which we're soon looking to release to stable alongside Drei.

[CodyJasonBennett]

This is now implemented with v9.