@react-spring/three doesn't update all position parameters when passed to r3f mesh

[midanosi]

🐛 Bug Report

I have a spring that I'm trying to use to make position parameter for a Box in r3f to go from e.g. [1,1,1] -> [2,2,2]

const position = expandSpring.to((expandFactor) => [
  xInit + Math.sign(xInit) * expandFactor,
  yInit + Math.sign(yInit) * expandFactor,
  zInit + Math.sign(zInit) * expandFactor
])

But despite interpolating the spring to an array of values, when I pass it into <a.mesh position={interpolatedPosition} />, it only updates the x coordinate. If I change the interpolation fn to make x value static, e.g.

[xInit, yInit + Math.sign(yInit) * expandFactor, zInit + Math.sign(zInit) * expandFactor]

then it'll now update only the y value.

I wasn't sure whether to raise this here or in the r3f repo! Sorry if I got the wrong place. Also aware that this might not be a bug, in which case may I ask for guidance on how to achieve what I'm aiming for?

To Reproduce

See codesandbox

Expected behavior

For all x, y, z parameters to be updated

Link to repro (highly encouraged)

https://codesandbox.io/s/exploding-square-forked-for-react-spring-github-issue-dont-touch-n48b9

Environment

• @react-spring/three v9.1.1
• react v17.0.2

Edit: changed sandbox link to stay static

[midanosi]

... only 10 minutes later I came across this in r3f api readme: https://github.com/pmndrs/react-three-fiber/blob/master/markdown/api.md#piercing-into-nested-properties

So I'm now setting individual positions using position-y and that's doing the trick!

May I still ask the newbie question of why this is the case? If it's easy to answer that question?
This definitely felt like a gotcha moment that'd be easy for others to stumble across. I'd be happy to add a small README section in one of the repos if it'd be appropriate to do so.

[joshuaellis]

Can you reduce your sandbox down just to highlight the core issue you're facing? There's too much noise for me to debug.

Side note, yes using position-x etc. will work, but I think position should be able to work too 🤔

[midanosi]

Good point, sorry about that. Have gotten rid of all the clutter, same link here: https://codesandbox.io/s/exploding-square-forked-for-react-spring-github-issue-dont-touch-n48b9?file=/src/App.js

[mysterybear]

I think this is a real issue, I was about to log this separately but I guess it's the same:

https://codesandbox.io/s/github/mysterybear/r3f-react-app/tree/spring-vector4-bug?file=/src/components/CropImage.tsx

The breakage occurs with using to or interpolate ... If you don't go through to or interpolate then it works... Plus it'd be very desirable behaviour if it did work... So that makes me think this is a real bug!

[joshuaellis]

@mysterybear potential bug is simply a flag so a maintainer knows they need to investigate it. However, if you're wanting to have this fixed soon I would suggest you look at fixing it yourself as I have a large list of bugs to try and get through.

[midanosi]

I had a go at debugging this!

(took me quite a long while, but...) I believe the offending line of code is:

react-spring/packages/animated/src/AnimatedArray.ts

Line 31 in 6ebafe3

return payload.some((node, i) => node.setValue(source[i]))

Swapping out the .some for .forEach, or commenting out that if block both fix the problem.
Thanks @joshuaellis for the help you gave me on Discord to get dev env set up.

I'm keen to make a PR, (with a unit test perhaps?), but I'm not sure if removing/changing that if block is getting rid of an optimisation, does it look safe to do that?

[joshuaellis]

Some will return a Boolean won't it? So what is the Boolean not returning true blocking from updating...? 🤔

Because I assume for each doesn't return true.

[jldinh]

I think I see what the problem is: some will stop iterating as soon as the function returns a truthy value for one item of the array.
To avoid that, we could rewrite to:
return payload.map((node, i) => node.setValue(source[i])).some(Boolean)
This way we can make sure all changes have a chance of being applied before checking if at least one change was made.

[joshuaellis]

Okay, well @midanosi ifn you want to make a PR with a test we can evaluate this change with all the demos we have in the repo etc. To check for unwanted side effects, although hopefully there wont be and if there are the tests will bring them up 👍🏼

[midanosi]

@joshuaellis I saw you assigned this to yourself yesterday. Life got in the way, but I finally submitted a pull request for this fix, wanted to let you know so that the work isn't re-done.

[joshuaellis]

released in v9.2.2, big thanks to @midanosi ⭐