From 614112b39021c44d43b92ea3e989dd52b5b661aa Mon Sep 17 00:00:00 2001 From: Cody Bennett <23324155+CodyJasonBennett@users.noreply.github.com> Date: Fri, 1 Sep 2023 04:45:07 -0500 Subject: [PATCH 1/4] fix(useLoader): memoize loaders, dispose --- packages/fiber/src/core/hooks.tsx | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/fiber/src/core/hooks.tsx b/packages/fiber/src/core/hooks.tsx index 42cd9449dc..8317e09fb4 100644 --- a/packages/fiber/src/core/hooks.tsx +++ b/packages/fiber/src/core/hooks.tsx @@ -79,13 +79,20 @@ export function useGraph(object: THREE.Object3D) { return React.useMemo(() => buildGraph(object), [object]) } +const memoizedLoaders = new WeakMap, Loader>() + function loadingFn>( extensions?: Extensions, onProgress?: (event: ProgressEvent) => void, ) { return function (Proto: L, ...input: string[]) { // Construct new loader and run extensions - const loader = new Proto() + let loader = memoizedLoaders.get(Proto)! + if (!loader) { + loader = new Proto() + memoizedLoaders.set(Proto, loader) + } + if (extensions) extensions(loader) // Go through the urls and load them return Promise.all( @@ -103,7 +110,7 @@ function loadingFn>( ), ), ), - ) + ).finally(() => (loader as any).dispose?.()) } } From 7fd16e68be7846bda86f749be5bc64f6b26b4e03 Mon Sep 17 00:00:00 2001 From: Cody Bennett <23324155+CodyJasonBennett@users.noreply.github.com> Date: Sun, 3 Sep 2023 16:58:56 -0500 Subject: [PATCH 2/4] chore: update mock --- packages/fiber/tests/core/hooks.test.tsx | 32 +++++++++++------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/packages/fiber/tests/core/hooks.test.tsx b/packages/fiber/tests/core/hooks.test.tsx index 0474bc9954..88e289c4d4 100644 --- a/packages/fiber/tests/core/hooks.test.tsx +++ b/packages/fiber/tests/core/hooks.test.tsx @@ -138,25 +138,21 @@ describe('hooks', () => { mesh2.name = 'Mesh 2' MockGroup.add(mesh1, mesh2) - jest.spyOn(Stdlib, 'GLTFLoader').mockImplementation( - () => - ({ - load: jest - .fn() - .mockImplementationOnce((_url, onLoad) => { - onLoad(MockMesh) - }) - .mockImplementationOnce((_url, onLoad) => { - onLoad({ scene: MockGroup }) - }), - setPath: () => {}, - } as unknown as Stdlib.GLTFLoader), - ) + class TestLoader extends THREE.Loader { + load = jest + .fn() + .mockImplementationOnce((_url, onLoad) => { + onLoad(MockMesh) + }) + .mockImplementationOnce((_url, onLoad) => { + onLoad(MockGroup) + }) + } + + const extensions = jest.fn() const Component = () => { - const [mockMesh, mockScene] = useLoader(Stdlib.GLTFLoader, ['/suzanne.glb', '/myModels.glb'], (loader) => { - loader.setPath('/public/models') - }) + const [mockMesh, mockScene] = useLoader(TestLoader, ['/suzanne.glb', '/myModels.glb'], extensions) return ( <> @@ -180,6 +176,8 @@ describe('hooks', () => { await waitFor(() => expect(scene.children[0]).toBeDefined()) expect(scene.children[0]).toBe(MockMesh) + expect(scene.children[1]).toBe(MockGroup) + expect(extensions).toBeCalledTimes(1) }) it('can handle useLoader with a loader extension', async () => { From 83ae5d1cde4f8ab93da7c8ac7bb3d1cdac31faa6 Mon Sep 17 00:00:00 2001 From: Cody Bennett <23324155+CodyJasonBennett@users.noreply.github.com> Date: Sun, 3 Sep 2023 18:04:29 -0500 Subject: [PATCH 3/4] fix: don't memoize --- packages/fiber/src/core/hooks.tsx | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/packages/fiber/src/core/hooks.tsx b/packages/fiber/src/core/hooks.tsx index 8317e09fb4..d8d38497e2 100644 --- a/packages/fiber/src/core/hooks.tsx +++ b/packages/fiber/src/core/hooks.tsx @@ -79,20 +79,13 @@ export function useGraph(object: THREE.Object3D) { return React.useMemo(() => buildGraph(object), [object]) } -const memoizedLoaders = new WeakMap, Loader>() - function loadingFn>( extensions?: Extensions, onProgress?: (event: ProgressEvent) => void, ) { return function (Proto: L, ...input: string[]) { // Construct new loader and run extensions - let loader = memoizedLoaders.get(Proto)! - if (!loader) { - loader = new Proto() - memoizedLoaders.set(Proto, loader) - } - + const loader = new Proto() if (extensions) extensions(loader) // Go through the urls and load them return Promise.all( From c388024c78d318be8d6ac153d1c01c9c286c6973 Mon Sep 17 00:00:00 2001 From: Cody Bennett <23324155+CodyJasonBennett@users.noreply.github.com> Date: Sun, 3 Sep 2023 18:34:18 -0500 Subject: [PATCH 4/4] Revert "fix: don't memoize" This reverts commit 83ae5d1cde4f8ab93da7c8ac7bb3d1cdac31faa6. --- packages/fiber/src/core/hooks.tsx | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/fiber/src/core/hooks.tsx b/packages/fiber/src/core/hooks.tsx index d8d38497e2..8317e09fb4 100644 --- a/packages/fiber/src/core/hooks.tsx +++ b/packages/fiber/src/core/hooks.tsx @@ -79,13 +79,20 @@ export function useGraph(object: THREE.Object3D) { return React.useMemo(() => buildGraph(object), [object]) } +const memoizedLoaders = new WeakMap, Loader>() + function loadingFn>( extensions?: Extensions, onProgress?: (event: ProgressEvent) => void, ) { return function (Proto: L, ...input: string[]) { // Construct new loader and run extensions - const loader = new Proto() + let loader = memoizedLoaders.get(Proto)! + if (!loader) { + loader = new Proto() + memoizedLoaders.set(Proto, loader) + } + if (extensions) extensions(loader) // Go through the urls and load them return Promise.all(