-
-
Notifications
You must be signed in to change notification settings - Fork 5
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
feat: add isCustomJsonable
and isJsonable
#128
Conversation
My |
About this implementationThis implementation is deficient for
const objBigInt = Object.assign(1n, { a: 1 });
assertEquals(typeof objBigInt, "object");
assertThrows(
() => JSON.stringify(objBigInt),
TypeError,
);
BigInt.prototype.toJSON = function() { return Number(this) };
const primitiveBigInt = 42n;
assertEquals(typeof primitiveBigInt, "bigint");
assertEquals(JSON.stringify(primitiveBigInt), "42");
const hasown = Object.assign(() => {}, { toJSON: () => "own" });
assertEquals(typeof hasown, "function")
assertEquals(JSON.stringify(hasown), '"own"');
Function.prototype.toJSON = () => "proto";
const hasproto = () => {};
assertEquals(typeof hasproto, "function")
assertEquals(JSON.stringify(hasproto), '"proto"'); |
Do you think we need P.S. Probably I'll implement
|
f308e9a
to
499b465
Compare
isJsonable
isCustomJsonable
and isJsonable
499b465
to
c75ca90
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #128 +/- ##
=======================================
Coverage 99.88% 99.89%
=======================================
Files 50 52 +2
Lines 909 954 +45
Branches 104 120 +16
=======================================
+ Hits 908 953 +45
Misses 1 1 ☔ View full report in Codecov by Sentry. |
142f99b
to
3227c4a
Compare
d0b46d2
to
dcc7d6d
Compare
6739c11
to
39e4eb1
Compare
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.
LGTM!
If we strictly adhere to the description of
JSON.stringify
, theisJsonable
function would be implemented like this.However, I’m not sure if this aligns with what @Milly originally intended. What we might actually need is a predicate function to determine if an object implements a
toJSON
function.@Milly What do you think?