feat: Improving the Context Helper #9
Conversation
klcodanr
commented
Mar 21, 2023
klcodanr
commented
- Change to be class based
- add new methods to better support SQS and request/transaction IDs
…ods to better support SQS and request/transaction IDs
|
This PR will trigger a minor release when merged. |
src/context.js
Outdated
| * Gets the SQS records from the context | ||
| * @returns {Array<QueueRecord>} the SQS record payload | ||
| */ | ||
| extractSqsRecords() { |
There was a problem hiding this comment.
I worry about exposing details of the underlying execution platform. Why would any of our processes need to know whether they're running on SQS? That feels like a leak of implementation details that we shouldn't be relying on, and a strong coupling to SQS.
There was a problem hiding this comment.
Very true, let me refactor this so it doesn't expose SQS details.
There was a problem hiding this comment.
I kept the methods, but removed the SQS specifics from the name. Since the request structures differ, I think it still makes sense to have a method to see what "type" of request it is (e.g. standard HTTP vs Queue) because a HTTP request only contains 'event' but a queue request can contain many. The alternative would be to make the extractOriginalEvent method smarter and have it return an array or an item (or an array of one item), but that seems clunky.
| @@ -11,6 +11,7 @@ | |||
| */ | |||
|
|
|||
| export * as contextHelper from './context.js'; | |||
| export { ContextHelper } from './context.js'; | |||
There was a problem hiding this comment.
why are there two imports for context.js?
There was a problem hiding this comment.
Let me try moving those into one, I kept the old export so it would be backwards compatible and marked all the old methods as deprecated so we can upgrade this without immediately refactoring a bunch of code (esp tests)
There was a problem hiding this comment.
I'm open to be wrong here, but I don't see how to do this without two imports :-/
There was a problem hiding this comment.
:/ that is fine. Just add a comment that one is for backwards compatibility
Codecov Report
@@ Coverage Diff @@
## main #9 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 7 7
Lines 621 703 +82
=========================================
+ Hits 621 703 +82
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
# [1.2.0](v1.1.0...v1.2.0) (2023-03-22) ### Features * Improving the Context Helper ([#9](#9)) ([4ca0b02](4ca0b02))
|
🎉 This PR is included in version 1.2.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
