aws-lambda: extend Handler return type to support Promises#22588
Merged
mhegazy merged 1 commit intoDefinitelyTyped:masterfrom Jan 3, 2018
Merged
aws-lambda: extend Handler return type to support Promises#22588mhegazy merged 1 commit intoDefinitelyTyped:masterfrom
mhegazy merged 1 commit intoDefinitelyTyped:masterfrom
Conversation
Contributor
|
@MichaelMarner @keita-nishimoto @daniel-cottone @OrthoDex @MichaelRBond @y13i @DanielRosenwasser @wwwy3y3 @sime @nexus-uw @coderbyheart can you please review. |
sime
approved these changes
Jan 3, 2018
Contributor
|
We've gotten sign-off from a reviewer 👏. A maintainer will soon review this PR and merge it if there are no issues. If it shouldn't be merged yet, please leave a comment saying so and we'll wait. Thank you for contributing to DefinitelyTyped! |
simonbuchan
added a commit
to simonbuchan/DefinitelyTyped
that referenced
this pull request
Feb 7, 2018
No tests added for the new types yet, I'll get on that tomorrow. Motivation --- The connection between the trigger event and the result types can be confusing sometimes - for example `CloudFormationCustomResourceResponse` is *not* returned from the lambda, but instead should be sent to the *Request `ResponseUrl`. This PR tries to help by providing pre-baked `Handler` types for each of the triggers (that have types already). There's also a few niggles with naming and the handler signature that have been bothering me for a while. Changes --- This PR: - Adds defaulted generic parameters to the existing `Handler` and `Callback` types. This increases the minimum TS version to 2.3. - Makes the `callback` parameter to `Handler`s "required" - this probably originally was meant to represent the Node 0.10 runtime, which did not pass a callback parameter, but that is no longer selectable when creating or editing a Lambda and makes the normal, recommended usage harder. In case anyone is confused, this doesn't break any code: it is required to be provided when being called, your handlers don't need to declare it. I'm not removing the legacy node 0.10 runtime methods `context.done()` etc., since they still work. - In the spirit of DefinitelyTyped#21874, make the default result type for `Handler`, `Callback` `any`, since the only requirement is that it be `JSON.stringify()`able, and there are things like `.toJSON()`, etc. so there is no meaningful type restriction here. - Revert the definition changes of DefinitelyTyped#22588, which changed the `Handler` result types to `void | Promise<void>`, which is misleading: Lambda will not accept or wait for a promise result (it by default waits for the node event loop to empty). This is not a breaking change for code that does return promises, since a `void` result type is compatible with any result type, even with strict function types. (Which is why the tests still pass.) - Adds `FooHandler` and `FooCallback` types for all `FooEvent`s and `FooResult`s, where possible - sometimes these don't match up. - Renamed, with aliases to the old name, various types to align them: in particular `ProxyResult` -> `APIGatewayProxyResult`. - `CloudFrontResult` (not the same type as `CloudFrontResponse`!) was missing for some reason.
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
npm run lint package-name(ortscif notslint.jsonis present).Select one of these and delete the others:
If changing an existing definition:
tslint.jsoncontaining{ "extends": "dtslint/dt.json" }.