test renderer state is not correct since 8.11.2

[timmevandermeer]

I'm experiencing several issues with the test renderer since version 8.11.2. The state of the renderer does not seem to be up-to-date or correct in every case, causing several of my tests to break. I created a repro of one of the issues here:
https://codesandbox.io/p/sandbox/react-three-test-renderer-issue-ynh75q

In this example I test the rotation of the cube by taking a snapshot

expect(cubes[0].instance.rotation.toArray()).toMatchInlineSnapshot(`
      [
        1.5707963267948966,
        0.7853981633974483,
        0,
        "XYZ",
      ]
    `);

However, when the rotation prop is set to a Three Euler object (not the tuple), the instance actually has a quite strange Euler inside an Euler as rotation.

Euler {
  isEuler: true,
  _x: Euler {
    isEuler: true,
    _x: 1.5707963267948966,
    _y: 0.7853981633974483,
    _z: 0,
    _order: 'XYZ'
  },
  _y: undefined,
  _z: undefined,
  _order: 'XYZ',
  _onChangeCallback: [Function: onRotationChange]
}

This is likely related to #2757, as that change included in 8.11.2 does actually refer to breaking something in the test renderer. When using r3f 8.11.1 or earlier the output is correct.

[CodyJasonBennett]

I'm not able to reproduce any longer on latest. I'll walk through to see if/when this changed, but it would be good to confirm whether this is still an issue on your end.

import ReactThreeTestRenderer from '@react-three/test-renderer'
import * as React from 'react'
import * as THREE from 'three'

// Euler {
//   _x: 1.5707963267948966,
//   _y: 0.7853981633974483,
//   _z: 0,
//   _order: 'XYZ',
//   _onChangeCallback: [Function: onRotationChange]
// }

it('works', async () => {
  const rotation = new THREE.Euler(Math.PI / 2, Math.PI / 4, 0)
  const renderer = await ReactThreeTestRenderer.create(<mesh rotation={rotation} />)
  const mesh = renderer.scene.findByType('Mesh')
  expect(mesh.instance.rotation.toArray()).toMatchInlineSnapshot(`
    Array [
      1.5707963267948966,
      0.7853981633974483,
      0,
      "XYZ",
    ]
  `)
})

[timmevandermeer]

I don't think it is solved on latest. You can run the test in the reproduction link by running the test task.

[timmevandermeer]

I also added your more minimal example to the reproduction but the same issue occurs. Not sure what the difference is between our setups, perhaps vitest? But I'm more wondering what change in 8.11.2 exactly causes this behavior

[CodyJasonBennett]

I see, I'll refer to your setup. I tried this locally in our test suite which uses Jest, but I'll have to study your environment more to see whether teardown is affecting this or a general issue with React's scheduler. Undefined behavior is very bad, but I'd also suspect it's related to changes with HMR since it works by replacing undefined or rather unset values with memoized properties.

[CodyJasonBennett]

Seems the change from comparing targetProp.constructor.name to targetProp.constructor against target/receiver props is causing the discrepancy in Vitest with/without isolate. cc @drcmda, does RTTR do anything special or wrap props in any way that break constructor lookups? I'm not inlining deps here for testing, although it's possible optimizeDeps from Vite is affecting this.

[joshuaellis]

does RTTR do anything special or wrap props in any way that break constructor lookups

No, it's very much typical behaviour – very similar to how react-test-renderer works as well. I would be cautious of vitest, it has a multitude of bugs from what I remember

[CodyJasonBennett]

I'm noticing that the props belong to separate implementations of the class, and that this only affects Vitest when loading the library as an external. The source of the classes also look to be minified or whitespace is not preserved. I also can't reproduce when testing locally in the repo with relative paths.

[CodyJasonBennett]

I'd see if you can ensure that only one copy of three.js is included and isn't getting inlined anywhere via optimizeDeps or else. I'm afraid there's not much we can do on our side.

[timmevandermeer]

The issue also occurs when excluding three and r3f in optimizeDeps, when disabling optimizeDeps entirely and also when disabling inlining in vitest config.

I have also been able to reproduce (see original repro) in jest (both ts-jest and @swc/jest), but only when using esm modules with the flag --experimental-vm-modules. So it seems the issue occurs when using ESM, and not when using CJS.

[CodyJasonBennett]

This issue occurs because there are separate copies of three.js classes that would break strict equality checks, whether inlined or duplicated via NPM. I'm not keen on using naive strict lookups, but that would be the only way this is actionable on our part.

[timmevandermeer]

I'm just not sure why this can't be regarded as a bug. The R3 test-renderer claims to be test-runner agnostic but fails with a very minimal test case when using a test runner based on ESM modules. In this repro that might even be more clear: the data gets stored incorrectly in the scene graph. https://stackblitz.com/edit/vitest-dev-vitest-hrjava?file=test%2Freact-three.test.tsx

I can't really think of why there would be duplicate instances of Three classes and the issue does not seem to occur in for instance Testing Library (or in the React part of RTTR for that matter)

[CodyJasonBennett]

This is a bug in Vite thus you should be continuing this in Vite -- the problem case would be strict equality amongst optimized deps (the default in Vite). We can explore a best-effort workaround, but a proper fix would be upstream so you don't encounter random breakage across dependencies.

[timmevandermeer]

The thing is that it also occurs with ts-jest and swc jest when using the esm flag (see example earlier in thread), and also when disabling optimized dependencies or excluding three from optimization or marking it as external. So I can report it at vitest but expect to get pointed back here. Will try I suppose

[CodyJasonBennett]

I'm happy to further explain the issue if there is any question upstream. For our purposes, try seeing if #3025 is sufficient which reverts the mentioned changes from #2757.

{
  "dependencies": {
    "@react-three/fiber": "https://pkg.csb.dev/pmndrs/react-three-fiber/commit/d9cee65a/@react-three/fiber"
  }
}

[timmevandermeer]

yes with the change reverted it works correctly, thanks for setting that up!

[timmevandermeer]

I reported the issue to Vitest. But in updating the reproduction I found that the issue even occurs when creating a class not from Three.js (see the FakeVector example at https://stackblitz.com/edit/vitest-dev-vitest-hrjava?file=test/react-three.test.tsx)

[CodyJasonBennett]

Well, that was convenient. I included the patch in v8.14.5. I'll follow the issue upstream.

Note that this is for @react-three/fiber and not @react-three/test-renderer.