`scene` from `useThree` is not always a `THREE.Scene` within portal

[mattrossman]

Sandbox: https://codesandbox.io/s/issue-createportal-and-state-scene-pvtlsz?file=/src/App.jsx

Output:

According to the docs and type definitions, scene returned from useThree should be a THREE.Scene representing the scene. However, when this value is read within a component that has been dynamically parented to another object with createPortal, the scene value contains the parent object which could be a different type such as THREE.Group.

I would expect scene to always contain a scene so that components can modify scene-specific properties such as .background. I would only expect the scene value to change if the createPortal target was a different THREE.Scene.

[mattrossman]

I think we should add a check here if container is an instanceof THREE.Scene, otherwise forward the existing state.scene. This would also remove the need for the as THREE.Scene type assertion.

https://github.com/pmndrs/react-three-fiber/blob/af77139ef65803b4e2d907477de01ed0a21671a5/packages/fiber/src/core/index.tsx#L471-L475

[drcmda]

a portal can be any object3d technically, and i think it's just a typing or naming issue. useThree(state => state.scene) should point to the root and maybe "root" would have been a better name from the get go but portals are a fairly advanced concept that didn't exist way back when.

I think we should add a check here if container is an instanceof THREE.Scene, otherwise forward the existing state.scene.

it's pretty important to get the root object inside a portal, for after all it's supposed to be a self contained sandbox. the underlying premise still holds, you get to root object which you can use to render out or do something else with it. if that points to something that is not the portals root that imo would be a bug.

[mattrossman]

I can appreciate the value in providing access to the root object.

However, some components rely on scene-specific behavior, so access to the root Object3D isn't always sufficient. The useThree hook already exposes the default THREE.Camera and THREE.WebGLRenderer managed by R3F so that users can write components that access these objects regardless of where they are mounted, it seems reasonable to expose a stable THREE.Scene property as well.

For example, the <Environment /> component from drei uses state.scene to change the background and envMap properties. I have also written a similar <BackgroundColor /> component in the past relying on state.scene.background which does not exist on the broader Object3D type.

Another use-case (the one that inspired this issue) is that I have a dynamically parented object which contains a THREE.SkeletonHelper. SkeletonHelpers are supposed to appear directly under the scene as they are affected by parent transforms, so I expect to be able to say createPortal(<skeletonHelper ... />, state.scene). Instead I have manually pass the outer state.scene down to my component or traverse up the parent chain to find the nearest scene.

Perhaps it is worth exposing both the root and scene which may or may not be the same reference.

[mattrossman]

Here is a utility I am using as a workaround:

/**
 * Get the nearest THREE.Scene ancestor, or null
 */
function useScene() {
  const { scene: root } = useThree()

  const scene = useMemo(() => {
    let object: THREE.Object3D | null = root
    while (object) {
      if (object instanceof THREE.Scene) return object
      object = object.parent
    }
    return null
  }, [root])

  return scene
}

[CodyJasonBennett]

There's two things that createPortal does that can make this tricky:

• it clones the root state for the portal and modifies its scene property with an Object3D
• the reconciler uses the scene property for top-level nodes of a root or portal to inform where to attach nodes

If we separate this behavior, it allows for two things:

• immediately, you can do createPortal(<element />, Object3D, { scene: Scene }) for the desired behavior (<element /> attaches to Object3D, Scene parents Object3D, although the default behavior is still the same
• we can apply the aforementioned internal changes by always creating and appending to a Scene which shouldn't change the perceived rendering behavior. This will need to consider cases where the target is either mounted or unmounted to an existing scene (it's valid behavior to have a Scene within a Scene)

The second can be breaking for those relying on the fact that scene would be a specific Object3D, even if less correct, so I'd want to relegate that to v9.

[drcmda]

btw @CodyJasonBennett i think that's actually possible right now, the third argument should do exactly what you laid out, it's for custom inject objects.

@mattrossman you can also access scene.__r3f and in there you will find the root and the previous root. btw if portal is the issue it could just use a scene instead of a group, maybe that's the easiest fix.

[CodyJasonBennett]

WRT https://github.com/pmndrs/react-three-fiber/issues/2725#issuecomment-1398597982, I believe this can be addressed by changing that to scene: container instanceof THREE.Scene ? container : new THREE.Scene().add(container) for the state model, and keeping the actual React portal target container.