Skip to content

fix(utils): handle null correctly in isJSONSerializable#595

Closed
chatman-media wants to merge 1 commit into
unjs:mainfrom
chatman-media:fix/isjsonserializable-null
Closed

fix(utils): handle null correctly in isJSONSerializable#595
chatman-media wants to merge 1 commit into
unjs:mainfrom
chatman-media:fix/isjsonserializable-null

Conversation

@chatman-media
Copy link
Copy Markdown

@chatman-media chatman-media commented Jun 6, 2026

Closes #571

What was wrong

isJSONSerializable in src/utils.ts had three bugs when called with null:

  1. Dead codet === null on line 22 (where t = typeof value) can never be true. typeof never returns the string "null", so this branch was unreachable.
  2. Wrong return valuenull is valid JSON (JSON.stringify(null) === "null"), but the function never identified it as serializable.
  3. Runtime crash — because null wasn't caught early, execution reached value.buffer, which throws TypeError: Cannot read properties of null (reading 'buffer').

Fix

Add an explicit value === null guard that returns true before any property access on value, and drop the dead t === null branch from the typeof check:

// Before (buggy)
const t = typeof value;
if (t === "string" || t === "number" || t === "boolean" || t === null) {
  return true;
}

// After
if (value === null) {
  return true;
}
const t = typeof value;
if (t === "string" || t === "number" || t === "boolean") {
  return true;
}

undefined and all other existing code paths are unaffected.

Tests

Added a dedicated describe("isJSONSerializable") block in test/index.test.ts covering:

  • null — does not throw, returns true
  • undefined — returns false
  • primitives (string, number, boolean) — return true
  • non-serializable types (function, symbol, bigint) — return false
  • plain objects and arrays — return true
  • objects with toJSON — return true
  • Buffer / Uint8Array — return false
  • FormData / URLSearchParams — return false

The null test fails on the original code (TypeError: Cannot read properties of null (reading 'buffer')) and passes with this fix.

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Enhanced null value handling in JSON serialization.
  • Tests

    • Added comprehensive test coverage for JSON serialization, including null, undefined, primitives, and special object types.

- Remove dead code: `typeof value` never returns `"null"`, so `t === null`
  was always false and the branch never matched.
- Add an early `value === null` guard that returns `true`; null is
  JSON-serializable (`JSON.stringify(null) === "null"`).
- The guard must come before any property access on `value` — without it,
  `isJSONSerializable(null)` throws
  `TypeError: Cannot read properties of null (reading 'buffer')`.
- Add a dedicated `isJSONSerializable` describe block in the test suite
  covering null (no-throw + correct return value), undefined, primitives,
  non-serializable types, plain objects/arrays, toJSON, Buffer/TypedArray,
  FormData, and URLSearchParams.

Closes unjs#571
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 6, 2026

Linter diff in the way? Review this PR in Change Stack to focus on meaningful changes and expand context only when needed.

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 06426def-2bae-4caa-95e6-ac039f77a709

📥 Commits

Reviewing files that changed from the base of the PR and between dfbe3ca and a02ddb4.

📒 Files selected for processing (2)
  • src/utils.ts
  • test/index.test.ts

📝 Walkthrough

Walkthrough

The PR fixes a bug in isJSONSerializable where null was not recognized as JSON-serializable and caused runtime errors. The function now includes an explicit value === null early return check. Comprehensive test coverage is added for null, undefined, primitives, objects with toJSON, and non-serializable types like Buffer, typed arrays, FormData, and URLSearchParams.

Changes

null handling and test coverage

Layer / File(s) Summary
isJSONSerializable fix and test suite
src/utils.ts, test/index.test.ts
isJSONSerializable function refactored to add explicit null check before typeof evaluation, eliminating dead code and fixing null serialization detection. Test suite added with imports and comprehensive coverage of null, undefined, primitives, plain objects, toJSON-providing objects, and non-serializable types including Buffer, typed arrays, FormData, and URLSearchParams.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

A null-handling fix, crisp and clean,
Where null now shines in JSON's sheen,
No more errors, no more dread,
The rabbit fixed what code misread! 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main fix: handling null correctly in the isJSONSerializable function.
Linked Issues check ✅ Passed All three objectives from issue #571 are met: null guard added before property access, dead typeof null check removed, and null correctly returns true as JSON-serializable.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the null handling bug in isJSONSerializable and adding appropriate test coverage for the fix.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@pi0 pi0 closed this Jun 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

isJSONSerializable bug

2 participants