fix: handle null in isJSONSerializable (#571)#577
Conversation
`typeof null` returns `"object"`, not `null`, so the check `t === null` was dead code. This caused `null` to fall through to the `value.buffer` check, throwing a TypeError. Fix by checking `value === null` explicitly before the typeof checks. Since `null` is valid JSON (`JSON.stringify(null)` === `"null"`), it should return `true`.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR fixes a bug in ChangesNull JSON serialization fix
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Closing this duplicate PR. The fix is also covered in #578. |
Fixes #571
Problem
The
isJSONSerializablefunction has a bug withnullvalues:typeof nullreturns"object", notnull", sot === null` is dead codenullis valid JSON (JSON.stringify(null)==="null"), so it should be detected as JSON serializablenullto the function throws:TypeError: Cannot read properties of null (reading 'buffer')Fix
Check
value === nullexplicitly before thetypeofchecks. Sincenullis valid JSON, returntrue.export function isJSONSerializable(value: any): boolean { if (value === undefined) { return false; } + if (value === null) { + return true; + } const t = typeof value; - if (t === "string" || t === "number" || t === "boolean" || t === null) { + if (t === "string" || t === "number" || t === "boolean") { return true; }Summary by CodeRabbit