Add typescript definitions for built-in middlewares

[Juanelorganelo]

This pull requests adds typings for the middlewares public API. The file is in the root due to how typescript module resolution works for typings. For more information see this thread

Closes #112

[codecov]

Codecov Report

Merging #117 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff          @@
##           master   #117   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          17     17           
  Lines         347    347           
  Branches       71     71           
=====================================
  Hits          347    347

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9880fcb...a40aec8. Read the comment docs.

[lmammino]

This is brilliant, thanks @Juanelorganelo!

Just one question before moving this on:

• Shouldn't we change this line in the package.json as well: https://github.com/middyjs/middy/blob/master/package.json#L13

Also, can we move the file in the typescript folder?

[Juanelorganelo]

@lmammino I tried every possible way I could think of to keep the middlewares.d.ts file in the typescript folder. Apparently this haves to do with how typescript resolves imports to submodules so we can do import * as middlewares from 'middy/middlewares'. Thats why I left the file in the root and I didn't change the typings property of package.json. I could put a postinstall script to move the file to the root, but as mentioned in the thread its kind of hacky and it doesn't work with npm link. Any feedback will be appreciated

[lmammino]

Sorry if this is taking too long, I have been very busy lately and I don't know enough about Typescript. It would be great to have some support by @joseSantacruz on this.

I'll try to play with this PR a bit and see if I can figure it out if we can avoid to keep the typescript file in the root folder

[lmammino]

@Juanelorganelo, I did some changes and my editor autocomplete seems to like the definitions now, but I am not sure I am providing types in the right way. Can you double check?

Also, it would be great to have some feedback from 2 typescript masters I know like @remojansen and @fsciuti 😇

I look forward to hearing from all of you guys, thanks in advance!

[joseSantacruz]

Excellent improvement adding the middlewares typings

[joseSantacruz]

I think is better if you use generics and the latest function type declaration so this interface should be:

interface ICacheOptions {
    calculateCacheId?<T>(event: T): Promise<string>;
    getValue?<T>(key: string): Promise<T>;
    setValue?(key: string): Promise<void>;
}

[joseSantacruz]

I think is better if you use generics and the latest function type declaration so this interface should be:

interface IWarmupOptions {
    isWarmingUp?<T>(event: T): boolean;
    onWarmup?<T>(event: T): void;
}

[joseSantacruz]

this is not going to works I tested and give me types error if you use import * as middy from 'middy';, and also remember that the default export is deprecated in favor of module imports. So should stay without default

[remojansen]

I had problems with this before, not sure if it is the same issues but the following helped me microsoft/TypeScript#5073

[joseSantacruz]

Hi @lmammino and @Juanelorganelo I put some comments on the PR, hope this help to improve this new feature

[remojansen]

It looks good but I would recommend adding a few test files under the typescript folder, like for example types.test.ts. This file should use the type definitions and then you can compile as them as part of the build. You can find a sample PR at https://github.com/godaddy/terminus/pull/4/files

[Juanelorganelo]

@lmammino Sorry for the delay, I've been really busy at work, anyway I just tested this with the remapped exports and import * as middlewares from "middy/middlewares" fails to compile with the strict flag :'(. I think the solution for this issue is either have like a dist directory with index.js, index.d.ts, middlewares.js and middlewares.d.ts, move the typescript defs to the root dir or make a postinstall script that moves the definitions afterwards. @remojansen also testing the types seems like a great idea, I'll add the tests as soon as we figure out the directory structure. Thank you all for your awesomeness. 👍

[lmammino]

Thanks @joseSantacruz and @remojansen! Your support is really really appreciated :)

@Juanelorganelo If it gets too tricky to have the typescript definitions inside the typescript folder I am ok for moving them back in the same directory of their related JS file.

[lmammino]

Thanks all for your valuable contribution on this!

@Juanelorganelo @joseSantacruz @remojansen 👍 🍻