Skip to content

chore(parser): return correct type for safeParse in envelopes #3339

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Nov 22, 2024
Merged

chore(parser): return correct type for safeParse in envelopes #3339

merged 5 commits into from
Nov 22, 2024

Conversation

jharlow
Copy link
Contributor

@jharlow jharlow commented Nov 21, 2024 •
edited by am29d
Loading

Summary

Provides type inference on successful results of EventBridgeEnvelope.safeParse

Changes

  • Currently, the EventBridgeEnvelope.safeParse method only validates, it does not parse the data provided in the typing system.

  • Small fix utilizes existing typing generics that weren't being assigned previously.

Issue: closes #3340

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@jharlow jharlow requested a review from a team November 21, 2024 00:52
@jharlow jharlow requested a review from a team as a code owner November 21, 2024 00:52
@boring-cyborg boring-cyborg bot added the parser This item relates to the Parser Utility label Nov 21, 2024
@boring-cyborg boring-cyborg
Copy link

boring-cyborg bot commented Nov 21, 2024

Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need.
In the meantime, check out the #typescript channel on our Powertools for AWS Lambda Discord: Invite link

@pull-request-size pull-request-size bot added the size/S PR between 10-29 LOC label Nov 21, 2024
@jharlow
fix(parser): properly parse eventbridge envelope when using safeParse
de1c104 
Currently, the EventBridgeEnvelope.safeParse method only validates, it does not parse the data provided in the typing system.
Small fix utilizing existing typing generics that weren't being assigned.
@jharlow jharlow force-pushed the fix/event-bridge-safe-parse-infer branch from 931ba6f to de1c104 Compare November 21, 2024 00:59
@pull-request-size pull-request-size bot added size/XS PR between 0-9 LOC and removed size/S PR between 10-29 LOC labels Nov 21, 2024
@github-actions github-actions bot added the bug Something isn't working label Nov 21, 2024
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@github-actions github-actions bot added do-not-merge This item should not be merged need-issue This PR needs an issue before it can be reviewed/worked on further labels Nov 21, 2024
@am29d
Copy link
Contributor

am29d commented Nov 21, 2024

Hey @jharlow thanks for the PR! I have created the issue to track it #3340 and I also notices there are other envelopes (LambdaFunctionUrl, VpcLattice, VpcLatticeV2) that need this improvement. Could you also add them and we can quickly merge it.

Thanks!

@am29d am29d removed bug Something isn't working need-issue This PR needs an issue before it can be reviewed/worked on further labels Nov 21, 2024
@am29d am29d changed the title fix(parser): properly parse eventbridge envelope when using safeParse chore(parser): return correct type for safeParse in envelopes Nov 21, 2024
@am29d am29d self-requested a review November 21, 2024 11:17
@jharlow
Copy link
Contributor Author

jharlow commented Nov 22, 2024

Thanks @am29d, appreciate how quickly you set up that issue and responded to this. I'll make those changes and ping you!

@pull-request-size pull-request-size bot added size/S PR between 10-29 LOC and removed size/XS PR between 0-9 LOC labels Nov 22, 2024
@jharlow
Copy link
Contributor Author

jharlow commented Nov 22, 2024

@am29d, I've extended these changes to the envelopes you mentioned, thanks for pointing that out. I did a quick sanity check and couldn't find any other envelopes missing that inferred typing. Hope that's ok but happy to make any other changes you want to see!

dreamorosi
dreamorosi approved these changes Nov 22, 2024
View reviewed changes
Copy link
Contributor

@dreamorosi dreamorosi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR and for addressing the review comments!

@dreamorosi dreamorosi removed the do-not-merge This item should not be merged label Nov 22, 2024
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@dreamorosi dreamorosi merged commit 471b3cf into aws-powertools:main Nov 22, 2024
37 checks passed
@boring-cyborg boring-cyborg
Copy link

boring-cyborg bot commented Nov 22, 2024

Awesome work, congrats on your first merged pull request and thank you for helping improve everyone's experience!

@github-actions GitHub Actions
Copy link
Contributor

@aws-powertools/lambda-typescript No related issues found. Please ensure 'pending-release' label is applied before releasing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dreamorosi dreamorosi dreamorosi approved these changes

@am29d am29d Awaiting requested review from am29d

Assignees

@jharlow jharlow

Labels
parser This item relates to the Parser Utility size/S PR between 10-29 LOC
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Maintenance: improve return type for safeParse in envelopes
3 participants
@jharlow @am29d @dreamorosi