Skip to content

fix: treat null as JSON-serializable in isJSONSerializable#607

Open
n0liu wants to merge 1 commit into
unjs:mainfrom
n0liu:fix/is-json-serializable-null
Open

fix: treat null as JSON-serializable in isJSONSerializable#607
n0liu wants to merge 1 commit into
unjs:mainfrom
n0liu:fix/is-json-serializable-null

Conversation

@n0liu

@n0liu n0liu commented Jul 7, 2026

Copy link
Copy Markdown

Since typeof null is "object", the t === null branch in isJSONSerializable was dead code (t is always a string). As a result isJSONSerializable(null) fell through to the value.buffer access and threw TypeError: Cannot read properties of null (reading 'buffer').

null is JSON-serializable (JSON.stringify(null) === "null"), so this checks value === null up front and returns true, which also removes the dead check.

No behavior change for $fetch itself — a null body is already short-circuited by the context.options.body && guard before this util is reached; the fix matters for direct/library use of the exported helper.

Resolves #571

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling of null values so they are now correctly recognized as JSON-serializable.
    • Expanded validation to better distinguish serializable values from unsupported types like undefined, functions, and typed arrays.
  • Tests

    • Added coverage for JSON-serializability checks across common primitives, objects, arrays, and edge cases.

typeof null is "object", so the t === null branch was dead code (t is
always a string). isJSONSerializable(null) fell through to value.buffer
and threw "Cannot read properties of null". null is JSON-serializable,
so check value === null and return true instead.

Resolves unjs#571
@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown

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: 6a7fe995-9137-44c9-bc98-990ee1f77b6d

📥 Commits

Reviewing files that changed from the base of the PR and between 9102908 and 2dd0215.

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

📝 Walkthrough

Walkthrough

Fixed a bug in isJSONSerializable where the null check compared typeof value to null (always false, dead code) instead of checking value === null directly, which previously caused the function to throw or misclassify null. Added corresponding tests.

Changes

isJSONSerializable null fix

Layer / File(s) Summary
Null check fix and tests
src/utils.ts, test/index.test.ts
Changed the initial type check from comparing typeof value to null (dead code) to value === null, and added a test suite verifying null is classified as serializable without throwing, alongside classification checks for other value types.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Related issues: Fixes #571 – isJSONSerializable bug where null values caused a TypeError and were incorrectly classified due to a dead-code comparison.

Suggested labels: bug, tests

Suggested reviewers: pi0

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main fix in the PR.
Linked Issues check ✅ Passed The code and tests directly address #571 by handling null as JSON-serializable without throwing.
Out of Scope Changes check ✅ Passed The only changes are the null fix and targeted tests, both in scope for the reported bug.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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.

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

1 participant