[jest-worker]: bigInt serialization in jest-worker#11624
[jest-worker]: bigInt serialization in jest-worker#11624sauravhiremath wants to merge 6 commits into
Conversation
sauravhiremath
changed the title
Codecov Report
@@ Coverage Diff @@
## master #11624 +/- ##
==========================================
- Coverage 69.01% 69.01% -0.01%
==========================================
Files 312 312
Lines 16335 16339 +4
Branches 4734 4735 +1
==========================================
+ Hits 11274 11276 +2
- Misses 5033 5035 +2
Partials 28 28
Continue to review full report at Codecov.
|
SimenB
left a comment
SimenB
left a comment
There was a problem hiding this comment.
Thanks! Could you add a test and a changelog entry?
sauravhiremath
force-pushed
the
master
branch
from
4f7cd03 to
e625b95
Compare
There was a problem hiding this comment.
This seems odd to me - how does only serializing work when we don't deserialize? Isn't the BigInt lost? In which case it'll be reported wrong if passed across a worker
$ node -p 'const JSONbig = require("json-bigint"); JSONbig.parse(JSONbig.stringify({hello: BigInt(42)}));'
[Object: null prototype] { hello: 42 }I'd like to see a test that sends an object to a worker and receives it back, and verifies they are equal.
This particular module also seems to be failing on Node 10.
Also, at some point we need to add support for circular objects (#10577) for which this module won't work. We can tackle that later of course, just showing that we need to properly serialize/deserialize 🙂
| return exposedMethods; | ||
| } | ||
|
|
||
| export function serializerWithBigInt(data: unknown): unknown { |
There was a problem hiding this comment.
this should not be in index to avoid circular dependencies. Please stick it into a utils, serializers or something
|
This PR is stale because it has been open 1 year with no activity. Remove stale label or comment or this will be closed in 30 days. |
|
This PR was closed because it has been stalled for 30 days with no activity. Please open a new PR if the issue is still relevant, linking to this one. |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
4 participants
Summary
Resolves #11617
Adds serialization support for BigInt Type - when passing messages through
jest-workerprocesses (parallel runs not usingworker-threads)Test plan
BigIntSerializationfor messages sent through jest-workerUpdate