Skip to content

Commit

Permalink
[v9] fix: primitive children mounting (#3248)
Browse files Browse the repository at this point in the history
  • Loading branch information
krispya committed May 3, 2024
1 parent cc3080b commit 1183029
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 6 deletions.
21 changes: 18 additions & 3 deletions packages/fiber/src/core/reconciler.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,12 @@ export const extend = <T extends Catalogue | ConstructorRepresentation>(
}
}

function createInstance(type: string, props: HostConfig['props'], root: RootStore): HostConfig['instance'] {
function createInstance(
type: string,
props: HostConfig['props'],
root: RootStore,
flushPrimitive = true,
): HostConfig['instance'] {
// Get target from catalogue
const name = `${type[0].toUpperCase()}${type.slice(1)}`
const target = catalogue[name]
Expand All @@ -121,6 +126,9 @@ function createInstance(type: string, props: HostConfig['props'], root: RootStor
// Throw if an object or literal was passed for args
if (props.args !== undefined && !Array.isArray(props.args)) throw new Error('R3F: The args prop must be an array!')

// Regenerate the R3F instance for primitives to simulate a new object
if (flushPrimitive && type === 'primitive' && props.object?.__r3f) delete props.object.__r3f

// Create instance
const instance = prepare(props.object, root, type, props)

Expand Down Expand Up @@ -309,7 +317,12 @@ function switchInstance(
if (oldInstance.isHidden) unhideInstance(oldInstance)

// Create a new instance
const newInstance = createInstance(type, props, oldInstance.root)
const newInstance = createInstance(type, props, oldInstance.root, false)

// Update attach props for primitives since we don't flush them
if (type === 'primitive') {
newInstance.props.attach = props.attach
}

// Move children to new instance
for (const child of oldInstance.children) {
Expand Down Expand Up @@ -377,7 +390,9 @@ export const reconciler = Reconciler<
supportsMutation: true,
supportsPersistence: false,
supportsHydration: false,
createInstance,
createInstance(type, props, root) {
return createInstance(type, props, root)
},
removeChild,
appendChild,
appendInitialChild: appendChild,
Expand Down
67 changes: 64 additions & 3 deletions packages/fiber/tests/renderer.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,69 @@ describe('renderer', () => {
expect(object.name).toBe('primitive')
})

it('should remove children from primitive when unmounted', async () => {
const object = new THREE.Group()

function Parent({ children, show }: { children: React.ReactNode; show: boolean }) {
return show ? <primitive object={object}>{children}</primitive> : null
}

function Component({ show }: { show: boolean }) {
return (
<Parent show={show}>
<group name="A" />
<group name="B" />
</Parent>
)
}

const store = await act(async () => root.render(<Component show={true} />))
const { scene } = store.getState()

expect(scene.children.length).toBe(1)
expect(scene.children[0]).toBe(object)
expect(object.children.length).toBe(2)

await act(async () => root.render(<Component show={false} />))

expect(scene.children.length).toBe(0)
expect(object.children.length).toBe(0)
})

it('should remove then add children from primitive when key changes', async () => {
const object = new THREE.Group()

function Parent({ children, primitiveKey }: { children: React.ReactNode; primitiveKey: string }) {
return (
<primitive key={primitiveKey} object={object}>
{children}
</primitive>
)
}

function Component({ primitiveKey }: { primitiveKey: string }) {
return (
<Parent primitiveKey={primitiveKey}>
<group name="A" />
<group name="B" />
</Parent>
)
}

const store = await act(async () => root.render(<Component primitiveKey="A" />))
const { scene } = store.getState()

expect(scene.children.length).toBe(1)
expect(scene.children[0]).toBe(object)
expect(object.children.length).toBe(2)

await act(async () => root.render(<Component primitiveKey="B" />))

expect(scene.children.length).toBe(1)
expect(scene.children[0]).toBe(object)
expect(object.children.length).toBe(2)
})

it('should go through lifecycle', async () => {
const lifecycle: string[] = []

Expand Down Expand Up @@ -444,11 +507,9 @@ describe('renderer', () => {
expect(scene.children.map((o) => o.name)).toStrictEqual(mixedArray.map((o) => o.name))
})

// TODO: fix this case, also see:
// https://github.com/pmndrs/react-three-fiber/issues/1892
// https://github.com/pmndrs/react-three-fiber/issues/3125
// https://github.com/pmndrs/react-three-fiber/issues/3143
it.skip('can swap 4 array primitives via attach', async () => {
it('can swap 4 array primitives via attach', async () => {
const a = new THREE.Group()
a.name = 'a'
const b = new THREE.Group()
Expand Down

0 comments on commit 1183029

Please sign in to comment.