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

useProgress - Warning: Cannot update a component (Loader) while rendering a different component #314

Open
nestorchura2019 opened this issue Feb 20, 2021 · 20 comments
Labels
bug Something isn't working

Comments

@nestorchura2019
Copy link

nestorchura2019 commented Feb 20, 2021

Im loading a gltb model and a texture(lightMap),

when using the useProgress I get the error, if I don't load the texture (const texture = useTexture("balightmap2.jpg")), the error is not displayed, I think the error comes out because I'm loading two things, the glb and the texture

import React, { Suspense, useRef, useState, useEffect } from "react"
import { Canvas, useFrame } from "react-three-fiber"
import { Html, useGLTF, OrbitControls, useProgress, useTexture } from "@react-three/drei"

//3d model
function Office(props) {
  const groupHouse = useRef()
  //loafing lightMap texture
  const texture = useTexture("balightmap2.jpg")
  texture.flipY = false
  const { nodes, materials } = useGLTF("/oficina2.glb")

  return (
    <group ref={groupHouse} {...props} dispose={null}>
      <mesh material={materials.oficina} material-lightMap={texture} geometry={nodes.oficina.geometry} />
      <mesh material={materials.piso_Mat} material-lightMap={texture} geometry={nodes.piso.geometry} />
    </group>
  )
}

function Loader() {
  const { active, progress, errors, item, loaded, total } = useProgress()
  return <Html center>{progress} % loaded</Html>
}

export default function App() {
  return (
    <>
      <Canvas concurrent pixelRatio={[1, 2]} camera={{ position: [0, 3, 2.75] }}>
        <Suspense fallback={<Loader />}>
          <Office />
        </Suspense>
        <OrbitControls /* minPolarAngle={Math.PI / 2} maxPolarAngle={Math.PI / 2} */ enableZoom={true} enablePan={true} />
      </Canvas>
    </>
  )
}

Codesandbox - https://codesandbox.io/s/z8zjw?file=/src/App.js

image

@nestorchura2019 nestorchura2019 added the bug Something isn't working label Feb 20, 2021
@joshuaellis joshuaellis added needs more info Needs more information before we can action something and removed needs more info Needs more information before we can action something labels Feb 20, 2021
@joshuaellis
Copy link
Member

Hi @nestorchura2019,

Thanks for this, I believe this is an issue with some changes made in #304. @RenaudRohlinger, would you mind looking into this? Or shall I just revert that commit?

@nestorchura2019
Copy link
Author

Hi @nestorchura2019,

Thanks for this, I believe this is an issue with some changes made in #304. @RenaudRohlinger, would you mind looking into this? Or shall I just revert that commit?

thanks

@RenaudRohlinger
Copy link
Member

@joshuaellis That's a known issue that has always been here. My last PR couldn't affect that part.
For example with v3.3.0 :
https://codesandbox.io/s/oficce-with-loading-ui-forked-1nyws?file=/src/App.js
It's just an issue about some data loaded asynchronously mixed with a react setter.

I don't really know how to fix that, maybe refactor the whole hook?

@joshuaellis
Copy link
Member

This isn't the case with v3.8.0 – https://codesandbox.io/s/oficce-with-loading-ui-forked-sjgs4?file=/src/App.js the example you've sent over uses a version of three that is not compatible with the version of drei.

@RenaudRohlinger
Copy link
Member

RenaudRohlinger commented Feb 22, 2021

@joshuaellis We did not had this error between 3.4.0 and 3.8.0 because I tried to fix it by using a separate instance of the loader for async multi loading which was wrong and broke the loader (between these 2 versions the loader was stuck at 0%). So I would not recommend reverting any commit. You can try any version before 3.4.0 and you will see the same error.

Ref introduced in 3.4.0: #289

The only solution would be to refactor the loader. There is a setter in the DefaultManager onStart event (https://github.com/pmndrs/drei/blob/master/src/core/useProgress.tsx#L16) but if multiple elements are loaded at the same time it will throw "Cannot update a component while rendering a different component" since they are conflicting with each other because they all use the same loading manager.

As I said in my last PR the only clean fix would be to work on the loading manager of threejs. Using the defaultManager of threejs is not the best solution and my last PR is just an improvement of something unstable from the beginning that allows to handle a new instance of loading

@nestorchura2019
Copy link
Author

This isn't the case with v3.8.0 – https://codesandbox.io/s/oficce-with-loading-ui-forked-sjgs4?file=/src/App.js the example you've sent over uses a version of three that is not compatible with the version of drei.

What version of three.js is the recommended or compatible?

@RenaudRohlinger
Copy link
Member

@nestorchura2019 in the meantime you could check the implementation with the official Loader component of Drei: https://codesandbox.io/s/oficce-with-loading-ui-forked-y6m2y?file=/src/App.js

It won't throw any error.

Maybe something to dig here @joshuaellis ?

@joshuaellis joshuaellis added the wontfix This will not be worked on label Mar 9, 2021
@joshuaellis
Copy link
Member

I'm closing this due to inactivity, it seems like a really niche issue & something weird react related, I personally have never had the issue. @nestorchura2019 if you want to look into fixing it you're more than welcome to and we can reopen the issue.

@r-oland
Copy link

r-oland commented Sep 25, 2021

Hi @joshuaellis! I'm running into the same problem right now and I think that I might have to disagree with you on it being a niche issue. Personally, I find myself wanting to use multiple loaders within a single component quite often. It's especially useful if you want to dispatch all the loaders within a scene in one context providing component for reusability purposes.

The solution above from @RenaudRohlinger with the drei component didn't work for me btw. It's probably a separate issue, but the completed state triggered too early with the useCubeTexture loader.

I'm relatively green when it comes to r3f so please do tell me if I'm missing something obvious here. ✌🏻

@steffendolan
Copy link

Im having the same problem. Any updates on this?

@lukehorvat
Copy link
Contributor

I encountered this error recently as soon as I added a useProgress to my app. My app needs to request a lot of data, with 2-3 useLoader calls per component.

I figured out a solution that solved the issue for me though. Delay every loader.load() until the next tick of the event loop with a withNextTickLoading abstraction:

export function withNextTickLoading<T, L extends LoaderProto<T>>(Proto: L): L {
  return class NextTickLoader extends Proto {
    load(
      ...args: Parameters<Loader<T extends unknown ? any : T>['load']>
    ): void {
      setTimeout(() => super.load(...args), 0);
    }
  };
}

interface Loader<T> extends THREE.Loader {
  load(
    url: string,
    onLoad?: (result: T) => void,
    onProgress?: (event: ProgressEvent) => void,
    onError?: (event: ErrorEvent) => void
  ): unknown;
}

type LoaderProto<T> = new (...args: any) => Loader<T extends unknown ? any : T>;

Which can be used like so:

- const data = useLoader(THREE.FileLoader, url);
+ const data = useLoader(withNextTickLoading(THREE.FileLoader), url);

I applied it to @nestorchura2019's code and it seems to fix it too: https://codesandbox.io/s/oficce-with-loading-ui-fixed-zcm2fh

Just thought I'd share in case this brings us closer to identifying what the proper fix for useProgress should be.

@arcasoy
Copy link

arcasoy commented Jan 29, 2024

I'm also running into this issue. I have a scene set up with a <GLTFSceneLoader /> component that uses useGLTF which calls a model from either an API endpoint or from a local file system. The scene is them passed to a <primitive /> object to render. Importantly, this component implements useProgress() to perform business logic after the model has loaded in.

As a sibling to the scene loader, there is a <EnvironmentContainer /> component that encapsulates the <Environment /> component, passing in a .hdr file into the files attribute.

So overall:

<Canvas>
<GLTFSceneLoader /> // contains useProgress & useGltf
<EnvironmentContainer /> // contains <Environment /> (which wraps useEnvironment)
</Canvas>

Oddly when the scene loader is calling a URL through useGLTF(), the error does not appear, but when calling a local file (i.e. './someFileName.glb'), the error described in OP's post appears. This makes me inclined to say that it's a timing/order of rendering issue, especially with @lukehorvat's solution that pushes the loader to the next tick.

Unfortunately, in my case the hooks used are built around useLoader() and I would prefer to not modify the package. I'm wondering if something like the solution could be implemented into useLoader() so logic dependent on it like useGltf() and useEnvironment() are fixed?

@arcasoy
Copy link

arcasoy commented Jan 30, 2024

@joshuaellis We did not had this error between 3.4.0 and 3.8.0 because I tried to fix it by using a separate instance of the loader for async multi loading which was wrong and broke the loader (between these 2 versions the loader was stuck at 0%). So I would not recommend reverting any commit. You can try any version before 3.4.0 and you will see the same error.

Ref introduced in 3.4.0: #289

The only solution would be to refactor the loader. There is a setter in the DefaultManager onStart event (https://github.com/pmndrs/drei/blob/master/src/core/useProgress.tsx#L16) but if multiple elements are loaded at the same time it will throw "Cannot update a component while rendering a different component" since they are conflicting with each other because they all use the same loading manager.

As I said in my last PR the only clean fix would be to work on the loading manager of threejs. Using the defaultManager of threejs is not the best solution and my last PR is just an improvement of something unstable from the beginning that allows to handle a new instance of loading

I see this PR in the R3F repository that enables loader instances within useLoader(). Could this potentially resolve the issue, or does an instantiation of DefaultLoadingManager need to be added to resolve this? CC: @RenaudRohlinger @joshuaellis

@CodyJasonBennett CodyJasonBennett removed the wontfix This will not be worked on label Apr 26, 2024
@CodyJasonBennett
Copy link
Member

That React warning is a real bug, so I'm elevating this issue.

@mikeIsobarPL
Copy link

I confirm, useProgress crashes whole app on start, what is teh status of this bug ?

@pierroo
Copy link

pierroo commented Jul 22, 2024

if that matters, useTexture is also raising the same issue in latest version 9.108.3 (in dev mode only, the red warning leaves in production but that remains scary)

@lezan
Copy link

lezan commented Aug 16, 2024

I confirm the issue also with useTexture().

@ghost
Copy link

ghost commented Aug 19, 2024

I also encountered the same problem when I used useGltf and useAnimations in a component and wrapped Suspense outside of it. When I tried to use useProgress to read the loading status, it would trigger this bug at a certain loading moment. But it seems to be caused by react. By the way, It is my version info: "@react-three/drei": "^9.109.5","@react-three/fiber": "^8.17.5","three": "^0.167.1","react": "^18.3.1", "react-dom": "^18.3.1",

@zrooda
Copy link

zrooda commented Sep 21, 2024

Also having this issue with multiple loaders in a component, rather easy to achieve with common usecases.

@lukehorvat's timeout is a clever hack but IMO going with a loader design that animates indeterminate progress avoiding useProgress() for now is preferred to somehow breaking React's render mechanism.

@richi-coder
Copy link

I've experienced the same, but in my case it must be because I am rendering a component (which is loading a texture) inside an imported model, so it throws the same error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests