Add support for more transactions, emit events #271
Conversation
|
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
infiniteflower
left a comment
infiniteflower
left a comment
There was a problem hiding this comment.
Just had some small comments
mcmire
left a comment
mcmire
left a comment
There was a problem hiding this comment.
Hello, I suspect there are plans here for events, but I've seen similar events in TransactionController as those you're adding here, so I'm curious how we intend to use them? Thanks.
|
|
||
| Object.entries(data).forEach(([uuid, stxStatus]) => { | ||
| const transactionHash = stxStatus?.minedHash; | ||
| this.eventEmitter.emit(`${uuid}:status`, stxStatus); |
There was a problem hiding this comment.
Where will this event be used if eventEmitter is private?
There was a problem hiding this comment.
The extension is able to use it. It's an easy way to see what is going on with an STX status in Console. @matthewwalsh0 actually wrote most of the code in this PR and I suppose he added the events module usage into the TransactionController, so he might elaborate on intended use a bit more.
More about the events node module here: https://www.npmjs.com/package/events
src/SmartTransactionsController.ts
Outdated
|
|
||
| private trackMetaMetricsEvent: any; | ||
|
|
||
| private eventEmitter: any; |
There was a problem hiding this comment.
Can we give this property a type? Perhaps:
| private eventEmitter: any; | |
| private eventEmitter: EventEmitter; |
Description
This PR does 2 things:
Testing