search

jeremydaly / lambda-api

Lightweight web framework for your serverless applications
https://serverless-api.com
MIT License
1.41k stars 125 forks source link

Define overloads for LoggerFunction in type definition #175

Closed Sleavely closed 1 year ago

Sleavely commented 3 years ago

I converted a route to Typescript and things went 💥

Turns out the type definition for the logger methods only expects a string.

This PR makes it behave in accordance with the README

coveralls commented 3 years ago

Pull Request Test Coverage Report for Build 289

Totals Coverage Status
Change from base Build 285: 0.0%
Covered Lines: 594
Relevant Lines: 595
💛 - Coveralls
brighttank commented 3 years ago

Is there an update on this?

Sleavely commented 3 years ago

Needs review/merge by @jeremydaly

naorpeled commented 1 year ago

Hey @Sleavely, thanks for your contribution, this issue was already resolved in a different PR that I've merged, in addition we allow several params and not only two so this code is not correct.

Again, thanks and sorry for the delayed response 🙏

Sleavely commented 1 year ago

No worries, I'm just glad the issue is being looked at :)

in addition we allow several params and not only two so this code is not correct.

From what I understand, the base logger supports a dynamic amount of arguments while the actual implementation in the Request object explicitly only accepts two?

https://github.com/jeremydaly/lambda-api/blob/85a8654e3592fde4436cc49919a1b2678a1a3af7/lib/request.js#L54

naorpeled commented 1 year ago

Hey @Sleavely, I've dug into this a bit further and you're correct, I misunderstood that part in the code, good catch.

I'll be opening a new PR that adds a test case for this and also fix a typo I've found in the doc. Will add you as a co-author, I hope that's okay with you, I want to release a new version today :)

Thank you very much!