Amplify-js: Typescript support for aws-amplify-react

[buggy]

Do you want to request a feature or report a bug?

Feature

What is the current behavior?

The aws-amplify-react package is written in Javascript instead of TypeScript which is used by aws-amplify and aws-amplify-angular. This prevents the TypeScript definitions from being automatically generated (see #305).

What I'm proposing is to convert the code base from Javascript to TypeScript so the definitions can be generated automatically. I think this process can be completed comfortably with 4 PR's:

1. Add webpack. This changes the build process while continuing to use Babel as the compiler.
2. Replace Babel with TypeScript. TypeScript will be configure to allow JS so there are no code changes.
3. Rename .js(x) files to .ts(x), ts-ignore any "errors", disable allowJs and enable type definition generation.
4. Remove the ts-ignore's and fix the underlying "errors".

There should probably be a gap between PR 3 and 4 to minimize the impact on any PR's that other people are working on.

[buggy]

@powerful23 I think you're the code owner for aws-amplify-react. I'm happy to work on the PR's but I realize you might have other plans so I want to check first.

[powerful23]

@buggy thanks but can we just add a index.d.ts file under that package instead of changing those js files like what amazon-cognito-identity-js does?

[buggy]

@powerful23 It's possible to write and maintain definitions independent of the code base but in my experience the definition files fall behind pretty quickly and never catch up. While converting the code base sounds daunting most of the work is in changing the build tools because TypeScript is a superset of Javascript so once that tools have been changed the rest is virtually just renaming file extensions. You then get the benefits of static types as well as the ability to automatically generate the .d.ts files

[ossareh]

@buggy is there not a 5th step of defining types and interfaces? Otherwise aren't most things typed as any? Is this what you mean when you say "fix the underlying errors"?

[buggy]

@ossareh Some interfaces and types will get fixed in step 4 but not all. My goal is to get it on par with aws-amplify which still makes heavy use of any but with a focus on having the public interfaces defined.

[powerful23]

@buggy That's a good point. But do we really need to use webpack, to generate a bundle? As I know we can generate by only tsc. Also I would prefer working on aws-amplify first for the typescript enhancement. @manueliglesias @mlabieniec what do you think?

[buggy]

@powerful23 aws-amplify already uses TypeScript so the hard part is done there. The rest can be achieved by slowly making the tsconfig more strict. I'm happy to skip webpack with aws-amplify-react. I only suggested it to bring the build process into line with aws-amplify.

[ossareh]

as an invested aws-amplify-react-native user I'd love to see these changes also occur over there. I'm happy to help in making this happen.

[powerful23]

@buggy great! I think that's a good start. Really appreciate if you could take a lead on this.

[buggy]

@powerful23 The final TypeScription conversion PR is almost ready. PR #2142 and #2143 should be merged first. I expect to have a WIP PR in the next 1-2 days with the final PR later this week.

[samuelcastro]

@buggy Was this released already?

[buggy]

@samuelcastro The early PR's have been merged. I need to work out why my local build is failing on circle CI so the PR to rename the files can be merged. Then I'll quickly copy across the interfaces definitions.

[samuelcastro]

ok great, thanks for the update @buggy

[awjreynolds]

@buggy Really great work. Do you know when this is being merged in? I am currently fighting Typescript trying to implement a custom sign in and I can leave this for a few days if there is a chance this will become available.

[buggy]

@awjreynolds Not I don't. I'll do my best to keep it current but when it gets merged is it entirely up to AWS.

[AndrewMorsillo]

@buggy @powerful23 Is there anything I can do to help un-stall this? It would be really nice to have typings for this stuff!

[buggy]

@AndrewMorsillo There's a PR ready to go. It needs to be reviewed and merged by someone from AWS. The only thing I can do is continue updating the PR so that it will merge smoothly once they're ready.

[yoDon]

@buggy thanks for all the work you are doing to keep this PR current - I'm not sure if AWS realizes the extent to which they are shaking people's confidence in their stewardship of and commitment to amplify-react by completely ignoring this incredibly valuable PR. It's almost a year old - any chance we can get some kind of update here, @powerful23?

[awhitford]

Does the PR also add typescript to the aws-amplify-react-native module? (Because I'd like that too.)

[buggy]

@yoDon The issue is 10 months old but due to the complexity I broke it into a number of PR. All of the early PR have been merged by @powerful23 (who has been providing feedback along the way).

The final PR was only created at the start of January and it required some changes based on feedback from multiple AWS staff. It did get "lost" for about 2 months but that's as much my fault because I was too busy to work on or push the PR. I resubmitted it 2-3 weeks ago and AWS is reviewing it now.

I would also point out that the final PR touches almost every file in the aws-amplify-react package (source, tests, snapshots, etc) so it may take a while for everyone to comfortable with merging it. I'm perfectly fine with that because I know this needs to be done right.

[buggy]

@awhitford This PR only covers aws-amplify-react and not aws-amplify-react-native.

[BarrySkidmore]

+1 Looking for this to get merged.

[buggy]

My apologies to everyone who was waiting for the PR to get merged. It's been 6 months since it was written and many months since I full tested it. I've decided to close the PR rather then keep it on life support. I may re-attempt it later if I have time.

[sammartinez]

Resolving as this now done. Thanks for working with us so much on it @buggy !