exvcr
exvcr copied to clipboard
Use Jason Instead of JSX
Continues from https://github.com/parroty/exvcr/pull/179
- [x] rebase
- [x] minimal changes wrt
master
👋 @parroty
I wonder what you think about using Jason
instead of jsx
This looks good @ruslandoga , maybe @parroty can merge it ? 👍
Thank you for the PR 🙇 . Do you have any insights around compatibility implications with this change? I am just wondering if there's any corner cases that can break by changing the JSON engine 🤔 .
👋 @parroty
Yes, it very well might break something since jsx
and Jason
have different defaults. For example, the previous attempt to use Jason
was blocked by Jason
being more strict and not encoding invalid bytes like jsx
https://github.com/parroty/exvcr/pull/179#discussion_r702496892
There might be other differences that weren't uncovered by the tests.
The breakage on certain conditions are concerning part of this change, though I understand the concern mentioned in the previous issue. Describe the procedure (for re-recording?), or something...? 🤔
Also https://github.com/parroty/exvcr/issues/153 lists some concerns people have about depending on jsx
through exjsx
I'll try out this branch (exvcr + jason) tomorrow on https://github.com/plausible/analytics and report back if anything breaks.
In Plausible all VCR tests pass without any re-recording: https://github.com/ruslandoga/analytics/pull/147
$ mix test --include slow
13:54:33.063 [info] Migrations already up
13:54:33.091 [info] Migrations already up
Including tags: [:slow]
.............................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................
Finished in 123.2 seconds (16.4s async, 106.7s sync)
9 doctests, 1396 tests, 0 failures
I had to update Finch version since ExVCR mocks request!/2,3 functions which were not present in Finch 0.14.
Imho even though it might break things downstream, keeping this unsupported dependency would also be a reason for people to drop exvcr.
Thank you for the feedbacks / comments. I am thinking to merge for v0.15.0
(after releasing other ones as v0.14.x
). Please allow some time to test on my other libraries too.