-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(replay): Add toHaveLastSentReplay
jest matcher
#6467
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.
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
Conversation
Really we renamed the previous `toHaveSentReplay` -> `toHaveLastSentReplay` and added `toHaveSentReplay` to match all calls to transport.
size-limit report 📦
|
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.
@mydea and I discussed custom Jest matchers with the rest our team, as we're not using them anywhere except for the Replay package. The general consense is that we'd like to continue it that way and if possible not add new matchers. In the long run, it'd be great to relace them in Replay as well for consistency.
Given however, that the Replay matchers are quite complex (at least to me), I wonder if it is reasonable to even convert them to e.g. helper functions we'd call in the respective tests. I'm not saying we can't merge this in but I'd be curious about your general opinion here. WDYT, is it reasonable to stop using matchers or do you want to continue using them?
expectedVal: SentReplayExpected[keyof SentReplayExpected]; | ||
actualVal: SentReplayExpected[keyof SentReplayExpected]; | ||
}; | ||
type Call = [ |
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.
nit: I'm wondering if "Call" is a good name here. Isn't this just an Envelope?
It's certainly possible to move the assertion logic to a helper function, but we'd lose out on some formatting when tests fail. Another possibility is to snapshot the call args. |
Really we renamed the previous
toHaveSentReplay
->toHaveLastSentReplay
and addedtoHaveSentReplay
to match all calls to transport.