Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes a bug when reusing primitives in an array. #3142

Closed
wants to merge 2 commits into from

Conversation

Inuniku
Copy link
Contributor

@Inuniku Inuniku commented Jan 7, 2024

I added a test for recreating the bug. Maybe there is a nicer method instead of inflating the LocalState.

const primitive1 = new THREE.Mesh()
const primitive2 = new THREE.Mesh()

// Initialize a list of primitives
await act(async () =>
  root.render(
    <group>
      <primitive key={'a'} object={primitive1} name="p1" dispose={null} />
    </group>,
  ),
)

expect(primitive1.__r3f?.type).toBe('primitive')

// New list of primitives while reusing primitive object
await act(async () =>
  root.render(
    <group>
      <primitive key={'b'} object={primitive1} name="p1" dispose={null} />
      <primitive key={'c'} object={primitive2} name="p2" dispose={null} />
    </group>,
  ),
)

expect(primitive1.__r3f?.type).toBe('primitive')
expect(primitive2.__r3f?.type).toBe('primitive')

p1 would here be invalid. (__r3f instance got cleared)

Copy link

codesandbox-ci bot commented Jan 7, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit ebddb72:

Sandbox Source
example Configuration

@CodyJasonBennett
Copy link
Member

Sorry, I missed this. Working on bugs surrounding arrays and us swapping elements in-place. We have a few flags already like autoRemovedBeforeAppend which workaround a more fundamental issue I need to track down.

@CodyJasonBennett
Copy link
Member

We'll be addressing this in #3248. Thanks for the help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants