Improve Type Declarations

[maxmellen]

This PR aims to improve the type declarations to more accurately reflect the documentation in the README as well as allow compilation in strict mode.

It also includes an updated package-lock.json file as a result of me simply running npm install. The only differences in that file seem to be values used by newer versions of NPM.

[coveralls]

Pull Request Test Coverage Report for Build 241

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 99.683%

---

Totals
Change from base Build 237:	0.0%
Covered Lines:	584
Relevant Lines:	585

---

💛 - Coveralls

[maxmellen]

Edit: This PR used to only make lambda-api compatible with TypeScript's strict mode, but after noticing the existing type declarations weren't so accurate, I also included some improvements in this PR and updated the PR's description.

[stawecki]

This works great! Not sure why the old req/res were optional.

[jsepia]

I switched to this fork to get proper TypeScript support for route parameters. Works for me. ✅

[maxmellen]

@jeremydaly Any chance of seeing this merged?

[jalooc]

I think you can improve this even further by allowing mixing normal middleware with error-handling middleware, cause if I see correctly the library allows it. Something like this, but prettier of course ;P :

type AnyMiddleware = Middleware | ErrorHandlingMiddleware

use(path: string, ...middleware: Middleware[]): void;
use(paths: string[], ...middleware: Middleware[]): void;
use(...middleware: AnyMiddleware[]): void;

[jalooc]

This PR is essential to use this library with TS, it fixes incorrectly typed basic functionalities. Let's merge it!

[jalooc]

Suggested change

	export declare type ErrorHandlingMiddleware = (error: Error, req: Request, res: Response, next: ErrorHandlingMiddleware) => void;
	export declare type ErrorHandlingMiddleware = (error: Error, req: Request, res: Response, next: () => void) => void;

[jalooc]

Sorry for atomic CR comments, I'm finding things as I develop.

[jalooc]

https://github.com/jeremydaly/lambda-api#adding-additional-detail

Suggested change

	export declare type LoggerFunction = (message: string) => void;
	export declare type LoggerFunction = (message: string, additionalDetail?: any) => void;

[jeremydaly]

@jalooc, let me know when you think this is ready.

[jalooc]

I'm not TS-savvy enough to say if it'd be possible, but it would be very nice to have req typed in a smarter way, for use cases like this:

api.use(async req => {
  req.connection = await createConnection()
  next()
})

// ...

api.finally(req => {
  return req.connection.close() // Can I have the req.connection type automatically inferred?
})

Does anybody know if it's possible? If not automatically inferred, then maybe a generic type for api could be used? 🤔

[maxmellen]

@jalooc You'd want to have the req argument to be typed like T extends Response and then you would write your custom type extending Response adding the fields you need. You would then cast inside of the handler:

interface UserReq extends Response {
  userToken: string
}

api.use(req => {
  const userReq = req as UserReq // TypeScript accepts this cast without any warning, even in the strictest settings
  // do something with userReq.userToken...
})

@jalooc @jeremydaly, I'm happy to apply the suggestions, but can we get this merged in a state not so far from how this PR currently stands? It would still be an incremental improvement in the right direction.

Or of course feel free to take over this PR.

[jalooc]

@maxmellen , I haven't found any more problems so far, so if you're willing to accept my change suggestions that'd be great (they just follow the lambda-api working, no subjective changes). It's a green light from my side. Nice job!

[jalooc]

So what we're doing with the discrepancies I listed? Another PR?

[jeremydaly]

Yes, please create a new PR.

[jalooc]

Just as a reference for others reading this thread and having a similar question as I did:

I'm not TS-savvy enough to say if it'd be possible, but it would be very nice to have req typed in a smarter way, for use cases like this:

api.use(async req => {
  req.connection = await createConnection()
  next()
})

// ...

api.finally(req => {
  return req.connection.close() // Can I have the req.connection type automatically inferred?
})

Does anybody know if it's possible? If not automatically inferred, then maybe a generic type for api could be used? 🤔

@maxmellen The way you suggested is okay in the short run, but after a while of playing with TS, I found a lot of shortcomings. First of all, you'd quickly sink in the depth of overridden interfaces. What's worse, you'd also have to always remember to manually cast X as Y to get the right type and that usually means a lot of places in a standard lambda. Besides, using such overridden type would prove problematic when interconneting it with other functionalities from the 3rd party library that don't understand it, so some as unknown-type hacks would be necessary.

I found an imho better and the proper way to do it: Declaration merging or module augmentation more specifically.

Following the example from my question, this would solve it:

declare module 'lambda-api' {
  interface Request {
    connection: SomeType,
  }
}

and from now on you have a typable req.connection everywhere.