React-spring: Typescript, desperately looking for help (maintenance or full source conversion)

Created on 8 Feb 2019  路  20Comments  路  Source: pmndrs/react-spring

TS has been a major trouble for me, i don't know it, and as a lib author i feel like i'm either being forced to learn it, for which i have no time right now, or rely on contributions, for which i'm super thankful, but they can be a step behind as contributors come and go, there's no one right now i could ask to look over a release.

If you know TS well and use this lib, would you want to be part of the react-spring org with access rights, to either convert the codebase, or maintain types? Or maybe some professional entity that uses this lib in prod wants to contribute - i'm open to all suggestions. It just has to do better. 馃槆

enhancement help wanted typescript

Most helpful comment

I can have a look, and maybe @Jessidhia can help review PRs? Do you have any major changes in a branch that I should base changes off, or should I work from master?

I figure it should happen in the following steps, integrated continously:

  1. Add babel preset and make sure everything builds
  2. Start converting files one by one. This might encounter bugs and require changes that should be reviewed and released separately.
  3. After everything is converted, replace the current type definitions with generated ones.

This way the work is also not 100% depending on a single person's work in a huge chunk.

All 20 comments

I can have a look, and maybe @Jessidhia can help review PRs? Do you have any major changes in a branch that I should base changes off, or should I work from master?

I figure it should happen in the following steps, integrated continously:

  1. Add babel preset and make sure everything builds
  2. Start converting files one by one. This might encounter bugs and require changes that should be reviewed and released separately.
  3. After everything is converted, replace the current type definitions with generated ones.

This way the work is also not 100% depending on a single person's work in a huge chunk.

Jessica is a master of typescript but i think she's super busy atm. I agree with the plan you've laid out.

I've been working with TS for a few years now. While not a master as Jessica I'd be willing to dedicate a bunch of hours every week to help out with the conversion/maintenance/reviewing PRs.

@jacobrask master is fine, there are no outstanding changes

@drcmda I'm working with TS for an year and like @stnwk I want help OSS project and improve my knowledge with TS 馃

awesome, if you like, you can get going, it's all there :D I would suggest only touching hooks though. I am still unsure how i can share code between renderprops and hooks, without taking functionality away.

@drcmda @jacobrask I took a look at the hooks code and saw that it uses a few files from the shared folder, what if we start by converting the files in there? None of it is public but would be nice if we do it first before moving on to the actual hooks.

So far I've added types to the interpolation functions in #528, and I have some work on the helper functions in a local branch.

I think the next step is to figure out the relationships between the Animated classes and where each property should be defined. Animated and AnimatedWithChildren should probably be abstract classes. To understand the types it's worth looking at the animated repo which has almost identical code but typed in Flow.

Should we get a chat going somewhere?

@jacobrask do you twitter? that would be a way. when i started with the lib i used animated as-is, then tried to remove some of the complexity we don't need. the base classes and the interpolation are the left overs. i changed them only a little to make them smaller but i think we can re-use the flow types.

We also need tests for the typescript types. 7.2.11 shipped with broken types 馃槩

../node_modules/react-spring/universal.d.ts:234:11 - error TS2430: Interface 'TrailProps<TItem, DS>' incorrectly extends interface 'SpringProps<{}>'.
  Types of property 'to' are incompatible.
    Type 'DS | undefined' is not assignable to type '{}'.
      Type 'undefined' is not assignable to type '{}'.

234 interface TrailProps<TItem, DS extends object = {}> extends SpringProps {
              ~~~~~~~~~~

I'd be well up for converting the lib to TypeScript. It would be nice to know how to coordinate so we don't duplicate efforts though; I don't really know where to start that isn't stepping on peoples' toes.

Perhaps someone could do the initial work of chopping up the job into semi-discrete chunks, and chuck 'em on a Github project so that people can assign themselves?

@janbaykara best you talk to @jacobrask he's already transformed some parts of it.

@janbaykara I've been working on better types for the interpolation and animation wrapper so far, so if you start working on converting any of the hooks that would be great. All my work in progress is in the springvalue-interface branch, which is basically done except that I need to fix CreateAnimatedComponent, currently it makes all properties of wrapped components required.

The idea is that the hooks should return SpringValue instead of OpaqueInterpolation.

By the way, I think it's a good ideat to keep types that are specifically there to improve the development experience for library consumers (extending CSSProperties, overloads, etc) out of the implementation files. This reduces the complexity and improves the DX for internal developers, which helps everyone but especially if you have limited TypeScript experience.

useSpring.ts for instance should only contain the "last" function declaration to satisfy all function overloads in an interface which is declared elsewhere (src/types). This way it's clear where you should look when you're fixing a typing issue, and also reduces the risk of conflict with other developers.

If you're going to write in typescript it's definitely better to use the typescript itself as the source of truth instead. But unfortunately that's not possible while using --allowJs...

I agree to eventually use TypeScript as source of truth for the generated type definitions, just that some aspects of the types that are not relevant to validating the correctness of the implementation can be kept separate.

When it comes to the values that are exposed by the API:s (currently they can be instances of AnimatedValue, AnimatedArray or AnimatedInterpolation) I decided to create a separate interface for the public API and make sure those classes implement that interface. The main reason is that no tool I've found to generate type bundles seems to both a) work, b) be able to strip internal methods, and we definitely don't want to expose implementation details and internal methods in public types.

Long term it would be nicer if these classes instead had their own base class (now they share it with classes that are never exposed in the API) with some methods marked @internal, but it requires some refactorings and improved tooling.

Note: I've reworked the typedefs for v9. It would be great if anyone in this thread could give them a test drive and provide feedback. Thanks!

npm install react-spring@next

@aleclarson I'm a total react-spring n00b, but I was trying to adapt https://codesandbox.io/embed/r5qmj8m6lq to typescript, and was running into really weird errors, then I installed react-spring@next, and they all went away. So, 馃憤 so far. ;)

Oh, I spoke slightly too soon. All my TS errors in vscode went away, but when I actually try to build my project I get:

node_modules/react-spring/lib/interpolation.d.ts(32,14): error TS2304: Cannot find name 'OneOrMore'.
node_modules/react-spring/lib/interpolation.d.ts(38,14): error TS2304: Cannot find name 'OneOrMore'.
node_modules/react-spring/lib/interpolation.d.ts(63,54): error TS2344: Type 'In' does not satisfy the constraint 'readonly any[]'.
node_modules/react-spring/web.d.ts(1,10): error TS2305: Module '".../node_modules/react-spring/lib/common"' has no exported member 'SpringValue'.

Thanks for the heads up! Similar comment here: https://github.com/react-spring/react-spring/issues/642#issuecomment-491309508

Was this page helpful?
0 / 5 - 0 ratings

Related issues

anton-g picture anton-g  路  3Comments

cmmartin picture cmmartin  路  3Comments

BertCh picture BertCh  路  3Comments

VincentCtr picture VincentCtr  路  3Comments

cklab picture cklab  路  4Comments