From e7a10252f23432298572ac7377e97a77d2fccbd2 Mon Sep 17 00:00:00 2001 From: Cody Bennett <23324155+CodyJasonBennett@users.noreply.github.com> Date: Thu, 21 Jul 2022 10:42:18 -0500 Subject: [PATCH 01/28] experiment: separate instance & object, create objects on commit --- packages/fiber/src/core/events.ts | 12 +- packages/fiber/src/core/index.tsx | 17 +- packages/fiber/src/core/renderer.ts | 434 +++++++++++----------------- packages/fiber/src/core/store.ts | 3 +- packages/fiber/src/core/utils.ts | 120 +++----- packages/fiber/src/index.tsx | 2 +- packages/fiber/src/native.tsx | 2 +- 7 files changed, 237 insertions(+), 353 deletions(-) diff --git a/packages/fiber/src/core/events.ts b/packages/fiber/src/core/events.ts index 749d17ca65..dd2c07ab0f 100644 --- a/packages/fiber/src/core/events.ts +++ b/packages/fiber/src/core/events.ts @@ -173,7 +173,7 @@ export function createEvents(store: UseBoundStore) { function filterPointerEvents(objects: THREE.Object3D[]) { return objects.filter((obj) => ['Move', 'Over', 'Enter', 'Out', 'Leave'].some( - (name) => (obj as unknown as Instance).__r3f?.handlers[('onPointer' + name) as keyof EventHandlers], + (name) => ((obj as any).__r3f as Instance)?.handlers[('onPointer' + name) as keyof EventHandlers], ), ) } @@ -239,7 +239,7 @@ export function createEvents(store: UseBoundStore) { let eventObject: THREE.Object3D | null = hit.object // Bubble event up while (eventObject) { - if ((eventObject as unknown as Instance).__r3f?.eventCount) intersections.push({ ...hit, eventObject }) + if (((eventObject as any).__r3f as Instance)?.eventCount) intersections.push({ ...hit, eventObject }) eventObject = eventObject.parent } } @@ -369,7 +369,7 @@ export function createEvents(store: UseBoundStore) { ) ) { const eventObject = hoveredObj.eventObject - const instance = (eventObject as unknown as Instance).__r3f + const instance = (eventObject as any).__r3f as Instance const handlers = instance?.handlers internal.hovered.delete(makeId(hoveredObj)) if (instance?.eventCount) { @@ -434,7 +434,7 @@ export function createEvents(store: UseBoundStore) { handleIntersects(hits, event, delta, (data: ThreeEvent) => { const eventObject = data.eventObject - const instance = (eventObject as unknown as Instance).__r3f + const instance = (eventObject as any).__r3f as Instance const handlers = instance?.handlers // Check presence of handlers if (!instance?.eventCount) return @@ -487,9 +487,7 @@ export function createEvents(store: UseBoundStore) { } function pointerMissed(event: MouseEvent, objects: THREE.Object3D[]) { - objects.forEach((object: THREE.Object3D) => - (object as unknown as Instance).__r3f?.handlers.onPointerMissed?.(event), - ) + objects.forEach((object: THREE.Object3D) => ((object as any).__r3f as Instance)?.handlers.onPointerMissed?.(event)) } return { handlePointer } diff --git a/packages/fiber/src/core/index.tsx b/packages/fiber/src/core/index.tsx index 2799b22423..45be9f225e 100644 --- a/packages/fiber/src/core/index.tsx +++ b/packages/fiber/src/core/index.tsx @@ -30,7 +30,7 @@ import { updateCamera, setDeep, } from './utils' -import { useStore } from './hooks' +import { useStore, useThree } from './hooks' const roots = new Map() const { invalidate, advance } = createLoop(roots) @@ -332,6 +332,11 @@ function render( return root.render(children) } +function Scene({ children }: { children: React.ReactNode }) { + const scene = useThree((state) => state.scene) + return {children} +} + function Provider({ store, children, @@ -355,7 +360,11 @@ function Provider({ if (!store.getState().events.connected) state.events.connect?.(rootElement) // eslint-disable-next-line react-hooks/exhaustive-deps }, []) - return {children} + return ( + + {children} + + ) } function unmountComponentAtNode(canvas: TElement, callback?: (canvas: TElement) => void) { @@ -506,7 +515,9 @@ function Portal({ return ( <> {reconciler.createPortal( - {children}, + + {children} + , usePortalStore, null, )} diff --git a/packages/fiber/src/core/renderer.ts b/packages/fiber/src/core/renderer.ts index 3ace7cd099..836f52b552 100644 --- a/packages/fiber/src/core/renderer.ts +++ b/packages/fiber/src/core/renderer.ts @@ -3,39 +3,39 @@ import { UseBoundStore } from 'zustand' import Reconciler from 'react-reconciler' import { unstable_IdlePriority as idlePriority, unstable_scheduleCallback as scheduleCallback } from 'scheduler' import { DefaultEventPriority } from 'react-reconciler/constants' -import { - is, - prepare, - diffProps, - DiffSet, - applyProps, - updateInstance, - invalidateInstance, - attach, - detach, -} from './utils' +import { is, diffProps, DiffSet, applyProps, invalidateInstance, attach, detach } from './utils' import { RootState } from './store' import { EventHandlers, removeInteractivity } from './events' export type Root = { fiber: Reconciler.FiberRoot; store: UseBoundStore } -export type LocalState = { - type: string +export type AttachFnType = (parent: any, self: any) => () => void +export type AttachType = string | AttachFnType + +export type InstanceProps = { + [key: string]: unknown +} & { + args?: any[] + object?: any + visible?: boolean + dispose?: null + attach?: AttachType +} + +export interface Instance { root: UseBoundStore - // objects and parent are used when children are added with `attach` instead of being added to the Object3D scene graph - objects: Instance[] + type: string parent: Instance | null - primitive?: boolean + children: Instance[] + props: InstanceProps + object: any | null + eventCount: number handlers: Partial attach?: AttachType - previousAttach: any - memoizedProps: { [key: string]: any } + previousAttach?: any } -export type AttachFnType = (parent: Instance, self: Instance) => () => void -export type AttachType = string | AttachFnType - interface HostConfig { type: string props: InstanceProps @@ -44,234 +44,174 @@ interface HostConfig { textInstance: never suspenseInstance: Instance hydratableInstance: Instance - publicInstance: Instance + publicInstance: any hostContext: never - updatePayload: Array + updatePayload: null | [true] | [false, DiffSet] childSet: never timeoutHandle: number | undefined noTimeout: -1 } -// This type clamps down on a couple of assumptions that we can make regarding native types, which -// could anything from scene objects, THREE.Objects, JSM, user-defined classes and non-scene objects. -// What they all need to have in common is defined here ... -export type BaseInstance = Omit & { - __r3f: LocalState - children: Instance[] - remove: (...object: Instance[]) => Instance - add: (...object: Instance[]) => Instance - raycast?: (raycaster: THREE.Raycaster, intersects: THREE.Intersection[]) => void -} -export type Instance = BaseInstance & { [key: string]: any } - -export type InstanceProps = { - [key: string]: unknown -} & { - args?: any[] - object?: object - visible?: boolean - dispose?: null - attach?: AttachType -} - interface Catalogue { [name: string]: { - new (...args: any): Instance + new (...args: any): any } } -let catalogue: Catalogue = {} -let extend = (objects: object): void => void (catalogue = { ...catalogue, ...objects }) +const catalogue: Catalogue = {} +const extend = (objects: object): void => void Object.assign(catalogue, objects) function createRenderer(_roots: Map, _getEventPriority?: () => any) { function createInstance( type: string, - { args = [], attach, ...props }: InstanceProps, + { args = [], object = null, ...props }: InstanceProps, root: UseBoundStore, - ) { - let name = `${type[0].toUpperCase()}${type.slice(1)}` - let instance: Instance - - // Auto-attach geometries and materials - if (attach === undefined) { - if (name.endsWith('Geometry')) attach = 'geometry' - else if (name.endsWith('Material')) attach = 'material' + ): Instance { + const name = `${type[0].toUpperCase()}${type.slice(1)}` + const target = catalogue[name] + + if (type !== 'primitive' && !target) + throw `${name} is not part of the THREE namespace! Did you forget to extend? See: https://docs.pmnd.rs/react-three-fiber/api/objects#using-3rd-party-objects-declaratively` + + if (type === 'primitive' && !object) throw `Primitives without 'object' are invalid!` + + const instance: Instance = { + root, + type, + parent: null, + children: [], + props: { ...props, args }, + object, + eventCount: 0, + handlers: {}, } - if (type === 'primitive') { - if (props.object === undefined) throw `Primitives without 'object' are invalid!` - const object = props.object as Instance - instance = prepare(object, { type, root, attach, primitive: true }) - } else { - const target = catalogue[name] - if (!target) { - throw `${name} is not part of the THREE namespace! Did you forget to extend? See: https://docs.pmnd.rs/react-three-fiber/api/objects#using-3rd-party-objects-declaratively` - } + if (object) object.__r3f = instance - // Throw if an object or literal was passed for args - if (!Array.isArray(args)) throw 'The args prop must be an array!' - - // Instanciate new object, link it to the root - // Append memoized props with args so it's not forgotten - instance = prepare(new target(...args), { - type, - root, - attach, - // Save args in case we need to reconstruct later for HMR - memoizedProps: { args }, - }) - } - - // It should NOT call onUpdate on object instanciation, because it hasn't been added to the - // view yet. If the callback relies on references for instance, they won't be ready yet, this is - // why it passes "true" here - // There is no reason to apply props to injects - if (name !== 'inject') applyProps(instance, props) return instance } - function appendChild(parentInstance: HostConfig['instance'], child: HostConfig['instance']) { - let added = false - if (child) { - // The attach attribute implies that the object attaches itself on the parent - if (child.__r3f?.attach) { - attach(parentInstance, child, child.__r3f.attach) - } else if (child.isObject3D && parentInstance.isObject3D) { - // add in the usual parent-child way - parentInstance.add(child) - added = true - } - // This is for anything that used attach, and for non-Object3Ds that don't get attached to props; - // that is, anything that's a child in React but not a child in the scenegraph. - if (!added) parentInstance.__r3f?.objects.push(child) - if (!child.__r3f) prepare(child, {}) - child.__r3f.parent = parentInstance - updateInstance(child) - invalidateInstance(child) - } + function appendChild(parent: HostConfig['instance'], child: HostConfig['instance']) { + child.parent = parent + parent.children.push(child) } function insertBefore( - parentInstance: HostConfig['instance'], + parent: HostConfig['instance'], child: HostConfig['instance'], beforeChild: HostConfig['instance'], ) { - let added = false - if (child) { - if (child.__r3f?.attach) { - attach(parentInstance, child, child.__r3f.attach) - } else if (child.isObject3D && parentInstance.isObject3D) { - child.parent = parentInstance as unknown as THREE.Object3D - child.dispatchEvent({ type: 'added' }) - const restSiblings = parentInstance.children.filter((sibling) => sibling !== child) - const index = restSiblings.indexOf(beforeChild) - parentInstance.children = [...restSiblings.slice(0, index), child, ...restSiblings.slice(index)] - added = true - } + if (!child) return - if (!added) parentInstance.__r3f?.objects.push(child) - if (!child.__r3f) prepare(child, {}) - child.__r3f.parent = parentInstance - updateInstance(child) - invalidateInstance(child) - } + child.parent = parent + parent.children.splice(parent.children.indexOf(beforeChild), 0, child) } - function removeRecursive(array: HostConfig['instance'][], parent: HostConfig['instance'], dispose: boolean = false) { - if (array) [...array].forEach((child) => removeChild(parent, child, dispose)) - } - - function removeChild(parentInstance: HostConfig['instance'], child: HostConfig['instance'], dispose?: boolean) { - if (child) { - // Clear the parent reference - if (child.__r3f) child.__r3f.parent = null - // Remove child from the parents objects - if (parentInstance.__r3f?.objects) - parentInstance.__r3f.objects = parentInstance.__r3f.objects.filter((x) => x !== child) - // Remove attachment - if (child.__r3f?.attach) { - detach(parentInstance, child, child.__r3f.attach) - } else if (child.isObject3D && parentInstance.isObject3D) { - parentInstance.remove(child) - // Remove interactivity - if (child.__r3f?.root) { - removeInteractivity(child.__r3f.root, child as unknown as THREE.Object3D) - } - } - - // Allow objects to bail out of recursive dispose altogether by passing dispose={null} - // Never dispose of primitives because their state may be kept outside of React! - // In order for an object to be able to dispose it has to have - // - a dispose method, - // - it cannot be a - // - it cannot be a THREE.Scene, because three has broken it's own api - // - // Since disposal is recursive, we can check the optional dispose arg, which will be undefined - // when the reconciler calls it, but then carry our own check recursively - const isPrimitive = child.__r3f?.primitive - const shouldDispose = dispose === undefined ? child.dispose !== null && !isPrimitive : dispose - - // Remove nested child objects. Primitives should not have objects and children that are - // attached to them declaratively ... - if (!isPrimitive) { - removeRecursive(child.__r3f?.objects, child, shouldDispose) - removeRecursive(child.children, child, shouldDispose) - } + function removeChild(parent: HostConfig['instance'], child: HostConfig['instance']) { + child.parent = null + const childIndex = parent.children.indexOf(child) + if (childIndex !== -1) parent.children.splice(childIndex, 1) - // Remove references - if (child.__r3f) { - delete ((child as Partial).__r3f as Partial).root - delete ((child as Partial).__r3f as Partial).objects - delete ((child as Partial).__r3f as Partial).handlers - delete ((child as Partial).__r3f as Partial).memoizedProps - if (!isPrimitive) delete (child as Partial).__r3f - } + if (child.props.attach) detach(parent, child, child.props.attach) + else if (child.object.isObject3D && parent.object.isObject3D) { + parent.object.remove(child.object) + removeInteractivity(child.root, child.object as unknown as THREE.Object3D) + } - // Dispose item whenever the reconciler feels like it - if (shouldDispose && child.dispose && child.type !== 'Scene') { + // Dispose object whenever the reconciler feels like it + if (child.type !== 'primitive' && child.type !== 'scene' && child.props.dispose !== null) { + const dispose = child.object.dispose + if (typeof dispose === 'function') { scheduleCallback(idlePriority, () => { try { - child.dispose() + dispose() } catch (e) { /* ... */ } }) } + } + child.object = null + + invalidateInstance(child) + } + + function commitInstance(instance: Instance) { + // Don't handle commit for containers + if (!instance.parent) return + + // Create object + if (instance.type !== 'primitive' && !instance.object) { + const name = `${instance.type[0].toUpperCase()}${instance.type.slice(1)}` + const target = catalogue[name] + + const { args = [] } = instance.props + instance.object = new target(...args) + instance.object.__r3f = instance + } + + // Auto-attach geometry and materials to meshes + if (!instance.props.attach) { + if (instance.type.endsWith('Geometry')) instance.props.attach = 'geometry' + else if (instance.type.endsWith('Material')) instance.props.attach = 'material' + } - invalidateInstance(parentInstance) + // Append children + for (const child of instance.children) { + if (child.props.attach) attach(instance, child, child.props.attach) + else if (child.object.isObject3D) instance.object.add(child.object) } + + // Append to container + if (!instance.parent.parent) { + if (instance.props.attach) attach(instance.parent, instance, instance.props.attach) + else if (instance.object.isObject3D) instance.parent.object.add(instance.object) + } + + // Apply props to object + applyProps(instance.object, instance.props) + + // Add interactivity + if (instance.object.raycast && instance.handlers && instance.eventCount) { + instance.root.getState().internal.interaction.push(instance.object as unknown as THREE.Object3D) + } + + invalidateInstance(instance) } function switchInstance( instance: HostConfig['instance'], type: HostConfig['type'], - newProps: HostConfig['props'], + props: HostConfig['props'], fiber: Reconciler.Fiber, ) { - const parent = instance.__r3f?.parent - if (!parent) return - - const newInstance = createInstance(type, newProps, instance.__r3f.root) - - // https://github.com/pmndrs/react-three-fiber/issues/1348 - // When args change the instance has to be re-constructed, which then - // forces r3f to re-parent the children and non-scene objects - // This can not include primitives, which should not have declarative children - if (type !== 'primitive' && instance.children) { - instance.children.forEach((child) => appendChild(newInstance, child)) - instance.children = [] - } - - instance.__r3f.objects.forEach((child) => appendChild(newInstance, child)) - instance.__r3f.objects = [] + // Create a new instance + const newInstance = createInstance(type, props, instance.root) + // Replace instance in scene-graph + const parent = instance.parent! removeChild(parent, instance) appendChild(parent, newInstance) - // Re-bind event handlers - if (newInstance.raycast && newInstance.__r3f.eventCount) { - const rootState = newInstance.__r3f.root.getState() - rootState.internal.interaction.push(newInstance as unknown as THREE.Object3D) + // Commit new instance object + commitInstance(newInstance) + + // Append to scene-graph + if (parent.parent) { + if (newInstance.props.attach) attach(parent, newInstance, newInstance.props.attach) + else if (newInstance.object.isObject3D) parent.object.add(newInstance.object) + } + + // Move children to new instance + if (instance.type !== 'primitive') { + for (const child of instance.children) { + appendChild(newInstance, child) + if (child.props.attach) { + detach(instance, child, child.props.attach) + attach(newInstance, child, child.props.attach) + } + } + instance.children = [] } // This evil hack switches the react-internal fiber node @@ -281,11 +221,13 @@ function createRenderer(_roots: Map, _getEventPriority?: if (fiber !== null) { fiber.stateNode = newInstance if (fiber.ref) { - if (typeof fiber.ref === 'function') (fiber as unknown as any).ref(newInstance) - else (fiber.ref as Reconciler.RefObject).current = newInstance + if (typeof fiber.ref === 'function') (fiber as unknown as any).ref(newInstance.object) + else (fiber.ref as Reconciler.RefObject).current = newInstance.object } } }) + + return newInstance } const reconciler = Reconciler< @@ -303,88 +245,56 @@ function createRenderer(_roots: Map, _getEventPriority?: HostConfig['timeoutHandle'], HostConfig['noTimeout'] >({ - createInstance, - removeChild, - appendChild, - appendInitialChild: appendChild, - insertBefore, supportsMutation: true, isPrimaryRenderer: false, supportsPersistence: false, supportsHydration: false, noTimeout: -1, - appendChildToContainer: (container, child) => { - const scene = container.getState().scene as unknown as Instance - // Link current root to the default scene - scene.__r3f.root = container - appendChild(scene, child) - }, - removeChildFromContainer: (container, child) => - removeChild(container.getState().scene as unknown as Instance, child), - insertInContainerBefore: (container, child, beforeChild) => - insertBefore(container.getState().scene as unknown as Instance, child, beforeChild), + createInstance, + removeChild, + appendChild, + appendInitialChild: appendChild, + insertBefore, + appendChildToContainer: () => {}, + removeChildFromContainer: () => {}, + insertInContainerBefore: () => {}, getRootHostContext: () => null, getChildHostContext: (parentHostContext) => parentHostContext, - finalizeInitialChildren(instance) { - const localState = instance?.__r3f ?? {} - // https://github.com/facebook/react/issues/20271 - // Returning true will trigger commitMount - return Boolean(localState.handlers) + prepareUpdate(instance, type, oldProps, newProps) { + // Reconstruct primitives if object prop changes + if (type === 'primitive' && oldProps.object !== newProps.object) return [true] + // Reconstruct elements if args change + if (newProps.args?.some((value, index) => value !== oldProps.args?.[index])) return [true] + + // Create a diff-set, flag if there are any changes + const diff = diffProps(instance, newProps, oldProps, true) + if (diff.changes.length) return [false, diff] + + // Otherwise do not touch the instance + return null }, - prepareUpdate(instance, _type, oldProps, newProps) { - // Create diff-sets - if (instance.__r3f.primitive && newProps.object && newProps.object !== instance) { - return [true] - } else { - // This is a data object, let's extract critical information about it - const { args: argsNew = [], children: cN, ...restNew } = newProps - const { args: argsOld = [], children: cO, ...restOld } = oldProps - - // Throw if an object or literal was passed for args - if (!Array.isArray(argsNew)) throw 'The args prop must be an array!' - - // If it has new props or arguments, then it needs to be re-instantiated - if (argsNew.some((value, index) => value !== argsOld[index])) return [true] - // Create a diff-set, flag if there are any changes - const diff = diffProps(instance, restNew, restOld, true) - if (diff.changes.length) return [false, diff] - - // Otherwise do not touch the instance - return null - } - }, - commitUpdate(instance, [reconstruct, diff]: [boolean, DiffSet], type, _oldProps, newProps, fiber) { + commitUpdate(instance, diff, type, _oldProps, newProps, fiber) { + const [reconstruct, changedProps] = diff! + // Reconstruct when args or instance, + finalizeInitialChildren: () => true, + commitMount: commitInstance, + getPublicInstance: (instance) => instance.object, prepareForCommit: () => null, - preparePortalMount: (container) => prepare(container.getState().scene), + preparePortalMount: (container) => container, resetAfterCommit: () => {}, shouldSetTextContent: () => false, clearContainer: () => false, hideInstance(instance) { - // Deatch while the instance is hidden - const { attach: type, parent } = instance?.__r3f ?? {} - if (type && parent) detach(parent, instance, type) - if (instance.isObject3D) instance.visible = false + if (instance.object.isObject3D) instance.object.visible = false invalidateInstance(instance) }, - unhideInstance(instance, props) { - // Re-attach when the instance is unhidden - const { attach: type, parent } = instance?.__r3f ?? {} - if (type && parent) attach(parent, instance, type) - if ((instance.isObject3D && props.visible == null) || props.visible) instance.visible = true + unhideInstance(instance) { + if (instance.object.isObject3D && instance.props.visible !== false) instance.object.visible = true invalidateInstance(instance) }, createTextInstance: () => { @@ -414,4 +324,4 @@ function createRenderer(_roots: Map, _getEventPriority?: return { reconciler, applyProps } } -export { prepare, createRenderer, extend } +export { createRenderer, extend } diff --git a/packages/fiber/src/core/store.ts b/packages/fiber/src/core/store.ts index ba5ef7629e..2fb2100919 100644 --- a/packages/fiber/src/core/store.ts +++ b/packages/fiber/src/core/store.ts @@ -1,7 +1,6 @@ import * as THREE from 'three' import * as React from 'react' import create, { GetState, SetState, StoreApi, UseBoundStore } from 'zustand' -import { prepare } from './renderer' import { DomEvent, EventManager, PointerCaptureTarget, ThreeEvent } from './events' import { calculateDpr, Camera, isOrthographicCamera, updateCamera } from './utils' @@ -204,7 +203,7 @@ const createStore = ( legacy: false, linear: false, flat: false, - scene: prepare(new THREE.Scene()), + scene: new THREE.Scene(), controls: null, clock: new THREE.Clock(), diff --git a/packages/fiber/src/core/utils.ts b/packages/fiber/src/core/utils.ts index 6784c10eae..4a0d1cfb11 100644 --- a/packages/fiber/src/core/utils.ts +++ b/packages/fiber/src/core/utils.ts @@ -1,8 +1,7 @@ import * as THREE from 'three' import * as React from 'react' -import { UseBoundStore } from 'zustand' import { EventHandlers } from './events' -import { AttachType, Instance, InstanceProps, LocalState } from './renderer' +import { AttachType, Instance, InstanceProps } from './renderer' import { Dpr, RootState, Size } from './store' export type Camera = THREE.OrthographicCamera | THREE.PerspectiveCamera @@ -50,11 +49,10 @@ export class ErrorBoundary extends React.Component< export const DEFAULT = '__default' export type DiffSet = { - memoized: { [key: string]: any } changes: [key: string, value: unknown, isEvent: boolean, keys: string[]][] } -export const isDiffSet = (def: any): def is DiffSet => def && !!(def as DiffSet).memoized && !!(def as DiffSet).changes +export const isDiffSet = (def: any): def is DiffSet => def && !!(def as DiffSet).changes export type ClassConstructor = { new (): void } export type ObjectMap = { @@ -70,7 +68,7 @@ export function calculateDpr(dpr: Dpr) { * Returns instance root state */ export const getRootState = (obj: THREE.Object3D): RootState | undefined => - (obj as unknown as Instance).__r3f?.root.getState() + ((obj as any).__r3f as Instance)?.root.getState() export type EquConfig = { /** Compare arrays by reference equality a === b (default), or by shallow equality */ @@ -135,26 +133,7 @@ export function dispose void; type?: string; [key } } -// Each object in the scene carries a small LocalState descriptor -export function prepare(object: T, state?: Partial) { - const instance = object as unknown as Instance - if (state?.primitive || !instance.__r3f) { - instance.__r3f = { - type: '', - root: null as unknown as UseBoundStore, - previousAttach: null, - memoizedProps: {}, - eventCount: 0, - handlers: {}, - objects: [], - parent: null, - ...state, - } - } - return object -} - -function resolve(instance: Instance, key: string) { +function resolve(instance: any, key: string) { let target = instance if (key.includes('-')) { const entries = key.split('-') @@ -172,26 +151,26 @@ export function attach(parent: Instance, child: Instance, type: AttachType) { // If attaching into an array (foo-0), create one if (INDEX_REGEX.test(type)) { const root = type.replace(INDEX_REGEX, '') - const { target, key } = resolve(parent, root) + const { target, key } = resolve(parent.object, root) if (!Array.isArray(target[key])) target[key] = [] } - const { target, key } = resolve(parent, type) - child.__r3f.previousAttach = target[key] - target[key] = child - } else child.__r3f.previousAttach = type(parent, child) + const { target, key } = resolve(parent.object, type) + child.previousAttach = target[key] + target[key] = child.object + } else child.previousAttach = type(parent.object, child.object) } export function detach(parent: Instance, child: Instance, type: AttachType) { if (is.str(type)) { - const { target, key } = resolve(parent, type) - const previous = child.__r3f.previousAttach + const { target, key } = resolve(parent.object, type) + const previous = child.previousAttach // When the previous value was undefined, it means the value was never set to begin with - if (previous === undefined) delete target[key] + if (previous === undefined) delete target.object[key] // Otherwise set the previous value - else target[key] = previous - } else child.__r3f?.previousAttach?.(parent, child) - delete child.__r3f?.previousAttach + else target.object[key] = previous + } else child.previousAttach?.(parent.object, child.object) + delete child.previousAttach } // This function prepares a set of changes to be applied to the instance @@ -201,7 +180,6 @@ export function diffProps( { children: cP, key: kP, ref: rP, ...previous }: InstanceProps = {}, remove = false, ): DiffSet { - const localState = (instance?.__r3f ?? {}) as LocalState const entries = Object.entries(props) const changes: [key: string, value: unknown, isEvent: boolean, keys: string[]][] = [] @@ -215,7 +193,7 @@ export function diffProps( entries.forEach(([key, value]) => { // Bail out on primitive object - if (instance.__r3f?.primitive && key === 'object') return + if (instance.type === 'primitive' && key === 'object') return // When props match bail out if (is.equ(value, previous[key])) return // Collect handlers and bail out @@ -226,36 +204,29 @@ export function diffProps( changes.push([key, value, false, entries]) }) - const memoized: { [key: string]: any } = { ...props } - if (localState.memoizedProps && localState.memoizedProps.args) memoized.args = localState.memoizedProps.args - if (localState.memoizedProps && localState.memoizedProps.attach) memoized.attach = localState.memoizedProps.attach - - return { memoized, changes } + return { changes } } // This function applies a set of changes to the instance -export function applyProps(instance: Instance, data: InstanceProps | DiffSet) { +export function applyProps(object: any, data: InstanceProps | DiffSet) { // Filter equals, events and reserved props - const localState = (instance.__r3f ?? {}) as LocalState - const root = localState.root + const instance = (object.__r3f ?? {}) as Instance + const root = instance.root const rootState = root?.getState?.() ?? {} - const { memoized, changes } = isDiffSet(data) ? data : diffProps(instance, data) - const prevHandlers = localState.eventCount - - // Prepare memoized props - if (instance.__r3f) instance.__r3f.memoizedProps = memoized + const { changes } = isDiffSet(data) ? data : diffProps(instance, data) + const prevHandlers = instance.eventCount changes.forEach(([key, value, isEvent, keys]) => { - let currentInstance = instance - let targetProp = currentInstance[key] + let target = object + let targetProp = target[key] // Revolve dashed props if (keys.length) { - targetProp = keys.reduce((acc, key) => acc[key], instance) + targetProp = keys.reduce((acc, key) => acc[key], object) // If the target is atomic, it forces us to switch the root if (!(targetProp && targetProp.set)) { const [name, ...reverseEntries] = keys.reverse() - currentInstance = reverseEntries.reverse().reduce((acc, key) => acc[key], instance) + target = reverseEntries.reverse().reduce((acc, key) => acc[key], object) key = name } } @@ -266,27 +237,29 @@ export function applyProps(instance: Instance, data: InstanceProps | DiffSet) { // with their respective constructor/set arguments // For removed props, try to set default values, if possible if (value === DEFAULT + 'remove') { + const args = instance.props?.args ?? [] + if (targetProp && targetProp.constructor) { // use the prop constructor to find the default it should be - value = new targetProp.constructor(...(memoized.args ?? [])) - } else if (currentInstance.constructor) { + value = new targetProp.constructor(...args) + } else if (target.constructor) { // create a blank slate of the instance and copy the particular parameter. // @ts-ignore - const defaultClassCall = new currentInstance.constructor(...(currentInstance.__r3f.memoizedProps.args ?? [])) + const defaultClassCall = new target.constructor(...args) value = defaultClassCall[targetProp] - // destory the instance + // destroy the instance if (defaultClassCall.dispose) defaultClassCall.dispose() - // instance does not have constructor, just set it to 0 } else { + // instance does not have constructor, just set it to 0 value = 0 } } // Deal with pointer events ... if (isEvent) { - if (value) localState.handlers[key as keyof EventHandlers] = value as any - else delete localState.handlers[key as keyof EventHandlers] - localState.eventCount = Object.keys(localState.handlers).length + if (value) instance.handlers[key as keyof EventHandlers] = value as any + else delete instance.handlers[key as keyof EventHandlers] + instance.eventCount = Object.keys(instance.handlers).length } // Special treatment for objects with support for set/copy, and layers else if (targetProp && targetProp.set && (targetProp.copy || targetProp instanceof THREE.Layers)) { @@ -322,40 +295,33 @@ export function applyProps(instance: Instance, data: InstanceProps | DiffSet) { } // Else, just overwrite the value } else { - currentInstance[key] = value + target[key] = value // Auto-convert sRGB textures, for now ... // https://github.com/pmndrs/react-three-fiber/issues/344 - if (!rootState.linear && currentInstance[key] instanceof THREE.Texture) { - currentInstance[key].encoding = THREE.sRGBEncoding + if (!rootState.linear && target[key] instanceof THREE.Texture) { + target[key].encoding = THREE.sRGBEncoding } } invalidateInstance(instance) }) - if (localState.parent && rootState.internal && instance.raycast && prevHandlers !== localState.eventCount) { + if (instance.parent && rootState.internal && instance.object.raycast && prevHandlers !== instance.eventCount) { // Pre-emptively remove the instance from the interaction manager - const index = rootState.internal.interaction.indexOf(instance as unknown as THREE.Object3D) + const index = rootState.internal.interaction.indexOf(instance.object as unknown as THREE.Object3D) if (index > -1) rootState.internal.interaction.splice(index, 1) // Add the instance to the interaction manager only when it has handlers - if (localState.eventCount) rootState.internal.interaction.push(instance as unknown as THREE.Object3D) + if (instance.eventCount) rootState.internal.interaction.push(instance.object as unknown as THREE.Object3D) } - // Call the update lifecycle when it is being updated, but only when it is part of the scene - if (changes.length && instance.parent) updateInstance(instance) - return instance } export function invalidateInstance(instance: Instance) { - const state = instance.__r3f?.root?.getState?.() + const state = instance.root?.getState?.() if (state && state.internal.frames === 0) state.invalidate() } -export function updateInstance(instance: Instance) { - instance.onUpdate?.(instance) -} - export function updateCamera(camera: Camera & { manual?: boolean }, size: Size) { // https://github.com/pmndrs/react-three-fiber/issues/92 // Do not mess with the camera if it belongs to the user diff --git a/packages/fiber/src/index.tsx b/packages/fiber/src/index.tsx index 5ba96c420c..6109f3fd52 100644 --- a/packages/fiber/src/index.tsx +++ b/packages/fiber/src/index.tsx @@ -1,7 +1,7 @@ export * from './three-types' import * as ReactThreeFiber from './three-types' export { ReactThreeFiber } -export type { BaseInstance, LocalState } from './core/renderer' +export type { Instance, InstanceProps } from './core/renderer' export type { Intersection, Subscription, diff --git a/packages/fiber/src/native.tsx b/packages/fiber/src/native.tsx index 667a0596e5..b17dc13e71 100644 --- a/packages/fiber/src/native.tsx +++ b/packages/fiber/src/native.tsx @@ -1,7 +1,7 @@ export * from './three-types' import * as ReactThreeFiber from './three-types' export { ReactThreeFiber } -export type { BaseInstance, LocalState } from './core/renderer' +export type { Instance, InstanceProps } from './core/renderer' export type { Intersection, Subscription, From 7eccdc0290dde3c2e4808eef75a096c0a8fa72a2 Mon Sep 17 00:00:00 2001 From: Cody Bennett <23324155+CodyJasonBennett@users.noreply.github.com> Date: Thu, 21 Jul 2022 11:46:31 -0500 Subject: [PATCH 02/28] fix(detach): overwrite target --- packages/fiber/src/core/utils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/fiber/src/core/utils.ts b/packages/fiber/src/core/utils.ts index 4a0d1cfb11..8979040353 100644 --- a/packages/fiber/src/core/utils.ts +++ b/packages/fiber/src/core/utils.ts @@ -166,9 +166,9 @@ export function detach(parent: Instance, child: Instance, type: AttachType) { const { target, key } = resolve(parent.object, type) const previous = child.previousAttach // When the previous value was undefined, it means the value was never set to begin with - if (previous === undefined) delete target.object[key] + if (previous === undefined) delete target[key] // Otherwise set the previous value - else target.object[key] = previous + else target[key] = previous } else child.previousAttach?.(parent.object, child.object) delete child.previousAttach } From 30b3227d0ecab863b8dab78c8d242f818c66bda5 Mon Sep 17 00:00:00 2001 From: Cody Bennett <23324155+CodyJasonBennett@users.noreply.github.com> Date: Thu, 21 Jul 2022 11:48:35 -0500 Subject: [PATCH 03/28] fix(renderer): add back removerecursive --- packages/fiber/src/core/renderer.ts | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/packages/fiber/src/core/renderer.ts b/packages/fiber/src/core/renderer.ts index 836f52b552..08a66c42b0 100644 --- a/packages/fiber/src/core/renderer.ts +++ b/packages/fiber/src/core/renderer.ts @@ -107,7 +107,11 @@ function createRenderer(_roots: Map, _getEventPriority?: parent.children.splice(parent.children.indexOf(beforeChild), 0, child) } - function removeChild(parent: HostConfig['instance'], child: HostConfig['instance']) { + function removeRecursive(array: HostConfig['instance'][], parent: HostConfig['instance'], dispose: boolean = false) { + if (array) [...array].forEach((child) => removeChild(parent, child, dispose)) + } + + function removeChild(parent: HostConfig['instance'], child: HostConfig['instance'], dispose?: boolean) { child.parent = null const childIndex = parent.children.indexOf(child) if (childIndex !== -1) parent.children.splice(childIndex, 1) @@ -118,8 +122,24 @@ function createRenderer(_roots: Map, _getEventPriority?: removeInteractivity(child.root, child.object as unknown as THREE.Object3D) } + // Allow objects to bail out of recursive dispose altogether by passing dispose={null} + // Never dispose of primitives because their state may be kept outside of React! + // In order for an object to be able to dispose it has to have + // - a dispose method, + // - it cannot be a + // - it cannot be a THREE.Scene, because three has broken its own api + // + // Since disposal is recursive, we can check the optional dispose arg, which will be undefined + // when the reconciler calls it, but then carry our own check recursively + const isPrimitive = child.type === 'primitive' + const shouldDispose = dispose ?? (!isPrimitive && child.props.dispose !== null) + + // Remove nested child objects. Primitives should not have objects and children that are + // attached to them declaratively ... + if (!isPrimitive) removeRecursive(child.children, child, shouldDispose) + // Dispose object whenever the reconciler feels like it - if (child.type !== 'primitive' && child.type !== 'scene' && child.props.dispose !== null) { + if (child.type !== 'scene' && shouldDispose) { const dispose = child.object.dispose if (typeof dispose === 'function') { scheduleCallback(idlePriority, () => { @@ -133,7 +153,7 @@ function createRenderer(_roots: Map, _getEventPriority?: } child.object = null - invalidateInstance(child) + if (dispose === undefined) invalidateInstance(child) } function commitInstance(instance: Instance) { From ee03b1d1980e74fa7cc57483137c6c2a5d21ee89 Mon Sep 17 00:00:00 2001 From: Cody Bennett <23324155+CodyJasonBennett@users.noreply.github.com> Date: Thu, 21 Jul 2022 11:48:57 -0500 Subject: [PATCH 04/28] fix(renderer): safely check objects when hiding/suspense --- packages/fiber/src/core/renderer.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/fiber/src/core/renderer.ts b/packages/fiber/src/core/renderer.ts index 08a66c42b0..0ae993e2ab 100644 --- a/packages/fiber/src/core/renderer.ts +++ b/packages/fiber/src/core/renderer.ts @@ -310,11 +310,11 @@ function createRenderer(_roots: Map, _getEventPriority?: shouldSetTextContent: () => false, clearContainer: () => false, hideInstance(instance) { - if (instance.object.isObject3D) instance.object.visible = false + if (instance.object?.isObject3D) instance.object.visible = false invalidateInstance(instance) }, unhideInstance(instance) { - if (instance.object.isObject3D && instance.props.visible !== false) instance.object.visible = true + if (instance.object?.isObject3D && instance.props.visible !== false) instance.object.visible = true invalidateInstance(instance) }, createTextInstance: () => { From 5e97a14863d014da1fc5c696ebbf53add19d42bc Mon Sep 17 00:00:00 2001 From: Cody Bennett <23324155+CodyJasonBennett@users.noreply.github.com> Date: Thu, 21 Jul 2022 12:56:35 -0500 Subject: [PATCH 05/28] fix(applyProps): use target args on HMR copy --- packages/fiber/src/core/utils.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/fiber/src/core/utils.ts b/packages/fiber/src/core/utils.ts index 8979040353..604f8765b8 100644 --- a/packages/fiber/src/core/utils.ts +++ b/packages/fiber/src/core/utils.ts @@ -237,15 +237,13 @@ export function applyProps(object: any, data: InstanceProps | DiffSet) { // with their respective constructor/set arguments // For removed props, try to set default values, if possible if (value === DEFAULT + 'remove') { - const args = instance.props?.args ?? [] - if (targetProp && targetProp.constructor) { // use the prop constructor to find the default it should be - value = new targetProp.constructor(...args) + value = new targetProp.constructor(...(instance.props.args ?? [])) } else if (target.constructor) { // create a blank slate of the instance and copy the particular parameter. // @ts-ignore - const defaultClassCall = new target.constructor(...args) + const defaultClassCall = new target.constructor(...(target.__r3f?.props.args ?? [])) value = defaultClassCall[targetProp] // destroy the instance if (defaultClassCall.dispose) defaultClassCall.dispose() From ebced55a2cd49fb768efc09f02fa2923efeb45ff Mon Sep 17 00:00:00 2001 From: Cody Bennett <23324155+CodyJasonBennett@users.noreply.github.com> Date: Thu, 21 Jul 2022 13:14:43 -0500 Subject: [PATCH 06/28] fix(diffProps): filter reserved keys --- packages/fiber/src/core/utils.ts | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/packages/fiber/src/core/utils.ts b/packages/fiber/src/core/utils.ts index 604f8765b8..78d0848808 100644 --- a/packages/fiber/src/core/utils.ts +++ b/packages/fiber/src/core/utils.ts @@ -173,24 +173,32 @@ export function detach(parent: Instance, child: Instance, type: AttachType) { delete child.previousAttach } +const REACT_RESERVED_PROPS = ['children', 'key', 'ref'] + // This function prepares a set of changes to be applied to the instance export function diffProps( instance: Instance, - { children: cN, key: kN, ref: rN, ...props }: InstanceProps, - { children: cP, key: kP, ref: rP, ...previous }: InstanceProps = {}, + props: InstanceProps, + previous: InstanceProps = {}, remove = false, ): DiffSet { - const entries = Object.entries(props) - const changes: [key: string, value: unknown, isEvent: boolean, keys: string[]][] = [] + // Filter react reserved keys + const entries: [string, any][] = [] + for (const key in props) { + if (REACT_RESERVED_PROPS.includes(key)) continue + else entries.push([key, props[key]]) + } // Catch removed props, prepend them so they can be reset or removed if (remove) { - const previousKeys = Object.keys(previous) - for (let i = 0; i < previousKeys.length; i++) { - if (!props.hasOwnProperty(previousKeys[i])) entries.unshift([previousKeys[i], DEFAULT + 'remove']) + for (const key in previous) { + if (REACT_RESERVED_PROPS.includes(key)) continue + else if (!props.hasOwnProperty(key)) entries.unshift([key, DEFAULT + 'remove']) } } + const changes: [key: string, value: unknown, isEvent: boolean, keys: string[]][] = [] + entries.forEach(([key, value]) => { // Bail out on primitive object if (instance.type === 'primitive' && key === 'object') return From 8334dadb3ba5c4b542e3c880990e0476cc7897c9 Mon Sep 17 00:00:00 2001 From: Cody Bennett <23324155+CodyJasonBennett@users.noreply.github.com> Date: Thu, 21 Jul 2022 15:35:38 -0500 Subject: [PATCH 07/28] refactor: cleanup prop diffing --- packages/fiber/src/core/renderer.ts | 20 +-- packages/fiber/src/core/utils.ts | 212 +++++++++++++--------------- 2 files changed, 110 insertions(+), 122 deletions(-) diff --git a/packages/fiber/src/core/renderer.ts b/packages/fiber/src/core/renderer.ts index 0ae993e2ab..05352b3ce7 100644 --- a/packages/fiber/src/core/renderer.ts +++ b/packages/fiber/src/core/renderer.ts @@ -3,7 +3,7 @@ import { UseBoundStore } from 'zustand' import Reconciler from 'react-reconciler' import { unstable_IdlePriority as idlePriority, unstable_scheduleCallback as scheduleCallback } from 'scheduler' import { DefaultEventPriority } from 'react-reconciler/constants' -import { is, diffProps, DiffSet, applyProps, invalidateInstance, attach, detach } from './utils' +import { is, diffProps, applyProps, invalidateInstance, attach, detach } from './utils' import { RootState } from './store' import { EventHandlers, removeInteractivity } from './events' @@ -46,7 +46,7 @@ interface HostConfig { hydratableInstance: Instance publicInstance: any hostContext: never - updatePayload: null | [true] | [false, DiffSet] + updatePayload: null | [true] | [false, InstanceProps] childSet: never timeoutHandle: number | undefined noTimeout: -1 @@ -116,7 +116,7 @@ function createRenderer(_roots: Map, _getEventPriority?: const childIndex = parent.children.indexOf(child) if (childIndex !== -1) parent.children.splice(childIndex, 1) - if (child.props.attach) detach(parent, child, child.props.attach) + if (child.props.attach) detach(parent, child) else if (child.object.isObject3D && parent.object.isObject3D) { parent.object.remove(child.object) removeInteractivity(child.root, child.object as unknown as THREE.Object3D) @@ -178,13 +178,13 @@ function createRenderer(_roots: Map, _getEventPriority?: // Append children for (const child of instance.children) { - if (child.props.attach) attach(instance, child, child.props.attach) + if (child.props.attach) attach(instance, child) else if (child.object.isObject3D) instance.object.add(child.object) } // Append to container if (!instance.parent.parent) { - if (instance.props.attach) attach(instance.parent, instance, instance.props.attach) + if (instance.props.attach) attach(instance.parent, instance) else if (instance.object.isObject3D) instance.parent.object.add(instance.object) } @@ -218,7 +218,7 @@ function createRenderer(_roots: Map, _getEventPriority?: // Append to scene-graph if (parent.parent) { - if (newInstance.props.attach) attach(parent, newInstance, newInstance.props.attach) + if (newInstance.props.attach) attach(parent, newInstance) else if (newInstance.object.isObject3D) parent.object.add(newInstance.object) } @@ -227,8 +227,8 @@ function createRenderer(_roots: Map, _getEventPriority?: for (const child of instance.children) { appendChild(newInstance, child) if (child.props.attach) { - detach(instance, child, child.props.attach) - attach(newInstance, child, child.props.attach) + detach(instance, child) + attach(newInstance, child) } } instance.children = [] @@ -287,8 +287,8 @@ function createRenderer(_roots: Map, _getEventPriority?: if (newProps.args?.some((value, index) => value !== oldProps.args?.[index])) return [true] // Create a diff-set, flag if there are any changes - const diff = diffProps(instance, newProps, oldProps, true) - if (diff.changes.length) return [false, diff] + const changedProps = diffProps(instance, newProps, oldProps, true) + if (Object.keys(changedProps).length) return [false, changedProps] // Otherwise do not touch the instance return null diff --git a/packages/fiber/src/core/utils.ts b/packages/fiber/src/core/utils.ts index 78d0848808..3282d3175b 100644 --- a/packages/fiber/src/core/utils.ts +++ b/packages/fiber/src/core/utils.ts @@ -46,13 +46,6 @@ export class ErrorBoundary extends React.Component< } } -export const DEFAULT = '__default' - -export type DiffSet = { - changes: [key: string, value: unknown, isEvent: boolean, keys: string[]][] -} - -export const isDiffSet = (def: any): def is DiffSet => def && !!(def as DiffSet).changes export type ClassConstructor = { new (): void } export type ObjectMap = { @@ -133,126 +126,132 @@ export function dispose void; type?: string; [key } } -function resolve(instance: any, key: string) { - let target = instance - if (key.includes('-')) { - const entries = key.split('-') - const last = entries.pop() as string - target = entries.reduce((acc, key) => acc[key], instance) - return { target, key: last } - } else return { target, key } +function resolve(root: any, key: string) { + let target = root[key] + if (!key.includes('-')) return { root, key, target } + + // Resolve pierced target + const chain = key.split('-') + target = chain.reduce((acc, key) => acc[key], root) + key = chain.pop()! + + // Switch root if atomic + if (!target?.set) root = chain.reduce((acc, key) => acc[key], root) + + return { root, key, target } } // Checks if a dash-cased string ends with an integer const INDEX_REGEX = /-\d+$/ -export function attach(parent: Instance, child: Instance, type: AttachType) { - if (is.str(type)) { +export function attach(parent: Instance, child: Instance) { + if (is.str(child.props.attach)) { // If attaching into an array (foo-0), create one - if (INDEX_REGEX.test(type)) { - const root = type.replace(INDEX_REGEX, '') - const { target, key } = resolve(parent.object, root) - if (!Array.isArray(target[key])) target[key] = [] + if (INDEX_REGEX.test(child.props.attach)) { + const index = child.props.attach.replace(INDEX_REGEX, '') + const { root, key } = resolve(parent.object, index) + if (!Array.isArray(root[key])) root[key] = [] } - const { target, key } = resolve(parent.object, type) - child.previousAttach = target[key] - target[key] = child.object - } else child.previousAttach = type(parent.object, child.object) + const { root, key } = resolve(parent.object, child.props.attach) + child.previousAttach = root[key] + root[key] = child.object + } else if (is.fun(child.props.attach)) { + child.previousAttach = child.props.attach(parent.object, child.object) + } } -export function detach(parent: Instance, child: Instance, type: AttachType) { - if (is.str(type)) { - const { target, key } = resolve(parent.object, type) +export function detach(parent: Instance, child: Instance) { + if (is.str(child.props.attach)) { + const { root, key } = resolve(parent.object, child.props.attach) const previous = child.previousAttach // When the previous value was undefined, it means the value was never set to begin with - if (previous === undefined) delete target[key] + if (previous === undefined) delete root[key] // Otherwise set the previous value - else target[key] = previous - } else child.previousAttach?.(parent.object, child.object) + else root[key] = previous + } else { + child.previousAttach?.(parent.object, child.object) + } + delete child.previousAttach } +const DEFAULT = '__default' const REACT_RESERVED_PROPS = ['children', 'key', 'ref'] +const INSTANCE_RESERVED_PROPS = ['args', 'object', 'dispose', 'attach'] // This function prepares a set of changes to be applied to the instance export function diffProps( instance: Instance, - props: InstanceProps, - previous: InstanceProps = {}, + newProps: InstanceProps, + oldProps: InstanceProps, remove = false, -): DiffSet { - // Filter react reserved keys - const entries: [string, any][] = [] - for (const key in props) { - if (REACT_RESERVED_PROPS.includes(key)) continue - else entries.push([key, props[key]]) +): InstanceProps { + const changedProps: InstanceProps = {} + + // Sort through props + for (const key in newProps) { + // Skip reserved keys + if (REACT_RESERVED_PROPS.includes(key as typeof REACT_RESERVED_PROPS[number])) continue + // Skip primitives + if (instance.type === 'primitive' && key === 'object') continue + // Skip if props match + if (is.equ(newProps[key], oldProps[key])) continue + + // Props changed, add them + changedProps[key] = newProps[key] } // Catch removed props, prepend them so they can be reset or removed if (remove) { - for (const key in previous) { + for (const key in oldProps) { if (REACT_RESERVED_PROPS.includes(key)) continue - else if (!props.hasOwnProperty(key)) entries.unshift([key, DEFAULT + 'remove']) + else if (!newProps.hasOwnProperty(key)) changedProps[key] = DEFAULT + 'remove' } } - const changes: [key: string, value: unknown, isEvent: boolean, keys: string[]][] = [] - - entries.forEach(([key, value]) => { - // Bail out on primitive object - if (instance.type === 'primitive' && key === 'object') return - // When props match bail out - if (is.equ(value, previous[key])) return - // Collect handlers and bail out - if (/^on(Pointer|Click|DoubleClick|ContextMenu|Wheel)/.test(key)) return changes.push([key, value, true, []]) - // Split dashed props - let entries: string[] = [] - if (key.includes('-')) entries = key.split('-') - changes.push([key, value, false, entries]) - }) - - return { changes } + return changedProps } // This function applies a set of changes to the instance -export function applyProps(object: any, data: InstanceProps | DiffSet) { - // Filter equals, events and reserved props - const instance = (object.__r3f ?? {}) as Instance - const root = instance.root - const rootState = root?.getState?.() ?? {} - const { changes } = isDiffSet(data) ? data : diffProps(instance, data) - const prevHandlers = instance.eventCount - - changes.forEach(([key, value, isEvent, keys]) => { - let target = object - let targetProp = target[key] - - // Revolve dashed props - if (keys.length) { - targetProp = keys.reduce((acc, key) => acc[key], object) - // If the target is atomic, it forces us to switch the root - if (!(targetProp && targetProp.set)) { - const [name, ...reverseEntries] = keys.reverse() - target = reverseEntries.reverse().reduce((acc, key) => acc[key], object) - key = name - } +export function applyProps(object: any, newProps: any, oldProps?: any) { + const instance = object.__r3f as Instance | undefined + const rootState = instance?.root.getState() + const prevHandlers = instance?.eventCount + + for (const prop in newProps) { + let value = newProps[prop] + + // Don't mutate reserved keys + if (REACT_RESERVED_PROPS.includes(prop as typeof REACT_RESERVED_PROPS[number])) continue + if (INSTANCE_RESERVED_PROPS.includes(prop as typeof INSTANCE_RESERVED_PROPS[number])) continue + + // Don't mutate unchanged keys + if (newProps[prop] === oldProps?.[prop]) continue + + // Deal with pointer events ... + if (instance && /^on(Pointer|Click|DoubleClick|ContextMenu|Wheel)/.test(prop)) { + if (value) instance.handlers[prop as keyof EventHandlers] = value as any + else delete instance.handlers[prop as keyof EventHandlers] + instance.eventCount = Object.keys(instance.handlers).length } + const { root, key, target } = resolve(object, prop) + // https://github.com/mrdoob/three.js/issues/21209 // HMR/fast-refresh relies on the ability to cancel out props, but threejs // has no means to do this. Hence we curate a small collection of value-classes // with their respective constructor/set arguments // For removed props, try to set default values, if possible if (value === DEFAULT + 'remove') { - if (targetProp && targetProp.constructor) { + if (target && target.constructor) { // use the prop constructor to find the default it should be - value = new targetProp.constructor(...(instance.props.args ?? [])) - } else if (target.constructor) { + value = new target.constructor(...(instance?.props.args ?? [])) + } else if (root.constructor) { // create a blank slate of the instance and copy the particular parameter. // @ts-ignore - const defaultClassCall = new target.constructor(...(target.__r3f?.props.args ?? [])) - value = defaultClassCall[targetProp] + const defaultClassCall = new root.constructor(...(root.__r3f?.props.args ?? [])) + value = defaultClassCall[target] // destroy the instance if (defaultClassCall.dispose) defaultClassCall.dispose() } else { @@ -261,58 +260,45 @@ export function applyProps(object: any, data: InstanceProps | DiffSet) { } } - // Deal with pointer events ... - if (isEvent) { - if (value) instance.handlers[key as keyof EventHandlers] = value as any - else delete instance.handlers[key as keyof EventHandlers] - instance.eventCount = Object.keys(instance.handlers).length - } // Special treatment for objects with support for set/copy, and layers - else if (targetProp && targetProp.set && (targetProp.copy || targetProp instanceof THREE.Layers)) { + if (target && target.set && (target.copy || target instanceof THREE.Layers)) { // If value is an array if (Array.isArray(value)) { - if (targetProp.fromArray) targetProp.fromArray(value) - else targetProp.set(...value) + if (target.fromArray) target.fromArray(value) + else target.set(...value) } // Test again target.copy(class) next ... else if ( - targetProp.copy && + target.copy && value && (value as ClassConstructor).constructor && - targetProp.constructor.name === (value as ClassConstructor).constructor.name + target.constructor.name === (value as ClassConstructor).constructor.name ) { - targetProp.copy(value) + target.copy(value) } // If nothing else fits, just set the single value, ignore undefined // https://github.com/pmndrs/react-three-fiber/issues/274 else if (value !== undefined) { - const isColor = targetProp instanceof THREE.Color + const isColor = target instanceof THREE.Color // Allow setting array scalars - if (!isColor && targetProp.setScalar) targetProp.setScalar(value) + if (!isColor && target.setScalar) target.setScalar(value) // Layers have no copy function, we must therefore copy the mask property - else if (targetProp instanceof THREE.Layers && value instanceof THREE.Layers) targetProp.mask = value.mask + else if (target instanceof THREE.Layers && value instanceof THREE.Layers) target.mask = value.mask // Otherwise just set ... - else targetProp.set(value) - // For versions of three which don't support THREE.ColorManagement, - // Auto-convert sRGB colors - // https://github.com/pmndrs/react-three-fiber/issues/344 - const supportsColorManagement = 'ColorManagement' in THREE - if (!supportsColorManagement && !rootState.linear && isColor) targetProp.convertSRGBToLinear() + else target.set(value) } // Else, just overwrite the value } else { - target[key] = value + root[key] = value // Auto-convert sRGB textures, for now ... // https://github.com/pmndrs/react-three-fiber/issues/344 - if (!rootState.linear && target[key] instanceof THREE.Texture) { - target[key].encoding = THREE.sRGBEncoding + if (!rootState?.linear && root[key] instanceof THREE.Texture) { + root[key].encoding = THREE.sRGBEncoding } } + } - invalidateInstance(instance) - }) - - if (instance.parent && rootState.internal && instance.object.raycast && prevHandlers !== instance.eventCount) { + if (instance?.parent && rootState?.internal && instance?.object.raycast && prevHandlers !== instance?.eventCount) { // Pre-emptively remove the instance from the interaction manager const index = rootState.internal.interaction.indexOf(instance.object as unknown as THREE.Object3D) if (index > -1) rootState.internal.interaction.splice(index, 1) @@ -320,7 +306,9 @@ export function applyProps(object: any, data: InstanceProps | DiffSet) { if (instance.eventCount) rootState.internal.interaction.push(instance.object as unknown as THREE.Object3D) } - return instance + if (instance) invalidateInstance(instance) + + return object } export function invalidateInstance(instance: Instance) { From 4f3da21c3a4b52e36d5f2ce7fcdad1298d2e5f2c Mon Sep 17 00:00:00 2001 From: Cody Bennett <23324155+CodyJasonBennett@users.noreply.github.com> Date: Thu, 21 Jul 2022 17:52:57 -0500 Subject: [PATCH 08/28] refactor(RTTR): use new instance structure, cleanup internal types --- .../test-renderer/src/createTestInstance.ts | 46 ++++++++----------- packages/test-renderer/src/helpers/graph.ts | 6 +-- packages/test-renderer/src/helpers/tree.ts | 21 +++------ packages/test-renderer/src/index.tsx | 32 ++++++------- packages/test-renderer/src/types/internal.ts | 23 +++------- 5 files changed, 49 insertions(+), 79 deletions(-) diff --git a/packages/test-renderer/src/createTestInstance.ts b/packages/test-renderer/src/createTestInstance.ts index 69a4b3b28c..b0ed075206 100644 --- a/packages/test-renderer/src/createTestInstance.ts +++ b/packages/test-renderer/src/createTestInstance.ts @@ -1,30 +1,34 @@ import { Object3D } from 'three' -import type { MockInstance, MockScene, Obj, TestInstanceChildOpts } from './types/internal' +import type { MockInstance, Obj, TestInstanceChildOpts } from './types/internal' import { expectOne, matchProps, findAll } from './helpers/testInstance' -export class ReactThreeTestInstance { - _fiber: MockInstance +export class ReactThreeTestInstance { + _fiber: MockInstance - constructor(fiber: MockInstance | MockScene) { - this._fiber = fiber as MockInstance + constructor(fiber: MockInstance) { + this._fiber = fiber } - public get instance(): Object3D { - return this._fiber as unknown as TInstance + public get fiber(): MockInstance { + return this._fiber + } + + public get instance(): TObject { + return this._fiber.object } public get type(): string { - return this._fiber.type + return this._fiber.object.type } public get props(): Obj { - return this._fiber.__r3f.memoizedProps + return this._fiber.props } public get parent(): ReactThreeTestInstance | null { - const parent = this._fiber.__r3f.parent + const parent = this._fiber.parent if (parent !== null) { return wrapFiber(parent) } @@ -42,20 +46,10 @@ export class ReactThreeTestInstance { private getChildren = ( fiber: MockInstance, opts: TestInstanceChildOpts = { exhaustive: false }, - ): ReactThreeTestInstance[] => { - if (opts.exhaustive) { - /** - * this will return objects like - * color or effects etc. - */ - return [ - ...(fiber.children || []).map((fib) => wrapFiber(fib as MockInstance)), - ...fiber.__r3f.objects.map((fib) => wrapFiber(fib as MockInstance)), - ] - } else { - return (fiber.children || []).map((fib) => wrapFiber(fib as MockInstance)) - } - } + ): ReactThreeTestInstance[] => + fiber.children + .filter((child) => !child.props.attach || opts.exhaustive) + .map((fib) => wrapFiber(fib as MockInstance)) public find = (decider: (node: ReactThreeTestInstance) => boolean): ReactThreeTestInstance => expectOne(findAll(this, decider), `matching custom checker: ${decider.toString()}`) @@ -79,8 +73,8 @@ export class ReactThreeTestInstance { findAll(this, (node: ReactThreeTestInstance) => Boolean(node.props && matchProps(node.props, props))) } -const fiberToWrapper = new WeakMap() -export const wrapFiber = (fiber: MockInstance | MockScene): ReactThreeTestInstance => { +const fiberToWrapper = new WeakMap() +export const wrapFiber = (fiber: MockInstance): ReactThreeTestInstance => { let wrapper = fiberToWrapper.get(fiber) if (wrapper === undefined) { wrapper = new ReactThreeTestInstance(fiber) diff --git a/packages/test-renderer/src/helpers/graph.ts b/packages/test-renderer/src/helpers/graph.ts index 0266dbaa11..7ab0da38cf 100644 --- a/packages/test-renderer/src/helpers/graph.ts +++ b/packages/test-renderer/src/helpers/graph.ts @@ -1,4 +1,4 @@ -import type { MockScene, MockSceneChild } from '../types/internal' +import type { MockInstance } from '../types/internal' import type { SceneGraphItem } from '../types/public' const graphObjectFactory = ( @@ -11,5 +11,5 @@ const graphObjectFactory = ( children, }) -export const toGraph = (object: MockScene | MockSceneChild): SceneGraphItem[] => - object.children.map((child) => graphObjectFactory(child.type, child.name || '', toGraph(child))) +export const toGraph = (object: MockInstance): SceneGraphItem[] => + object.children.map((child) => graphObjectFactory(child.object.type, child.object.name ?? '', toGraph(child))) diff --git a/packages/test-renderer/src/helpers/tree.ts b/packages/test-renderer/src/helpers/tree.ts index d69f20f2f8..45ca6890a6 100644 --- a/packages/test-renderer/src/helpers/tree.ts +++ b/packages/test-renderer/src/helpers/tree.ts @@ -1,5 +1,5 @@ import type { TreeNode, Tree } from '../types/public' -import type { MockSceneChild, MockScene } from '../types/internal' +import type { MockInstance } from '../types/internal' import { lowerCaseFirstLetter } from './strings' const treeObjectFactory = ( @@ -12,20 +12,13 @@ const treeObjectFactory = ( children, }) -const toTreeBranch = (obj: MockSceneChild[]): TreeNode[] => - obj.map((child) => { +const toTreeBranch = (children: MockInstance[]): TreeNode[] => + children.map((child) => { return treeObjectFactory( - lowerCaseFirstLetter(child.type || child.constructor.name), - { ...child.__r3f.memoizedProps }, - toTreeBranch([...(child.children || []), ...child.__r3f.objects]), + lowerCaseFirstLetter(child.object.type || child.object.constructor.name), + child.props, + toTreeBranch(child.children), ) }) -export const toTree = (root: MockScene): Tree => - root.children.map((obj) => - treeObjectFactory( - lowerCaseFirstLetter(obj.type), - { ...obj.__r3f.memoizedProps }, - toTreeBranch([...(obj.children as MockSceneChild[]), ...(obj.__r3f.objects as MockSceneChild[])]), - ), - ) +export const toTree = (root: MockInstance): Tree => toTreeBranch(root.children) diff --git a/packages/test-renderer/src/index.tsx b/packages/test-renderer/src/index.tsx index d71bde7ff4..35a3676aab 100644 --- a/packages/test-renderer/src/index.tsx +++ b/packages/test-renderer/src/index.tsx @@ -11,7 +11,7 @@ import { createCanvas } from './createTestCanvas' import { createWebGLContext } from './createWebGLContext' import { createEventFirer } from './fireEvent' -import type { MockScene } from './types/internal' +import type { MockInstance } from './types/internal' import type { CreateOptions, Renderer, Act } from './types/public' import { wrapFiber } from './createTestInstance' @@ -34,28 +34,22 @@ const create = async (element: React.ReactNode, options?: Partial }, }) - const _fiber = canvas + const _root = createRoot(canvas).configure({ frameloop: 'never', ...options, events: undefined }) + const _store = mockRoots.get(canvas)!.store - const _root = createRoot(_fiber).configure({ frameloop: 'never', ...options, events: undefined }) - - let scene: MockScene = null! - - await act(async () => { - scene = _root.render(element).getState().scene as unknown as MockScene - }) - - const _store = mockRoots.get(_fiber)!.store + await act(async () => _root.render(element)) + const scene = (_store.getState().scene as any).__r3f as MockInstance return { scene: wrapFiber(scene), - unmount: async () => { + async unmount() { await act(async () => { _root.unmount() }) }, - getInstance: () => { + getInstance() { // this is our root - const fiber = mockRoots.get(_fiber)?.fiber + const fiber = mockRoots.get(canvas)?.fiber const current = fiber?.current.child.child if (current) { const root = { @@ -78,8 +72,8 @@ const create = async (element: React.ReactNode, options?: Partial return null } }, - update: async (newElement: React.ReactNode) => { - const fiber = mockRoots.get(_fiber)?.fiber + async update(newElement: React.ReactNode) { + const fiber = mockRoots.get(canvas)?.fiber if (fiber) { await act(async () => { reconciler.updateContainer(newElement, fiber, null, () => null) @@ -87,14 +81,14 @@ const create = async (element: React.ReactNode, options?: Partial } return }, - toTree: () => { + toTree() { return toTree(scene) }, - toGraph: () => { + toGraph() { return toGraph(scene) }, fireEvent: createEventFirer(act, _store), - advanceFrames: async (frames: number, delta: number | number[] = 1) => { + async advanceFrames(frames: number, delta: number | number[] = 1) { const state = _store.getState() const storeSubscribers = state.internal.subscribers diff --git a/packages/test-renderer/src/types/internal.ts b/packages/test-renderer/src/types/internal.ts index d4b54d936b..debaa16d52 100644 --- a/packages/test-renderer/src/types/internal.ts +++ b/packages/test-renderer/src/types/internal.ts @@ -1,24 +1,13 @@ -import * as THREE from 'three' import { UseBoundStore } from 'zustand' - -import type { BaseInstance, LocalState, RootState } from '@react-three/fiber' +import type { Instance, RootState } from '@react-three/fiber' export type MockUseStoreState = UseBoundStore -export interface MockInstance extends Omit { - __r3f: Omit & { - root: MockUseStoreState - objects: MockSceneChild[] - parent: MockInstance - } -} - -export interface MockSceneChild extends Omit { - children: MockSceneChild[] -} - -export interface MockScene extends Omit, Pick { - children: MockSceneChild[] +export interface MockInstance extends Omit { + root: MockUseStoreState + parent: MockInstance + children: MockInstance[] + object: O } export type CreateCanvasParameters = { From 3aea5b2035135b021339f345b1aa5494422d279f Mon Sep 17 00:00:00 2001 From: Cody Bennett <23324155+CodyJasonBennett@users.noreply.github.com> Date: Fri, 22 Jul 2022 05:27:06 -0500 Subject: [PATCH 09/28] fix: safely add object3ds on commit --- packages/fiber/src/core/renderer.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/fiber/src/core/renderer.ts b/packages/fiber/src/core/renderer.ts index 05352b3ce7..375451d6d6 100644 --- a/packages/fiber/src/core/renderer.ts +++ b/packages/fiber/src/core/renderer.ts @@ -178,14 +178,20 @@ function createRenderer(_roots: Map, _getEventPriority?: // Append children for (const child of instance.children) { - if (child.props.attach) attach(instance, child) - else if (child.object.isObject3D) instance.object.add(child.object) + if (child.props.attach) { + attach(instance, child) + } else if (child.object.isObject3D && instance.object.isObject3D) { + instance.object.add(child.object) + } } // Append to container if (!instance.parent.parent) { - if (instance.props.attach) attach(instance.parent, instance) - else if (instance.object.isObject3D) instance.parent.object.add(instance.object) + if (instance.props.attach) { + attach(instance.parent, instance) + } else if (instance.object.isObject3D && instance.parent.object.isObject3D) { + instance.parent.object.add(instance.object) + } } // Apply props to object From ae3c219bdcaca7f939ff8981a0a461942b192159 Mon Sep 17 00:00:00 2001 From: Cody Bennett <23324155+CodyJasonBennett@users.noreply.github.com> Date: Fri, 22 Jul 2022 06:56:25 -0500 Subject: [PATCH 10/28] fix(renderer): update instance on commitupdate, cleanup prop filter --- packages/fiber/src/core/renderer.ts | 17 +++++++----- packages/fiber/src/core/utils.ts | 40 ++++++++++++++--------------- 2 files changed, 29 insertions(+), 28 deletions(-) diff --git a/packages/fiber/src/core/renderer.ts b/packages/fiber/src/core/renderer.ts index 375451d6d6..d426c228e7 100644 --- a/packages/fiber/src/core/renderer.ts +++ b/packages/fiber/src/core/renderer.ts @@ -116,8 +116,9 @@ function createRenderer(_roots: Map, _getEventPriority?: const childIndex = parent.children.indexOf(child) if (childIndex !== -1) parent.children.splice(childIndex, 1) - if (child.props.attach) detach(parent, child) - else if (child.object.isObject3D && parent.object.isObject3D) { + if (child.props.attach) { + detach(parent, child) + } else if (child.object.isObject3D && parent.object.isObject3D) { parent.object.remove(child.object) removeInteractivity(child.root, child.object as unknown as THREE.Object3D) } @@ -286,14 +287,14 @@ function createRenderer(_roots: Map, _getEventPriority?: insertInContainerBefore: () => {}, getRootHostContext: () => null, getChildHostContext: (parentHostContext) => parentHostContext, - prepareUpdate(instance, type, oldProps, newProps) { + prepareUpdate(instance, _type, oldProps, newProps) { // Reconstruct primitives if object prop changes - if (type === 'primitive' && oldProps.object !== newProps.object) return [true] + if (instance.type === 'primitive' && oldProps.object !== newProps.object) return [true] // Reconstruct elements if args change if (newProps.args?.some((value, index) => value !== oldProps.args?.[index])) return [true] // Create a diff-set, flag if there are any changes - const changedProps = diffProps(instance, newProps, oldProps, true) + const changedProps = diffProps(newProps, oldProps, true) if (Object.keys(changedProps).length) return [false, changedProps] // Otherwise do not touch the instance @@ -303,9 +304,11 @@ function createRenderer(_roots: Map, _getEventPriority?: const [reconstruct, changedProps] = diff! // Reconstruct when args or true, commitMount: commitInstance, diff --git a/packages/fiber/src/core/utils.ts b/packages/fiber/src/core/utils.ts index 3282d3175b..a54d16dd40 100644 --- a/packages/fiber/src/core/utils.ts +++ b/packages/fiber/src/core/utils.ts @@ -1,7 +1,7 @@ import * as THREE from 'three' import * as React from 'react' import { EventHandlers } from './events' -import { AttachType, Instance, InstanceProps } from './renderer' +import { Instance, InstanceProps } from './renderer' import { Dpr, RootState, Size } from './store' export type Camera = THREE.OrthographicCamera | THREE.PerspectiveCamera @@ -177,24 +177,26 @@ export function detach(parent: Instance, child: Instance) { } const DEFAULT = '__default' -const REACT_RESERVED_PROPS = ['children', 'key', 'ref'] -const INSTANCE_RESERVED_PROPS = ['args', 'object', 'dispose', 'attach'] +const RESERVED_PROPS = [ + // React internal props + 'children', + 'key', + 'ref', + // Instance props + 'args', + 'dispose', + 'attach', + // 'object', -- internal to primitives +] // This function prepares a set of changes to be applied to the instance -export function diffProps( - instance: Instance, - newProps: InstanceProps, - oldProps: InstanceProps, - remove = false, -): InstanceProps { +export function diffProps(newProps: InstanceProps, oldProps: InstanceProps, remove = false): InstanceProps { const changedProps: InstanceProps = {} // Sort through props for (const key in newProps) { // Skip reserved keys - if (REACT_RESERVED_PROPS.includes(key as typeof REACT_RESERVED_PROPS[number])) continue - // Skip primitives - if (instance.type === 'primitive' && key === 'object') continue + if (RESERVED_PROPS.includes(key)) continue // Skip if props match if (is.equ(newProps[key], oldProps[key])) continue @@ -205,7 +207,7 @@ export function diffProps( // Catch removed props, prepend them so they can be reset or removed if (remove) { for (const key in oldProps) { - if (REACT_RESERVED_PROPS.includes(key)) continue + if (RESERVED_PROPS.includes(key)) continue else if (!newProps.hasOwnProperty(key)) changedProps[key] = DEFAULT + 'remove' } } @@ -214,20 +216,16 @@ export function diffProps( } // This function applies a set of changes to the instance -export function applyProps(object: any, newProps: any, oldProps?: any) { +export function applyProps(object: any, props: any) { const instance = object.__r3f as Instance | undefined const rootState = instance?.root.getState() const prevHandlers = instance?.eventCount - for (const prop in newProps) { - let value = newProps[prop] + for (const prop in props) { + let value = props[prop] // Don't mutate reserved keys - if (REACT_RESERVED_PROPS.includes(prop as typeof REACT_RESERVED_PROPS[number])) continue - if (INSTANCE_RESERVED_PROPS.includes(prop as typeof INSTANCE_RESERVED_PROPS[number])) continue - - // Don't mutate unchanged keys - if (newProps[prop] === oldProps?.[prop]) continue + if (RESERVED_PROPS.includes(prop)) continue // Deal with pointer events ... if (instance && /^on(Pointer|Click|DoubleClick|ContextMenu|Wheel)/.test(prop)) { From 67ecde94f23a27be6df2d65ac2513898b8c2a2d5 Mon Sep 17 00:00:00 2001 From: Cody Bennett <23324155+CodyJasonBennett@users.noreply.github.com> Date: Fri, 22 Jul 2022 07:55:53 -0500 Subject: [PATCH 11/28] fix: safely check container parents, de-dup interaction on commit --- packages/fiber/src/core/renderer.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/fiber/src/core/renderer.ts b/packages/fiber/src/core/renderer.ts index d426c228e7..d6401f5ec4 100644 --- a/packages/fiber/src/core/renderer.ts +++ b/packages/fiber/src/core/renderer.ts @@ -190,7 +190,7 @@ function createRenderer(_roots: Map, _getEventPriority?: if (!instance.parent.parent) { if (instance.props.attach) { attach(instance.parent, instance) - } else if (instance.object.isObject3D && instance.parent.object.isObject3D) { + } else if (instance.object.isObject3D && instance.parent.object?.isObject3D) { instance.parent.object.add(instance.object) } } @@ -198,11 +198,6 @@ function createRenderer(_roots: Map, _getEventPriority?: // Apply props to object applyProps(instance.object, instance.props) - // Add interactivity - if (instance.object.raycast && instance.handlers && instance.eventCount) { - instance.root.getState().internal.interaction.push(instance.object as unknown as THREE.Object3D) - } - invalidateInstance(instance) } From 7dec56652efe9564c19464202dc23cc645d6b9c2 Mon Sep 17 00:00:00 2001 From: Cody Bennett <23324155+CodyJasonBennett@users.noreply.github.com> Date: Fri, 22 Jul 2022 07:58:54 -0500 Subject: [PATCH 12/28] fix(RTTR): correctly traverse root react element --- packages/test-renderer/src/index.tsx | 36 +++++++++++++--------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/packages/test-renderer/src/index.tsx b/packages/test-renderer/src/index.tsx index 35a3676aab..4d5917d72a 100644 --- a/packages/test-renderer/src/index.tsx +++ b/packages/test-renderer/src/index.tsx @@ -50,27 +50,23 @@ const create = async (element: React.ReactNode, options?: Partial getInstance() { // this is our root const fiber = mockRoots.get(canvas)?.fiber - const current = fiber?.current.child.child - if (current) { - const root = { - /** - * we wrap our child in a Provider component - * and context.Provider, so do a little - * artificial dive to get round this and - * pass context.Provider as if it was the - * actual react root - */ - current, - } + const current = fiber?.current?.child?.child?.child?.child + if (!current) return null - /** - * so this actually returns the instance - * the user has passed through as a Fiber - */ - return reconciler.getPublicRootInstance(root) - } else { - return null - } + /** + * we wrap our child in a Provider component + * and context.Provider, so do a little + * artificial dive to get round this and + * pass context.Provider as if it was the + * actual react root + */ + const root = { current } + + /** + * so this actually returns the instance + * the user has passed through as a Fiber + */ + return reconciler.getPublicRootInstance(root) }, async update(newElement: React.ReactNode) { const fiber = mockRoots.get(canvas)?.fiber From 58d656fa6d593853bb69d0f36dca5980423c6b84 Mon Sep 17 00:00:00 2001 From: Cody Bennett <23324155+CodyJasonBennett@users.noreply.github.com> Date: Fri, 22 Jul 2022 07:59:11 -0500 Subject: [PATCH 13/28] chore(tests): update instance snapshot --- .../src/__tests__/RTTR.core.test.tsx | 70 +-- .../__snapshots__/RTTR.core.test.tsx.snap | 432 +++++++++++++++++- 2 files changed, 426 insertions(+), 76 deletions(-) diff --git a/packages/test-renderer/src/__tests__/RTTR.core.test.tsx b/packages/test-renderer/src/__tests__/RTTR.core.test.tsx index 52eb84a103..539e52a2cc 100644 --- a/packages/test-renderer/src/__tests__/RTTR.core.test.tsx +++ b/packages/test-renderer/src/__tests__/RTTR.core.test.tsx @@ -82,19 +82,7 @@ describe('ReactThreeTestRenderer Core', () => { const renderer = await ReactThreeTestRenderer.create() - expect(renderer.toGraph()).toEqual([ - { - type: 'Group', - name: '', - children: [ - { - type: 'Mesh', - name: '', - children: [], - }, - ], - }, - ]) + expect(renderer.toGraph()).toMatchSnapshot() }) it('renders some basics with an update', async () => { @@ -167,7 +155,7 @@ describe('ReactThreeTestRenderer Core', () => { }) it('exposes the instance', async () => { - class Mesh extends React.PureComponent { + class Instance extends React.PureComponent { state = { standardMat: false } handleStandard() { @@ -184,51 +172,17 @@ describe('ReactThreeTestRenderer Core', () => { } } - const renderer = await ReactThreeTestRenderer.create() - - expect(renderer.toTree()).toEqual([ - { - type: 'mesh', - props: { - args: [], - }, - children: [ - { type: 'boxGeometry', props: { args: [2, 2] }, children: [] }, - { - type: 'meshBasicMaterial', - props: { - args: [], - }, - children: [], - }, - ], - }, - ]) + const renderer = await ReactThreeTestRenderer.create() + + expect(renderer.toTree()).toMatchSnapshot() - const instance = renderer.getInstance() as Mesh + const instance = renderer.getInstance() as Instance await ReactThreeTestRenderer.act(async () => { instance.handleStandard() }) - expect(renderer.toTree()).toEqual([ - { - type: 'mesh', - props: { - args: [], - }, - children: [ - { type: 'boxGeometry', props: { args: [2, 2] }, children: [] }, - { - type: 'meshStandardMaterial', - props: { - args: [], - }, - children: [], - }, - ], - }, - ]) + expect(renderer.toTree()).toMatchSnapshot() }) it('updates children', async () => { @@ -316,15 +270,7 @@ describe('ReactThreeTestRenderer Core', () => { ) const renderer = await ReactThreeTestRenderer.create() - expect(renderer.toTree()).toEqual([ - { - type: 'group', - props: { - args: [], - }, - children: [], - }, - ]) + expect(renderer.toTree()).toMatchSnapshot() }) it('correctly builds a tree', async () => { diff --git a/packages/test-renderer/src/__tests__/__snapshots__/RTTR.core.test.tsx.snap b/packages/test-renderer/src/__tests__/__snapshots__/RTTR.core.test.tsx.snap index bc4b647a4b..7cfdf9056d 100644 --- a/packages/test-renderer/src/__tests__/__snapshots__/RTTR.core.test.tsx.snap +++ b/packages/test-renderer/src/__tests__/__snapshots__/RTTR.core.test.tsx.snap @@ -1,9 +1,53 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`ReactThreeTestRenderer Core can render a composite component & correctly build simple graph 1`] = ` +Array [ + Object { + "children": Array [ + Object { + "children": Array [], + "name": "", + "type": undefined, + }, + Object { + "children": Array [ + Object { + "children": Array [], + "name": "", + "type": "BoxGeometry", + }, + Object { + "children": Array [], + "name": "", + "type": "MeshBasicMaterial", + }, + ], + "name": "", + "type": "Mesh", + }, + ], + "name": "", + "type": "Group", + }, +] +`; + exports[`ReactThreeTestRenderer Core correctly builds a tree 1`] = ` Array [ Object { "children": Array [ + Object { + "children": Array [], + "props": Object { + "args": Array [ + 0, + 0, + 255, + ], + "attach": "background", + }, + "type": "color", + }, Object { "children": Array [ Object { @@ -32,6 +76,7 @@ Array [ -1, 1, ], + "attach": "attributes-position", "count": 6, "itemSize": 3, }, @@ -40,6 +85,34 @@ Array [ ], "props": Object { "args": Array [], + "attach": "geometry", + "children": , }, "type": "bufferGeometry", }, @@ -47,6 +120,7 @@ Array [ "children": Array [], "props": Object { "args": Array [], + "attach": "material", "color": "hotpink", }, "type": "meshBasicMaterial", @@ -54,30 +128,155 @@ Array [ ], "props": Object { "args": Array [], + "children": Array [ + + + , + , + ], }, "type": "mesh", }, + ], + "props": Object { + "args": Array [], + "children": Array [ + , + , + , + ], + "position": Array [ + 1, + 2, + 3, + ], + }, + "type": "group", + }, +] +`; + +exports[`ReactThreeTestRenderer Core exposes the instance 1`] = ` +Array [ + Object { + "children": Array [ Object { "children": Array [], "props": Object { "args": Array [ - 0, - 0, - 255, + 2, + 2, ], + "attach": "geometry", }, - "type": "color", + "type": "boxGeometry", + }, + Object { + "children": Array [], + "props": Object { + "args": Array [], + "attach": "material", + }, + "type": "meshBasicMaterial", }, ], "props": Object { "args": Array [], - "position": Array [ - 1, - 2, - 3, + "children": Array [ + , + , ], }, - "type": "group", + "type": "mesh", + }, +] +`; + +exports[`ReactThreeTestRenderer Core exposes the instance 2`] = ` +Array [ + Object { + "children": Array [ + Object { + "children": Array [], + "props": Object { + "args": Array [ + 2, + 2, + ], + "attach": "geometry", + }, + "type": "boxGeometry", + }, + Object { + "children": Array [], + "props": Object { + "args": Array [], + "attach": "material", + }, + "type": "meshStandardMaterial", + }, + ], + "props": Object { + "args": Array [], + "children": Array [ + , + , + ], + }, + "type": "mesh", }, ] `; @@ -94,12 +293,23 @@ Array [ 0, 0, ], + "attach": "background", }, "type": "color", }, ], "props": Object { "args": Array [], + "children": , }, "type": "group", }, @@ -113,12 +323,23 @@ Array [ 0, 255, ], + "attach": "background", }, "type": "color", }, ], "props": Object { "args": Array [], + "children": , }, "type": "group", }, @@ -132,10 +353,33 @@ Array [ 0, 0, ], + "attach": "background", }, "type": "color", }, ], + "props": Object { + "args": Array [], + "children": , + }, + "type": "group", + }, +] +`; + +exports[`ReactThreeTestRenderer Core toTree() handles nested Fragments 1`] = ` +Array [ + Object { + "children": Array [], "props": Object { "args": Array [], }, @@ -157,6 +401,7 @@ Array [ 2, 2, ], + "attach": "geometry", }, "type": "boxGeometry", }, @@ -164,12 +409,24 @@ Array [ "children": Array [], "props": Object { "args": Array [], + "attach": "material", }, "type": "meshBasicMaterial", }, ], "props": Object { "args": Array [], + "children": Array [ + , + , + ], "position-z": 12, }, "type": "mesh", @@ -183,6 +440,7 @@ Array [ 4, 4, ], + "attach": "geometry", }, "type": "boxGeometry", }, @@ -190,12 +448,24 @@ Array [ "children": Array [], "props": Object { "args": Array [], + "attach": "material", }, "type": "meshBasicMaterial", }, ], "props": Object { "args": Array [], + "children": Array [ + , + , + ], "position-y": 12, }, "type": "mesh", @@ -209,6 +479,7 @@ Array [ 6, 6, ], + "attach": "geometry", }, "type": "boxGeometry", }, @@ -216,12 +487,24 @@ Array [ "children": Array [], "props": Object { "args": Array [], + "attach": "material", }, "type": "meshBasicMaterial", }, ], "props": Object { "args": Array [], + "children": Array [ + , + , + ], "position-x": 12, }, "type": "mesh", @@ -229,6 +512,47 @@ Array [ ], "props": Object { "args": Array [], + "children": Array [ + + + + , + + + + , + + + + , + ], }, "type": "group", }, @@ -245,9 +569,10 @@ Array [ "children": Array [], "props": Object { "args": Array [ - 6, - 6, + 2, + 2, ], + "attach": "geometry", }, "type": "boxGeometry", }, @@ -255,13 +580,25 @@ Array [ "children": Array [], "props": Object { "args": Array [], + "attach": "material", }, "type": "meshBasicMaterial", }, ], "props": Object { "args": Array [], - "rotation-x": 1, + "children": Array [ + , + , + ], + "position-z": 12, }, "type": "mesh", }, @@ -274,6 +611,7 @@ Array [ 4, 4, ], + "attach": "geometry", }, "type": "boxGeometry", }, @@ -281,12 +619,24 @@ Array [ "children": Array [], "props": Object { "args": Array [], + "attach": "material", }, "type": "meshBasicMaterial", }, ], "props": Object { "args": Array [], + "children": Array [ + , + , + ], "position-y": 12, }, "type": "mesh", @@ -297,9 +647,10 @@ Array [ "children": Array [], "props": Object { "args": Array [ - 2, - 2, + 6, + 6, ], + "attach": "geometry", }, "type": "boxGeometry", }, @@ -307,12 +658,24 @@ Array [ "children": Array [], "props": Object { "args": Array [], + "attach": "material", }, "type": "meshBasicMaterial", }, ], "props": Object { "args": Array [], + "children": Array [ + , + , + ], "position-x": 12, }, "type": "mesh", @@ -320,6 +683,47 @@ Array [ ], "props": Object { "args": Array [], + "children": Array [ + + + + , + + + + , + + + + , + ], }, "type": "group", }, From 9ab0eb7f07750493d214705d91f81aa616fa415c Mon Sep 17 00:00:00 2001 From: Cody Bennett <23324155+CodyJasonBennett@users.noreply.github.com> Date: Fri, 22 Jul 2022 08:23:58 -0500 Subject: [PATCH 14/28] chore(RTTR): make sure fireEvent warns on invalid prop --- packages/test-renderer/src/__tests__/RTTR.events.test.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/test-renderer/src/__tests__/RTTR.events.test.tsx b/packages/test-renderer/src/__tests__/RTTR.events.test.tsx index 01b31b9bc6..c2254c799a 100644 --- a/packages/test-renderer/src/__tests__/RTTR.events.test.tsx +++ b/packages/test-renderer/src/__tests__/RTTR.events.test.tsx @@ -50,8 +50,13 @@ describe('ReactThreeTestRenderer Events', () => { const { scene, fireEvent } = await ReactThreeTestRenderer.create() - expect(async () => await fireEvent(scene.children[0], 'onPointerUp')).not.toThrow() + const warn = console.warn.bind(console) + console.warn = jest.fn() + expect(async () => await fireEvent(scene.children[0], 'onPointerUp')).not.toThrow() + expect(console.warn).toBeCalled() expect(handlePointerDown).not.toHaveBeenCalled() + + console.warn = warn }) }) From 0720009d08cbef45889c049acb2c2caf2735c46c Mon Sep 17 00:00:00 2001 From: Cody Bennett <23324155+CodyJasonBennett@users.noreply.github.com> Date: Fri, 22 Jul 2022 09:18:51 -0500 Subject: [PATCH 15/28] fix(renderer): safely append to container --- packages/fiber/src/core/renderer.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/fiber/src/core/renderer.ts b/packages/fiber/src/core/renderer.ts index d6401f5ec4..ef888ef600 100644 --- a/packages/fiber/src/core/renderer.ts +++ b/packages/fiber/src/core/renderer.ts @@ -187,10 +187,10 @@ function createRenderer(_roots: Map, _getEventPriority?: } // Append to container - if (!instance.parent.parent) { + if (!instance.parent.parent && instance.parent.object) { if (instance.props.attach) { attach(instance.parent, instance) - } else if (instance.object.isObject3D && instance.parent.object?.isObject3D) { + } else if (instance.object.isObject3D && instance.parent.object.isObject3D) { instance.parent.object.add(instance.object) } } From c112c8d20a598c6ea499fe5a23a81efb4d342799 Mon Sep 17 00:00:00 2001 From: Cody Bennett <23324155+CodyJasonBennett@users.noreply.github.com> Date: Fri, 22 Jul 2022 09:56:26 -0500 Subject: [PATCH 16/28] fix(renderer): loosen commit append to parents --- packages/fiber/src/core/renderer.ts | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/packages/fiber/src/core/renderer.ts b/packages/fiber/src/core/renderer.ts index ef888ef600..742b14e4c7 100644 --- a/packages/fiber/src/core/renderer.ts +++ b/packages/fiber/src/core/renderer.ts @@ -29,7 +29,6 @@ export interface Instance { children: Instance[] props: InstanceProps object: any | null - eventCount: number handlers: Partial attach?: AttachType @@ -43,8 +42,8 @@ interface HostConfig { instance: Instance textInstance: never suspenseInstance: Instance - hydratableInstance: Instance - publicInstance: any + hydratableInstance: never + publicInstance: Instance['object'] hostContext: never updatePayload: null | [true] | [false, InstanceProps] childSet: never @@ -66,7 +65,7 @@ function createRenderer(_roots: Map, _getEventPriority?: type: string, { args = [], object = null, ...props }: InstanceProps, root: UseBoundStore, - ): Instance { + ): HostConfig['instance'] { const name = `${type[0].toUpperCase()}${type.slice(1)}` const target = catalogue[name] @@ -75,7 +74,7 @@ function createRenderer(_roots: Map, _getEventPriority?: if (type === 'primitive' && !object) throw `Primitives without 'object' are invalid!` - const instance: Instance = { + const instance: HostConfig['instance'] = { root, type, parent: null, @@ -107,8 +106,14 @@ function createRenderer(_roots: Map, _getEventPriority?: parent.children.splice(parent.children.indexOf(beforeChild), 0, child) } - function removeRecursive(array: HostConfig['instance'][], parent: HostConfig['instance'], dispose: boolean = false) { - if (array) [...array].forEach((child) => removeChild(parent, child, dispose)) + function removeRecursive( + children: HostConfig['instance'][], + parent: HostConfig['instance'], + dispose: boolean = false, + ) { + for (const child of children) { + removeChild(parent, child, dispose) + } } function removeChild(parent: HostConfig['instance'], child: HostConfig['instance'], dispose?: boolean) { @@ -157,7 +162,7 @@ function createRenderer(_roots: Map, _getEventPriority?: if (dispose === undefined) invalidateInstance(child) } - function commitInstance(instance: Instance) { + function commitInstance(instance: HostConfig['instance']) { // Don't handle commit for containers if (!instance.parent) return @@ -186,8 +191,8 @@ function createRenderer(_roots: Map, _getEventPriority?: } } - // Append to container - if (!instance.parent.parent && instance.parent.object) { + // Append to parent + if (instance.parent.object) { if (instance.props.attach) { attach(instance.parent, instance) } else if (instance.object.isObject3D && instance.parent.object.isObject3D) { From f43e768eccb6c0e9e9b8d086bf5a9f379f30406b Mon Sep 17 00:00:00 2001 From: Cody Bennett <23324155+CodyJasonBennett@users.noreply.github.com> Date: Fri, 22 Jul 2022 10:42:49 -0500 Subject: [PATCH 17/28] fix(renderer): de-ref oldinstance after swap --- packages/fiber/src/core/renderer.ts | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/packages/fiber/src/core/renderer.ts b/packages/fiber/src/core/renderer.ts index 742b14e4c7..18742fdf61 100644 --- a/packages/fiber/src/core/renderer.ts +++ b/packages/fiber/src/core/renderer.ts @@ -207,17 +207,16 @@ function createRenderer(_roots: Map, _getEventPriority?: } function switchInstance( - instance: HostConfig['instance'], + oldInstance: HostConfig['instance'], type: HostConfig['type'], props: HostConfig['props'], fiber: Reconciler.Fiber, ) { // Create a new instance - const newInstance = createInstance(type, props, instance.root) + const newInstance = createInstance(type, props, oldInstance.root) - // Replace instance in scene-graph - const parent = instance.parent! - removeChild(parent, instance) + // Link up new instance + const parent = oldInstance.parent! appendChild(parent, newInstance) // Commit new instance object @@ -230,17 +229,21 @@ function createRenderer(_roots: Map, _getEventPriority?: } // Move children to new instance - if (instance.type !== 'primitive') { - for (const child of instance.children) { - appendChild(newInstance, child) - if (child.props.attach) { - detach(instance, child) - attach(newInstance, child) - } + for (const child of oldInstance.children) { + appendChild(newInstance, child) + if (child.props.attach) { + detach(oldInstance, child) + attach(newInstance, child) + } else if (child.object.isObject3D && oldInstance.object.isObject3D) { + oldInstance.object.remove(child.object) + newInstance.object.add(child.object) } - instance.children = [] } + // Cleanup old instance + oldInstance.children = [] + removeChild(parent, oldInstance) + // This evil hack switches the react-internal fiber node // https://github.com/facebook/react/issues/14983 // https://github.com/facebook/react/pull/15021 From 656d11bee90975d0c4a21b1109a714ff884fa608 Mon Sep 17 00:00:00 2001 From: Cody Bennett <23324155+CodyJasonBennett@users.noreply.github.com> Date: Fri, 22 Jul 2022 12:23:53 -0500 Subject: [PATCH 18/28] fix(RTTR): don't use internal container methods --- .../__snapshots__/RTTR.core.test.tsx.snap | 28 +++++++++---------- packages/test-renderer/src/index.tsx | 12 ++++---- 2 files changed, 19 insertions(+), 21 deletions(-) diff --git a/packages/test-renderer/src/__tests__/__snapshots__/RTTR.core.test.tsx.snap b/packages/test-renderer/src/__tests__/__snapshots__/RTTR.core.test.tsx.snap index 7cfdf9056d..aab79e87fb 100644 --- a/packages/test-renderer/src/__tests__/__snapshots__/RTTR.core.test.tsx.snap +++ b/packages/test-renderer/src/__tests__/__snapshots__/RTTR.core.test.tsx.snap @@ -569,8 +569,8 @@ Array [ "children": Array [], "props": Object { "args": Array [ - 2, - 2, + 6, + 6, ], "attach": "geometry", }, @@ -591,14 +591,14 @@ Array [ , , ], - "position-z": 12, + "rotation-x": 1, }, "type": "mesh", }, @@ -646,21 +646,21 @@ Array [ Object { "children": Array [], "props": Object { - "args": Array [ - 6, - 6, - ], - "attach": "geometry", + "args": Array [], + "attach": "material", }, - "type": "boxGeometry", + "type": "meshBasicMaterial", }, Object { "children": Array [], "props": Object { - "args": Array [], - "attach": "material", + "args": Array [ + 2, + 2, + ], + "attach": "geometry", }, - "type": "meshBasicMaterial", + "type": "boxGeometry", }, ], "props": Object { diff --git a/packages/test-renderer/src/index.tsx b/packages/test-renderer/src/index.tsx index 4d5917d72a..5db294c87c 100644 --- a/packages/test-renderer/src/index.tsx +++ b/packages/test-renderer/src/index.tsx @@ -69,13 +69,11 @@ const create = async (element: React.ReactNode, options?: Partial return reconciler.getPublicRootInstance(root) }, async update(newElement: React.ReactNode) { - const fiber = mockRoots.get(canvas)?.fiber - if (fiber) { - await act(async () => { - reconciler.updateContainer(newElement, fiber, null, () => null) - }) - } - return + if (!mockRoots.has(canvas)) return console.warn('RTTR: attempted to update an unmounted root!') + + await act(async () => { + _root.render(newElement) + }) }, toTree() { return toTree(scene) From f2db2f988bc8514eddb2de569efb738972a27474 Mon Sep 17 00:00:00 2001 From: Cody Bennett <23324155+CodyJasonBennett@users.noreply.github.com> Date: Fri, 22 Jul 2022 12:59:00 -0500 Subject: [PATCH 19/28] refactor(RTTR): cleanup fiber traversal --- packages/test-renderer/src/index.tsx | 34 ++++++++++------------------ 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/packages/test-renderer/src/index.tsx b/packages/test-renderer/src/index.tsx index 5db294c87c..14fd7fc77b 100644 --- a/packages/test-renderer/src/index.tsx +++ b/packages/test-renderer/src/index.tsx @@ -38,34 +38,24 @@ const create = async (element: React.ReactNode, options?: Partial const _store = mockRoots.get(canvas)!.store await act(async () => _root.render(element)) - const scene = (_store.getState().scene as any).__r3f as MockInstance + const _scene = (_store.getState().scene as any).__r3f as MockInstance return { - scene: wrapFiber(scene), + scene: wrapFiber(_scene), async unmount() { await act(async () => { _root.unmount() }) }, getInstance() { - // this is our root - const fiber = mockRoots.get(canvas)?.fiber - const current = fiber?.current?.child?.child?.child?.child - if (!current) return null - - /** - * we wrap our child in a Provider component - * and context.Provider, so do a little - * artificial dive to get round this and - * pass context.Provider as if it was the - * actual react root - */ - const root = { current } - - /** - * so this actually returns the instance - * the user has passed through as a Fiber - */ + // Bail if canvas is unmounted + if (!mockRoots.has(canvas)) return null + + // Traverse fiber nodes for R3F root + let root = { current: mockRoots.get(canvas)!.fiber.current } + while (root.current.stateNode !== _scene) root.current = root.current.child + + // Return R3F instance from root return reconciler.getPublicRootInstance(root) }, async update(newElement: React.ReactNode) { @@ -76,10 +66,10 @@ const create = async (element: React.ReactNode, options?: Partial }) }, toTree() { - return toTree(scene) + return toTree(_scene) }, toGraph() { - return toGraph(scene) + return toGraph(_scene) }, fireEvent: createEventFirer(act, _store), async advanceFrames(frames: number, delta: number | number[] = 1) { From d5fb4a7f4ccad9ccf397050bd4bf1dc0691dd771 Mon Sep 17 00:00:00 2001 From: Cody Bennett <23324155+CodyJasonBennett@users.noreply.github.com> Date: Fri, 22 Jul 2022 13:43:16 -0500 Subject: [PATCH 20/28] chore(types): don't export internal props --- packages/fiber/src/index.tsx | 2 +- packages/fiber/src/native.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/fiber/src/index.tsx b/packages/fiber/src/index.tsx index 6109f3fd52..543b26408d 100644 --- a/packages/fiber/src/index.tsx +++ b/packages/fiber/src/index.tsx @@ -1,7 +1,7 @@ export * from './three-types' import * as ReactThreeFiber from './three-types' export { ReactThreeFiber } -export type { Instance, InstanceProps } from './core/renderer' +export type { Instance } from './core/renderer' export type { Intersection, Subscription, diff --git a/packages/fiber/src/native.tsx b/packages/fiber/src/native.tsx index b17dc13e71..014371c76b 100644 --- a/packages/fiber/src/native.tsx +++ b/packages/fiber/src/native.tsx @@ -1,7 +1,7 @@ export * from './three-types' import * as ReactThreeFiber from './three-types' export { ReactThreeFiber } -export type { Instance, InstanceProps } from './core/renderer' +export type { Instance } from './core/renderer' export type { Intersection, Subscription, From 975a2ba1e43f4244a38e1db81495282c3d9bee3d Mon Sep 17 00:00:00 2001 From: Cody Bennett <23324155+CodyJasonBennett@users.noreply.github.com> Date: Fri, 22 Jul 2022 13:52:03 -0500 Subject: [PATCH 21/28] fix: hide non-renderable objects on suspend --- packages/fiber/src/core/renderer.ts | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/packages/fiber/src/core/renderer.ts b/packages/fiber/src/core/renderer.ts index 18742fdf61..bf31c9cc37 100644 --- a/packages/fiber/src/core/renderer.ts +++ b/packages/fiber/src/core/renderer.ts @@ -322,11 +322,25 @@ function createRenderer(_roots: Map, _getEventPriority?: shouldSetTextContent: () => false, clearContainer: () => false, hideInstance(instance) { - if (instance.object?.isObject3D) instance.object.visible = false + if (!instance.object) return + + if (instance.props.attach && instance.parent?.object) { + detach(instance.parent, instance) + } else if (instance.object.isObject3D) { + instance.object.visible = false + } + invalidateInstance(instance) }, unhideInstance(instance) { - if (instance.object?.isObject3D && instance.props.visible !== false) instance.object.visible = true + if (!instance.object) return + + if (instance.props.attach && instance.parent?.object) { + attach(instance.parent, instance) + } else if (instance.object.isObject3D && instance.props.visible !== false) { + instance.object.visible = true + } + invalidateInstance(instance) }, createTextInstance: () => { From 191a4fcb5b4d33799867ff632fbe5b130fb4ad48 Mon Sep 17 00:00:00 2001 From: Cody Bennett <23324155+CodyJasonBennett@users.noreply.github.com> Date: Fri, 29 Jul 2022 10:37:49 -0500 Subject: [PATCH 22/28] fix(renderer): always attach instance localstate --- packages/fiber/src/core/renderer.ts | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/packages/fiber/src/core/renderer.ts b/packages/fiber/src/core/renderer.ts index 15ec08e9b6..31fea16377 100644 --- a/packages/fiber/src/core/renderer.ts +++ b/packages/fiber/src/core/renderer.ts @@ -85,8 +85,6 @@ function createRenderer(_roots: Map, _getEventPriority?: handlers: {}, } - if (object) object.__r3f = instance - return instance } @@ -100,8 +98,6 @@ function createRenderer(_roots: Map, _getEventPriority?: child: HostConfig['instance'], beforeChild: HostConfig['instance'], ) { - if (!child) return - child.parent = parent parent.children.splice(parent.children.indexOf(beforeChild), 0, child) } @@ -157,25 +153,28 @@ function createRenderer(_roots: Map, _getEventPriority?: }) } } + delete child.object.__r3f child.object = null if (dispose === undefined) invalidateInstance(child) } function commitInstance(instance: HostConfig['instance']) { - // Don't handle commit for containers - if (!instance.parent) return - // Create object - if (instance.type !== 'primitive' && !instance.object) { + if (instance.type !== 'primitive') { const name = `${instance.type[0].toUpperCase()}${instance.type.slice(1)}` const target = catalogue[name] const { args = [] } = instance.props instance.object = new target(...args) - instance.object.__r3f = instance } + // Attach object instance handle + instance.object.__r3f = instance + + // Don't handle children for containers + if (!instance.parent) return + // Auto-attach geometry and materials to meshes if (!instance.props.attach) { if (instance.type.endsWith('Geometry')) instance.props.attach = 'geometry' From 35056e871b4532a969842a3f71c4d6e73524588b Mon Sep 17 00:00:00 2001 From: Cody Bennett <23324155+CodyJasonBennett@users.noreply.github.com> Date: Sat, 30 Jul 2022 17:22:25 -0500 Subject: [PATCH 23/28] fix: handle text cases, add coverage --- packages/fiber/src/core/renderer.ts | 20 +++++++++++++++----- packages/fiber/tests/core/renderer.test.tsx | 20 ++++++++++++++++++++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/packages/fiber/src/core/renderer.ts b/packages/fiber/src/core/renderer.ts index 054a510d12..c3c8477d6f 100644 --- a/packages/fiber/src/core/renderer.ts +++ b/packages/fiber/src/core/renderer.ts @@ -88,16 +88,20 @@ function createRenderer(_roots: Map, _getEventPriority?: return instance } - function appendChild(parent: HostConfig['instance'], child: HostConfig['instance']) { + function appendChild(parent: HostConfig['instance'], child: HostConfig['instance'] | HostConfig['textInstance']) { + if (!child) return + child.parent = parent parent.children.push(child) } function insertBefore( parent: HostConfig['instance'], - child: HostConfig['instance'], - beforeChild: HostConfig['instance'], + child: HostConfig['instance'] | HostConfig['textInstance'], + beforeChild: HostConfig['instance'] | HostConfig['textInstance'], ) { + if (!child || !beforeChild) return + child.parent = parent parent.children.splice(parent.children.indexOf(beforeChild), 0, child) } @@ -112,7 +116,13 @@ function createRenderer(_roots: Map, _getEventPriority?: } } - function removeChild(parent: HostConfig['instance'], child: HostConfig['instance'], dispose?: boolean) { + function removeChild( + parent: HostConfig['instance'], + child: HostConfig['instance'] | HostConfig['textInstance'], + dispose?: boolean, + ) { + if (!child) return + child.parent = null const childIndex = parent.children.indexOf(child) if (childIndex !== -1) parent.children.splice(childIndex, 1) @@ -318,7 +328,7 @@ function createRenderer(_roots: Map, _getEventPriority?: }, finalizeInitialChildren: () => true, commitMount: commitInstance, - getPublicInstance: (instance) => instance!.object, + getPublicInstance: (instance) => instance?.object, prepareForCommit: () => null, preparePortalMount: (container) => container, resetAfterCommit: () => {}, diff --git a/packages/fiber/tests/core/renderer.test.tsx b/packages/fiber/tests/core/renderer.test.tsx index 74207e3f4d..29766d2c23 100644 --- a/packages/fiber/tests/core/renderer.test.tsx +++ b/packages/fiber/tests/core/renderer.test.tsx @@ -17,6 +17,7 @@ import { import { UseBoundStore } from 'zustand' import { privateKeys, RootState } from '../../src/core/store' import { Instance } from '../../src/core/renderer' +import { suspend } from 'suspend-react' type ComponentMesh = THREE.Mesh @@ -738,4 +739,23 @@ describe('renderer', () => { const respectedKeys = privateKeys.filter((key) => overwrittenKeys.includes(key) || state[key] === portalState[key]) expect(respectedKeys).toStrictEqual(privateKeys) }) + + it('should gracefully handle text', async () => { + // Mount + await act(async () => root.render(<>one)) + // Update + await act(async () => root.render(<>two)) + // Unmount + await act(async () => root.render(<>)) + + // Suspense + const Test = () => suspend(async () => null, []) + await act(async () => { + root.render( + + + , + ) + }) + }) }) From d5402e07a33f7987d5e5727e894fec50faf285d0 Mon Sep 17 00:00:00 2001 From: Cody Bennett <23324155+CodyJasonBennett@users.noreply.github.com> Date: Thu, 4 Aug 2022 14:30:23 -0500 Subject: [PATCH 24/28] chore: fix conflicts --- packages/fiber/src/core/renderer.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/fiber/src/core/renderer.ts b/packages/fiber/src/core/renderer.ts index c3c8477d6f..bfa5af1c0c 100644 --- a/packages/fiber/src/core/renderer.ts +++ b/packages/fiber/src/core/renderer.ts @@ -70,9 +70,9 @@ function createRenderer(_roots: Map, _getEventPriority?: const target = catalogue[name] if (type !== 'primitive' && !target) - throw `${name} is not part of the THREE namespace! Did you forget to extend? See: https://docs.pmnd.rs/react-three-fiber/api/objects#using-3rd-party-objects-declaratively` + throw new Error(`R3F: ${name} is not part of the THREE namespace! Did you forget to extend? See: https://docs.pmnd.rs/react-three-fiber/api/objects#using-3rd-party-objects-declaratively`) - if (type === 'primitive' && !object) throw `Primitives without 'object' are invalid!` + if (type === 'primitive' && !object) throw new Error(`R3F: Primitives without 'object' are invalid!`) const instance: HostConfig['instance'] = { root, From 59a3aef4dc605e4bdec564413bc9f908d755e7a3 Mon Sep 17 00:00:00 2001 From: Cody Bennett <23324155+CodyJasonBennett@users.noreply.github.com> Date: Thu, 4 Aug 2022 15:14:05 -0500 Subject: [PATCH 25/28] chore: lint --- packages/fiber/src/core/renderer.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/fiber/src/core/renderer.ts b/packages/fiber/src/core/renderer.ts index bfa5af1c0c..8440e68eb1 100644 --- a/packages/fiber/src/core/renderer.ts +++ b/packages/fiber/src/core/renderer.ts @@ -70,7 +70,9 @@ function createRenderer(_roots: Map, _getEventPriority?: const target = catalogue[name] if (type !== 'primitive' && !target) - throw new Error(`R3F: ${name} is not part of the THREE namespace! Did you forget to extend? See: https://docs.pmnd.rs/react-three-fiber/api/objects#using-3rd-party-objects-declaratively`) + throw new Error( + `R3F: ${name} is not part of the THREE namespace! Did you forget to extend? See: https://docs.pmnd.rs/react-three-fiber/api/objects#using-3rd-party-objects-declaratively`, + ) if (type === 'primitive' && !object) throw new Error(`R3F: Primitives without 'object' are invalid!`) From 50a1fad5b7a4bfcbbe6767a9bcc5c7993c3c7ee9 Mon Sep 17 00:00:00 2001 From: Cody Bennett <23324155+CodyJasonBennett@users.noreply.github.com> Date: Thu, 4 Aug 2022 17:43:10 -0500 Subject: [PATCH 26/28] experiment: add suspense boundaries to portals example --- example/src/demos/Portals.tsx | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/example/src/demos/Portals.tsx b/example/src/demos/Portals.tsx index 9cfc027d10..2d82425bc6 100644 --- a/example/src/demos/Portals.tsx +++ b/example/src/demos/Portals.tsx @@ -1,6 +1,6 @@ import * as THREE from 'three' import * as React from 'react' -import { useCallback, useLayoutEffect, useRef, useState } from 'react' +import { useCallback, useLayoutEffect, useRef, useState, Suspense } from 'react' import { Canvas, useFrame, useThree, createPortal } from '@react-three/fiber' import { useGLTF, OrbitControls, useFBO, Environment } from '@react-three/drei' @@ -110,16 +110,22 @@ const App = () => ( {/* First layer, a portal */} - - + + + + - - + + + + - - + + + + From ac63742a3413518ea8fcd7aba639c4b8b6e08d63 Mon Sep 17 00:00:00 2001 From: Cody Bennett <23324155+CodyJasonBennett@users.noreply.github.com> Date: Sat, 20 Aug 2022 14:55:24 -0500 Subject: [PATCH 27/28] experiment: commit on finalizeInitialChildren --- example/src/demos/Portals.tsx | 20 +++++++------------- packages/fiber/src/core/renderer.ts | 7 +++++-- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/example/src/demos/Portals.tsx b/example/src/demos/Portals.tsx index 2d82425bc6..9cfc027d10 100644 --- a/example/src/demos/Portals.tsx +++ b/example/src/demos/Portals.tsx @@ -1,6 +1,6 @@ import * as THREE from 'three' import * as React from 'react' -import { useCallback, useLayoutEffect, useRef, useState, Suspense } from 'react' +import { useCallback, useLayoutEffect, useRef, useState } from 'react' import { Canvas, useFrame, useThree, createPortal } from '@react-three/fiber' import { useGLTF, OrbitControls, useFBO, Environment } from '@react-three/drei' @@ -110,22 +110,16 @@ const App = () => ( {/* First layer, a portal */} - - - - + + - - - - + + - - - - + + diff --git a/packages/fiber/src/core/renderer.ts b/packages/fiber/src/core/renderer.ts index 8440e68eb1..e91bb699ea 100644 --- a/packages/fiber/src/core/renderer.ts +++ b/packages/fiber/src/core/renderer.ts @@ -328,8 +328,11 @@ function createRenderer(_roots: Map, _getEventPriority?: Object.assign(instance.props, newProps) applyProps(instance.object, changedProps) }, - finalizeInitialChildren: () => true, - commitMount: commitInstance, + finalizeInitialChildren(instance) { + commitInstance(instance) + return false + }, + // commitMount: commitInstance, getPublicInstance: (instance) => instance?.object, prepareForCommit: () => null, preparePortalMount: (container) => container, From eabc13ba7d0c08a69b98a936830cd0bc6118d353 Mon Sep 17 00:00:00 2001 From: Cody Bennett <23324155+CodyJasonBennett@users.noreply.github.com> Date: Sat, 20 Aug 2022 15:05:08 -0500 Subject: [PATCH 28/28] fix: revert commit on finalizeInitialChildren --- packages/fiber/src/core/renderer.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/fiber/src/core/renderer.ts b/packages/fiber/src/core/renderer.ts index e91bb699ea..8440e68eb1 100644 --- a/packages/fiber/src/core/renderer.ts +++ b/packages/fiber/src/core/renderer.ts @@ -328,11 +328,8 @@ function createRenderer(_roots: Map, _getEventPriority?: Object.assign(instance.props, newProps) applyProps(instance.object, changedProps) }, - finalizeInitialChildren(instance) { - commitInstance(instance) - return false - }, - // commitMount: commitInstance, + finalizeInitialChildren: () => true, + commitMount: commitInstance, getPublicInstance: (instance) => instance?.object, prepareForCommit: () => null, preparePortalMount: (container) => container,