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

Feature Request: Typescript Support #26

Closed
thupi opened this issue Apr 12, 2018 · 46 comments
Closed

Feature Request: Typescript Support #26

thupi opened this issue Apr 12, 2018 · 46 comments
Labels
kind: request New feature or request that should be actioned type: good first issue Good for newcomers

Comments

@thupi
Copy link

thupi commented Apr 12, 2018

Hi,

This is kind of ground breaking. I think this is er very easy and well thought abstraction for animating stuff based on state. I will definitely take a closer look into this tool :-)

My biggest problem(which is kind of small) at the moment is that there aren't any typings to find for this package yet. I would really love to see some typings for this libary as it would make the whole API easier to get into and make it way easier to use in Typescript projects :-)

@thupi
Copy link
Author

thupi commented Apr 12, 2018

I have a hard time finding time to write a good and complete type definition for this project soon but i will definitely let you know if or when i have the time to go through the type definition :-)

This might be useful in order to write the typings :-) https://github.com/drcmda/react-spring/blob/master/API.md

@drcmda
Copy link
Member

drcmda commented Apr 13, 2018

I welcome PRs. I have little experience in TS myself and found it hard to wrap my head around. If you find the time to create typings, that would be awesome. 😀

@drcmda drcmda added kind: request New feature or request that should be actioned type: good first issue Good for newcomers labels Apr 16, 2018
@Cryrivers
Copy link

I'm currently evaluating the transition from react-motion to react-spring in our products, might have time to make the type definition.

@nelsliu9121
Copy link

Should this issue be reopened? Due to the fact that the typing is only half finished, and ts compiler will scream when you use components other than Spring.

@drcmda
Copy link
Member

drcmda commented Apr 26, 2018

Sure.

@drcmda drcmda reopened this Apr 26, 2018
@Cryrivers
Copy link

@nelsliu9121 PR welcomed!

@radudascaluntt
Copy link
Contributor

radudascaluntt commented May 8, 2018

Hey everyone,

I've tried to add the missing types. I'm relatively new to TypeScript and to react-spring thus I'm sure I did something wrong. Basically I was just copy & pasted the stuff I have found in the initial index.d.ts.

It would be great if I can get some advice/help on how to test the annotations before I do a pull request.

@Cryrivers
Copy link

@radudascaluntt no worries, you can just create a PR!

@radudascaluntt
Copy link
Contributor

Will make the PR from another account. I forgot I was using this one yesterday :)

@crimx
Copy link
Contributor

crimx commented May 12, 2018

Hi @radudascaluntt I wrote the current Spring typing. Didn't write others because I haven't got a change to use them yet.

You can use tsc or vscode to test the syntax. It's a bit hard to test integration because the project itself doesn't have testing yet.

I first edited the index.d.ts directly within my own project to make sure it worked. And then eyeballed the source for places I wasn't quite sure about.

Most of the time you just need to follow the propTypes.

Hope this helps.

@raspo
Copy link

raspo commented May 14, 2018

I would imagine that keeping the declaration file up to date is going to become tedious in the long run...
Have you considered using something like this https://github.com/Microsoft/dts-gen?

@swashata
Copy link

For the time being I've found that adding the path to jsconfig.js file makes vscode provide some intellisense

{
  "paths": {
  // ...
    "react-spring": ["./node_modules/react-spring/src/index.js"], // Not ready yet
  },
  // ...
}

@drcmda
Copy link
Member

drcmda commented May 28, 2018

@raspo could you make a PR and add it to the build? TS support is supper spotty it seems and the current PRs have all fallen asleep.

@ticruz38
Copy link

ticruz38 commented Jun 4, 2018

Providing only the Spring component in typings.d.ts is not a good idea.
VSCode then thinks the library does have a typings file (which is incomplete) and trigger errors when importing any other component such as Transition.
Either update the typings or remove it completely.

@jariz
Copy link

jariz commented Jun 5, 2018

...adding to @ticruz38's comment,
@drcmda I would just merge #84, it probably still misses some things, but it's better than having typings for only 1 out of 5 components, which is the state we're in now.

@drcmda
Copy link
Member

drcmda commented Jun 5, 2018

@jariz thanks, very happy about getting help here - i'll merge it

@simonbuchan
Copy link

I've been working on filling out typings for this, but I've ran into an issue where typescript stops being able to infer the render argument type as soon as native is supported. Simplifying:

declare class AnimatedValue<T> {
  interpolate<U>(map: (value: T) => U): AnimatedValue<U>;
}

type AnimatedValueMap<T> = { [N in keyof T]: AnimatedValue<T[N]>; };

declare const animated: // distractingly big, maps JSX.IntrinsicElements to ComponentType<> with props that take AnimatedValue

type RenderPropsBase<T> = {
  render: (value: T) => React.ReactNode;
}; // | { children: ... }, etc...

type RenderProps<T> =
  | RenderPropsBase<T> & { native?: false }
  | RenderPropsBase<AnimatedValueMap<T>> & { native: true }
  ;

type TransitionProps<T> = RenderProps<T> & { from?: T; enter?: T; leave?: T; };

declare class Transition<T> extends React.PureComponent<TransitionProps<T>> { }

const dynamicRenderElement = (
  <Transition
    from={{ x: 0 }}
    enter={{ x: 100 }}
    render={props => ( // Parameter 'props' implicitly has an 'any' type.
      <div
        style={{ left: props.x }}
      />
    )}
  />
)

const nativeRenderElement = (
  <Transition
    native
    from={{ x: 0 }}
    enter={{ x: 100 }}
    render={props => ( // Parameter 'props' implicitly has an 'any' type.
      <animated.div
        style={props.x.interpolate(left => ({ left }))}
      />
    )}
  />
)

If either option in RenderProps<T> is commented out, the other one works with full inference, so this is pretty annoying, otherwise you have to annotate the props types on both sides as : { x: number } or : { x : AnimatedValue<number> } (or in general, : T and : AnimatedValueMap<T>).

The existing typings seem to "solve" this by removing any type association between the provided values (from, enter) and the render / children argument, meaning you don't have any type checking on the values, or inference on the args, but I haven't double-checked this.

I'll keep poking at this, but would it make sense from the JS side to offer, e.g. <native.Transition /> as a friendlier to typescript option to <Transition native />?

@drcmda
Copy link
Member

drcmda commented Jun 11, 2018

@simonbuchan i see, but is there no other way? Can't props be either an object of values and strings or an object of animatedValues? This would cause some confusion otherwise. I wanted the api to stay simple, where "native" wouldn't be that different from the default. Making additional forward-components isn't the problem:

const NativeSpring = props => class extends Component {
  static propTypes = {
    // native type declarations here
  }
  render() {
    return <Spring native {...props} />
  }
} 

But it would create unnecessary overhead.

PS. you only need to interpolate values if you change their structure, your example interpolates x, but it could simply be passed as always:

render={props => (
  <animated.div style={{ left: props.x }} />
)}

@simonbuchan
Copy link

simonbuchan commented Jun 11, 2018

Nope, then you can't use props.foo.interpolate(), because props.foo might be number instead of AnimatedValue<number>, and you can't assign props.x to left, because animated.div's left is AnimatedValue<number> and props.x might be a number, and vice-versa for non-animated div.

Regarding style, that was due to the omitted animated typings, so that's relevant: is style specialized, or do animated components deep inspect all props with object values for AnimatedValue? Or is only style animatable?

@drcmda
Copy link
Member

drcmda commented Jun 11, 2018

It depends, web (the default) can animate element styles and attributes (svg paths would be attributes, for instance: <path d={animatedPath} />). On other targets it could be props, for instance react-konva: https://codesandbox.io/embed/812jvploqj Eventually it's the target-renderer that defines these things.

@simonbuchan
Copy link

Right, animated will get a little more scary then :)

@drcmda
Copy link
Member

drcmda commented Jun 11, 2018

Yeah 😱Does TS run in CodeSandBox? If we re-create your snippet above to demonstrate and isolate the problem, maybe it helps showing it around to get some input.

If nothing helps we could make a typescript target with forward components, it would be easy to define, the build chain is already equipped for it. Then you would have two components for each primitive, Spring and NativeSpring, etc. each with correct types. You'd import it like so:

import { NativeSpring } from 'react-spring/dist/web-ts'

Or something like that. At least it wouldn't disturb plain javascript users, though i would have no idea how to document that without causing one hell of confusion.

@simonbuchan
Copy link

Since it's just a typing issue, the standard is the playground for repro issues, e.g. a TS bug report, but I would like to reduce it a bit more first.

What I've got so far now implies that TS doesn't know how to contextually type the JSX props from the component signature, and it works without the union since the non-contextual inference still works, but that doesn't seem to make sense with the parameter being contextually typed. 😵

I could throw up what I've got tomorrow (9:00 PM here 😉) for this side, though, if anyone wants to give it a stab.

@kuuup-at-work
Copy link

@simonbuchan

Nope, then you can't use props.foo.interpolate(), because props.foo might be number instead of AnimatedValue, and you can't assign props.x to left, because animated.div's left is AnimatedValue and props.x might be a number, and vice-versa for non-animated div.

You must check props.foo with typeof during runtime and then cast it to number or AnimatedValue<number>.

@drcmda
Copy link
Member

drcmda commented Jun 12, 2018

@simonbuchan i'm a total beginner when it comes to TS, but does that mean it can be solved without having to duplicate all primitives (NativeSpring, etc.) ?

@simonbuchan
Copy link

I mean, what I've got will work fine so long as you provide the type for the render argument, the win over the current types being that it can check if the values are compatible - but that's not really up to my expectations (you shouldn't have to read docs on how to use the typings for a library). Typescript already has all the information to infer everything itself, and can figure out similar, but simpler cases like in the example above, so I think this is either just a bug, or an improvement to TS they would be happy to add.

The suggestion for <native.Foo> would mean it wouldn't have to wait for that, though, (and I kind of like the symmetry with <animated.foo>), but you should in general only be changing libraries to fit typescript if it actually makes the library better even without types, which this isn't clearly so - one reason you might break the native types out for is if it means dead code elimination works better, for example?

@drcmda
Copy link
Member

drcmda commented Jun 12, 2018

The additional payload for native and animated.element isn't that big. If size is really an issue there would be more efficient ways, for instance react-spring/dist/universal (doesn't contain native, no colors) or react-spring-numerical (can be native, but no interpolations, just numbers => 2kb).

@msuntharesan
Copy link

msuntharesan commented Jun 15, 2018

I see that in TransitionProps, keys property is not marked as optional even though the comment says it can be ommited if children/render aren't array. This is causing type error (Property 'keys' is missing in type)
Edit: This is happening in current version installed from npm.

@ghost
Copy link

ghost commented Jun 18, 2018

Thanks for the work on the missing type definitions. The imperative API still seems to be missing types.

@will-stone
Copy link

I presume this is related to this ticket, TSLint is giving me this error for Transition and Spring components:

[ts] JSX element type 'Transition' does not have any construct or call signatures.
(alias) class Transition<S extends object, DS extends object>
import Transition

@will-stone
Copy link

Scratch that. All I had to do was upgrade the React and ReactDOM types to

"@types/react": "^16.4.6",
"@types/react-dom": "^16.0.6",

@baktun14
Copy link

baktun14 commented Jul 9, 2018

Hi, I was trying to reproduce an example with typescript and realized there are missing typings for the Keyframes.
image

@drcmda
Copy link
Member

drcmda commented Jul 11, 2018

@mbeauchamp7 looks like it's an old definition. Do you know your way around TS? I welcome PR's.

There are 3 statics for Spring, Trail & Transition (+ static create for custom primitives), script is already gone, and the component takes 1 prop: state.

The 3 statics accept either a function or an object

https://github.com/drcmda/react-spring/blob/master/src/Keyframes.js#L80-L90

@baktun14
Copy link

@drcmda I'm not very good with definition files but I can give it a try!

@drcmda
Copy link
Member

drcmda commented Jul 12, 2018

Someone just updated typing, i'll try to get some more fixes in and will release soon.

@lasseborly
Copy link

lasseborly commented Jul 20, 2018

Very cool @drcmda !

Full typings would be so cool.
I still get weird errors on Transition though.. Which does seem to have typings?

@baktun14
Copy link

baktun14 commented Jul 20, 2018

@lasseborly I think they have been updated, but not released on npm

@drcmda
Copy link
Member

drcmda commented Jul 20, 2018

Right, i just updated ...

@lasseborly
Copy link

lasseborly commented Jul 20, 2018

Ok.. I think I have managed to track my problem down. Limited experience with both react-spring and Typescript might make me come across as ignorant. But here we go.

If I try to go by the docs and make use of Transition as described I run into some problems.

  <Transition
    from={{ opacity: 0 }}
    enter={{ opacity: 1 }}
    leave={{ opacity: 0 }}
  >
    {loaded && Welcome}
  </Transition>

Usage like that resolves in an error: TypeError: Cannot add property props, object is not extensible. Looking at the proptypes supplied and at other code makes it clear that children and render only accepts a function or an array of functions.
So doing that works fine:

  <Transition
    from={{ opacity: 0 }}
    enter={{ opacity: 1 }}
    leave={{ opacity: 0 }}
  >
    {loaded &&
      (styles => (
        <Welcome style={styles}>Hello world {styles.opacity}</Welcome>
      ))}
  </Transition>

Until we venture into Typescript land where the interface of the Transition prop types are exactly like their prop-types counterparts. A function or an array of functions.
Which means that it bugs out when it sees loaded which is a boolean.
Adding boolean as part of the Transition props interface solves the problem.

If I'm not misunderstanding the problem and I'm not doing something fundamentally wrong you guys can correct me. If not a proposal for a PR I'll gladly produce sounds as follow.

  • Rewrite the parts of the documentation that are faulty or misleading.
  • Extend the Transition interface definition to accept a booleanvalue.
  • Extend the Transition prop type to accept a boolean value. Primarily for parity with the TS interface definition.

Again, I'll gladly put up the PR. Really love this library and would be grateful for the opportunity to contribute. Even if it's "only" typings. 😊

@drcmda
Copy link
Member

drcmda commented Oct 26, 2018

Could anyone help fixing types for 6.0? Transition and Trail have changed a little. They accept a single function child: (item, state, index) => props => ReactNode

item is the individual item, transiton and trail are driven by the items prop

state can be "enter"|"update"|"leave"

index is the index of the item

http://react-spring.surge.sh/transition

Right now the types declare SpringRendererFunc, which is wrong i guess ...

@borisyordanov
Copy link

borisyordanov commented Nov 21, 2018

Has anyone managed to use react-spring with typescript?

I tried doing this:

import { config, Transition } from 'react-spring'
const AnimatedRoutes = (props: RouteComponentProps) => (
    <Transition
        config={config.slow}
        from={{ transform: 'translateY(100px)', opacity: 0 }}
        enter={{ transform: 'translateY(0px)', opacity: 1 }}
        leave={{ transform: 'translateY(100px)', opacity: 0 }}
    >
        <Switch location={props.location}>
            <Route path="/" exact={true} component={LandingPage} />
            <Route path="/about" component={HowWorks} />
            <Route path="/1" component={Step1} />
            <Route path="/2" component={Step2} />
            <Route path="/3" component={Step3} />
            <Route path="/4" component={Step4} />
            <Route path="/5" component={Step5} />
        </Switch>
    </Transition>
);
class App extends React.Component {
    public render() {
        return (
            <div className="App">
                <ApolloProvider client={client}>
                    <ReduxProvider store={store}>
                        <Router>
                            <Route render={AnimatedRoutes} />
                        </Router>
                    </ReduxProvider>
                </ApolloProvider>
            </div>
        );
    }
}

and got this error:

(27,6): Type '{ children: Element; config: SpringConfig; from: { transform: string; opacity: number; }; enter: { transform: string; opacity: number; }; leave: { transform: string; opacity: number; }; }' is not assignable to type '(IntrinsicAttributes & IntrinsicClassAttributes<Transition<{}, {}, { transform: string; opacity: number; }, { transform: string; opacity: number; }, { transform: string; opacity: number; }, {}>> & Readonly<...> & Readonly<...>) |
(IntrinsicAttributes & ... 2 more ... & Readonly<...>)'.
  Type '{ children: Element; config: SpringConfig; from: { transform: string; opacity: number; }; enter: { transform: string; opacity: number; }; leave: { transform: string; opacity: number; }; }' is not assignable to type 'Readonly<TransitionProps<{}, {}, { transform: string; opacity: number; }, { transform: string; opacity: number; }, { transform: string; opacity: number; }, {}>>'.
    Property 'items' is missing in type '{ children: Element; config: SpringConfig; from: { transform: string; opacity: number; }; enter: { transform: string; opacity: number; }; leave: { transform: string; opacity: number; }; }'.

Why is items required? The type definitions seem to be totally off. What am i missing? (i'm using v 6.4.1 or react-slick)

@pollen8
Copy link

pollen8 commented Nov 22, 2018

items is a required Transition prop, so add

 <Transition
  items={[1]}

Also Transition`s child needs to be a function, see the docs http://react-spring.surge.sh/transition

@borisyordanov
Copy link

borisyordanov commented Nov 22, 2018

@pollen8 Thanks for the help! You pointed me in the right direction (with the help of this comment) and i resolved the type errors, but atm the route changes are not animated. Any idea why?

Here's what i ended up with:


const SpringSwitch = ({ location, children }: { location: any; children?: any }) => {
    return (
        <Transition
            // native={true}
            items={[1]}
            keys={location.pathname}
            config={config.slow}
            from={{ transform: 'translateY(100px)', opacity: 0 }}
            enter={{ transform: 'translateY(0px)', opacity: 1 }}
            leave={{ transform: 'translateY(100px)', opacity: 0 }}
        >
            // this returns a function because (item:any)=>SpringRendererFunc<{}> is the expected child of Transition
            {(item: any) => () => (
                <Switch>
                    {React.Children.map(children, (child: any) => {
                        const { component, ...childProps } = child.props;
                        const AnimatedComponent = animated(component);
                        return React.createElement(child.type, {
                            ...childProps,
                            render: (props: any) => <AnimatedComponent {...{ ...props, item }} />
                        });
                    })}
                </Switch>
            )}
        </Transition>
    );
};

const AnimatedSwitch = withRouter(SpringSwitch);
class App extends React.Component {
    public render() {
        return (
            <ApolloProvider client={client}>
                <ReduxProvider store={store}>
                    <Router>
                        <AnimatedSwitch>
                            <Route path="/" exact={true} component={LandingPage} />
                            <Route path="/how-it-works" component={HowWorks} />
                            <Route path="/step1" component={Step1} />
                            <Route path="/step2" component={Step2} />
                            <Route path="/step3" component={Step3} />
                            <Route path="/step4" component={Step4} />
                            <Route path="/step5" component={Step5} />
                        </AnimatedSwitch>
                    </Router>
                </ReduxProvider>
            </ApolloProvider>
        );
    }
}

Not only the route transitions not animated, but CSS animations i have don't work. Scrolling is super laggy and when i open one of my routes i get this error:

Function components cannot be given refs. Attempts to access this ref will fail.
Check the render method of `AnimatedComponent`.

And i'm not using refs anywhere in any of the components on that route.

@canastro
Copy link

canastro commented Mar 2, 2019

With the latest version of react-spring (with hooks) and using react-konva:

import { useSpring, animated } from 'react-spring/konva';

I get the following error:

 TS7016: Could not find a declaration file for module 'react-spring/konva'. 
'/.../node_modules/react-spring/konva.js' implicitly has an 'any' type.

Any tip on how I can fix this?

@drcmda
Copy link
Member

drcmda commented Mar 3, 2019

The module exists: https://unpkg.com/react-spring@8.0.10/konva.js

But konva doesn't have types.

@pmndrs pmndrs locked as resolved and limited conversation to collaborators Apr 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind: request New feature or request that should be actioned type: good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests