Update typescript types, fix react-spring/addons, add react-spring/hooks

[Jessidhia]

You people who wrote the hooks, particularly useKeyframes are magic 🤯

Because the package.json is specifying a specific type file to use for declaring the types, the types had to be refactored to be ambient declarations of modules. An alternative is to put them all on the top level and remove the typings key from the package.json entirely -- typescript will then find the index.d.ts, addons.d.ts et al on its own if they are on the top level of the package.

Disclaimer: I have not actually tested this at runtime to see if the types are correct -- I just read through the code and declared the types as they appear to be, and checked in the IDE that the input/output types are as expected. There might be mistakes.

(note: the bulk of the changes to index.d.ts are a reindent; but git can't detect that. It still thinks it's a delete and a new file.)

[Jessidhia]

Babel is compiling const [state, count] = ... to array index accesses, which "work" (they return undefined), but if there was no loose mode babel, this would run the iterator and hit done: true before the expected number of iterations and thus throw.

The proper fix would be to have const [state = undefined, count = undefined] = ...; that wouldn't crash on real engines either.

[Jessidhia]

i.e. you have to write

spring({
  foo: (async next => next({ x: 0 })) as KeyframeFn<{ x: number }>
})

because of how we cannot tell TypeScript the possible function type of an arbitrary key.

If we add an index type, it then is no longer possible to have config be a function that receives a string, or if we do make it possible, then there are two possible function signatures to pick so TypeScript picks neither and we're back at the implicit any problem.

Even then, because of how TypeScript changes its inference when calculating a generic argument, it would not work. TypeScript would first derive an entirely new type for the argument (similar to doing const x = (the argument)), and then check if the derived type fits the extends clause. It's similar to subclassing -- subclasses in TypeScript don't inherit their parents' types when they override methods, instead they get their very own fresh method type, and then later the compiler checks if it is actually compatible with the superclass'.

Making it not generic so we could use the index signature goes back to problem 1, config can also be a function and it has a different signature, but its signature has to be in the index type too.

[Jessidhia]

An alternative I thought of is to also have the keys be generic; this would allow us to not have to use an index signature.

    export function spring<TSlot extends string, DS extends object>(
      slots: Record<TSlot, DS | ReadonlyArray<DS> | KeyframeFn<DS>> & SpringKeyframeSlotsConfig,
      initialProps?: UseSpringProps<DS>
    ): UseSpringKeyframesWithSlots<Record<TSlot, DS>>

However, because there's no way to indicate to typescript that something that extends object must also not be a function, TS just infers DS as a function signature with an implicit any and it's back to problem 1 again.

I could also remove DS as a generic argument but then it's not possible to check if the props you produce are assignable to the props of the component you're spreading them into.

[Jessidhia]

I wonder if I'm not doing things backwards; maybe what you do need is to pass your component type (or props) as a generic so we can derive what to Pick from it.

But I still can't find a good DX for it, not until TypeScript has placeholder types (should be in 3.3?)

[Jessidhia]

interpolateTo's fault 😭

This is likely why the current types for Spring, Trail et al have a required to prop. This is my attempt to make the hooks not have the to prop required.

[Jessidhia]

Actually, I can't find the code that was doing it anymore? 🤔

EDIT: it's on master, just not on 7.1.3.

By the way, there's a potential bug: it's never reset to null. If the ref object comes from another component through props (say, animationRef={...}) and the component with useSpring unmounts, you can leak a Controller.

Probably just needs a useEffect(() => () => { ref.current = null }, [ref]).

[Jessidhia]

I could implement this with a fake nominal type if you still want to keep Controller opaque 🤔

[Jessidhia]

I tried following around to see if Controller or useSpring did something that would effectively give that 3rd argument but I didn't see anything.

Documentation mistake / copy&paste from the Keyframe class?

[drcmda]

Thanks so much for this! If we can properly bring hooks through the experimental phase and type them accordingly that would be awesome! Do you plan to amend this or is this ready?

[Jessidhia]

I think this is probably ok as a first iteration, to actually try it and see what works.

Would likely require further breaking changes before it's "the done deal", though; hopefully that's ok for experimental stuff anyway.

---

Actually, I want to see if I can change it to use plain module declarations instead of ambient module declarations, but that should only take a few minutes if it works

[Jessidhia]

I managed to do it 🎉

As a bonus, there are also now defs for /universal, /native and /konva.

I added konva and react-konva as devDeps because they're both needed for react-konva's types, but I didn't want to make them peerDeps. It's just to be able to check if the types were correct.

[drcmda]

@Kovensky Fantastic! Btw, since we moved the repo to its own GH org, we're looking for developers that don't shy away from digging in. You really seem to know your way around TS, and if you like, and would want to be part of it, i could add you to the team/org with maintainer/push rights.

Anyway, thanks so much!

[Jessidhia]

I'm unsure of how much time I can put into it (work was pretty busy the rest of this week), but sure, I'm up for helping!