fix: make SpringRef instances callable to avoid breaking change #1359

[dbismut]

Fixes #1359

const [springs, api] = useSpring(() => ({}))
api.start({}) // this works
api({}) // this also works

Other changes

Fixes PickAnimated type eaf473f

Before - tries to union all props, including arrayed props, resulting in unknown types:

After - infer SpringValue types only from the from prop when it exists:

This — I think — should be fine considering that the from prop is mandatory.

[codesandbox-ci]

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 9a39ae2:

Sandbox	Source
spring-card	Configuration
spring-goo-blobs	Configuration
spring-flip-card	Configuration
spring-slide	Configuration
spring-draggable-list	Configuration
spring-cards-stack	Configuration
spring-viewpager	Configuration
spring-simple-transition	Configuration
spring-image-fade	Configuration
spring-list-reordering	Configuration
react-spring (set)	Issue #1359
react-spring#1118	Issue #1359

[a-type]

I don't think this has the intended effect. A few issues stand out:

1. .set doesn't animate. This is subtle but noticeable in the Codesandbox. .start must be used to animate, instead (see the difference by replacing the ref() calls with the commented lines below them in this sandbox)
2. v8 functionality included the immediate: true flag as a field in set which would apply values without animation. If the goal is to avoid backwards-incompatible changes, this should be added as well (probably forwarding to .set)

[dbismut]

@a-type sorry just seen your comment now, we were discussing internally and @joshuaellis made me realize the existence of start. With the latest commits, this is the current situation of the PR:

// v9.0.0-rc3
const [springs, set] = useSpring(() => ({ x: 0 }))
set({ x: 3 }) // this animates to 3

// v9.0.0
const [springs, set] = useSpring(() => ({ x: 0 }))
set({ x: 3 }) // this doesn't work

// v9.0.0
const [springs, { set }] = useSpring(() => ({ x: 0 }))
set({ x: 3 }) // this doesn't work because this.current is undefined

// v9.0.0
const [springs, api] = useSpring(() => ({ x: 0 }))
api.set({ x: 3 }) // this does work but doesn't animate
api.start({ x: 3 }) // this does work and animates

// v9.0.0 PR #1387
const [springs, api] = useSpring(() => ({ x: 0 }))
api({ x: 3 }) // this animates to 3 and avoids breaking change

So at this point I think we're good, since we're fixing breaking changes.

[a-type]

@dbismut Yeah the new changes look better - although I don't think immediate is supported in .start. To do a true back-compat interop you'll probably have to detect immediate and delegate to .set instead.

[dbismut]

@a-type actually it seems like start does support immediate. Have a look at https://codesandbox.io/s/spring-slide-3yu8o

[a-type]

@dbismut oh nice, it wasn't working for some reason when I was testing my sandbox with the newest commit. Perhaps I did something wrong.

[joshuaellis]

@dbismut I've added a utility function - once loosely based off lodash.once. Tested it with GooBlobs where the API is called in an event listener, only logs once 👍🏼 I'm happy with this PR if you are.

[dbismut]

I am! Initially I thought I would keep this PR open and push all fixes based on potential bugs I would find in the demo sandboxes, so I don't know if you want to merge on master now or wait until I complete the sandboxes?

[joshuaellis]

Let's wait till we've merged those other changes, but I would like to merge this separately just so if we need to revert for whatever reason, its straight forward to do so, so maybe we could merge into master post merging the others?

[dbismut]

Makes sense! We can change the target to master later on. Let's do the demos first then!

[mmintel]

great job! glad you already found a solution. is there a workaround until this one is merged?

[dbismut]

@mmintel the workaround is actually what we recommend using as we're deprecating calling the ref directly.

const [springs, api] = useSpring(() => ({x: 0}))
api.start({x: 3})

The base branch was changed.

[HQ92]

I has a few questions about this, this works for me in relation to useSpring, but I've tried this with a wrapped useTrail hook and it is complaining. When trying to call the spring ref it replies with "This expression is not callable. Type '{}' has no call signatures.ts(2349)".

The sandbox still seems to work but in my project the settings are setup to prevent it from building with typescript errors like this:

https://codesandbox.io/s/white-sunset-26qsi?file=/src/App.tsx

[joshuaellis]

To be honest, i'm not encouraging the use of this. So unless it's a real blocker like #1423 I don't think we're going to look into issues with it. You should just call api.start like in the documentation.

[HQ92]

Ok that's fair enough, I've copied and pasted my spring wrapper into the above sandbox and it shows no typescript errors:

https://codesandbox.io/s/white-sunset-26qsi?file=/src/App.tsx

But inside vscode there seems to be some sort of an issue maybe with the typings in the library that is flagged an error?

The error specifically being:

[joshuaellis]

This is our example of useTrail with the API – https://codesandbox.io/s/quizzical-cohen-p43s2?file=/src/App.tsx in typescript, I can't see any TS errors. So im not sure what to say.

[HQ92]

Hmmm that's strange, it seems to only be an issue with that particular line, I've gone through and updated my dependencies including typescript to match the sandbox but was still receiving the error. I put in a @ts-ignore flag and in the background react-spring seems to work as expected. It must be an issue with the typescript setup somewhere along the line.