Conversation
}) | ||
.on("error", err => subscriber.next(err)); | ||
}); | ||
const observable = new Subject(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the reason for changing to Subject? I seem to recall an earlier discussion that we had on changing the Subjects to Observables, since you couldn't push your own events to an Observable it was safer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need the Subject to be able to call .complete
on it outside of the function. I suppose you could assign it to a temp variable by using its closure.
@adrianmcli CI is failing |
For some reason, using Subject for This might cause problems down the road, but I'm not sure what we can do at this point. I'm not sure why using Subject causes it to hang. btw, when I say "hang", I mean getting the following at the end of successfully passing all tests:
Do note that this only happens on Travis, not locally. |
@honestbonsai I found this comment: jestjs/jest#1456 (comment) And I basically just set |
Hmm still broken, I'll take a closer look if you're busy. @adrianmcli |
@honestbonsai I triggered a rebuild and it passed. It just didn't update on github. |
Ok that works for me then |
Fixes #81