Skip to content

fix(utils): handle null safely and check instanceof before buffer property in isJSONSerializable - #636

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

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

Conversation

@swaraj792725

@swaraj792725 swaraj792725 commented Sep 27, 2026 •

Copy link
Copy Markdown

Summary

Fixes #571 and #580.

Previously, had two critical bugs:

  1. in JS evaluates to "object", not "null". The check if (t === "null") was dead code, causing isJSONSerializable(null) to fall through to value.buffer, throwing TypeError: Cannot read properties of null (reading 'buffer').
  2. value.buffer property check was performed before instanceof FormData and instanceof URLSearchParams, and without checking whether FormData/URLSearchParams are defined in the global context.

Fix

  1. Handle value === null explicitly up front (returning true since null is a valid JSON value).
  2. Move instanceof checks for FormData and URLSearchParams before checking value.buffer, and guard with typeof FormData !== "undefined".
  3. Handle objects created with Object.create(null) without crashing on missing constructor.

Test Plan

  • Added unit tests for isJSONSerializable covering null, undefined, primitives, objects created with Object.create(null), FormData, and URLSearchParams.
  • All 30 unit tests and lint checks pass cleanly.

Summary by CodeRabbit

  • Bug Fixes
    • Improved JSON serializability checks for null values, primitive values, and plain objects.
    • Checks now safely handle environments where form-data and URL-parameter features are unavailable.
    • Values such as undefined and symbols continue to be treated as non-serializable.

@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 6a291620-8272-4aea-aa24-015ae7e8f615

📥 Commits

Reviewing files that changed from the base of the PR and between 1dbc37f and c320e5b.

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

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

isJSONSerializable now accepts null, rejects undefined, and guards checks for FormData and URLSearchParams when those globals are unavailable. Tests cover these cases and other primitive and object values.

Changes

JSON serializability checks

Layer / File(s) Summary
Serializability behavior and validation
src/utils.ts, test/index.test.ts
The function updates its handling of nullish values, optional globals, and objects without constructors. Tests cover primitive values, plain and null-prototype objects, symbols, FormData, and URLSearchParams.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to c320e

The serializability changes appear ready to merge after normal checks.

Architecture Summary

Architecture risk: 🔵 Low · up to c320e

The change affects 2 systems.

Changed systems: src, test

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — src (service) was modified; 1 changed file maps to changed impact.
  • observed — test (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in src/utils.ts: isJSONSerializable now rejects undefined before type checks and accepts null. It retains primitive and array handling, rejects FormData and URLSearchParams only when the corresponding global exists, and treats objects without a constructor as eligible for the final plain-object/toJSON check; previously those globals were accessed unconditionally and the plain-object check required a constructor.
  • observed — Modified behavior in test/index.test.ts: Added tests for isJSONSerializable covering primitive and object values, including expected results for null, undefined, symbols, null-prototype objects, FormData, and URLSearchParams.
🚥 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%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. 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 describes the primary change: safer null handling and guarded instance checks in isJSONSerializable.
Linked Issues check ✅ Passed The change satisfies [#571]. isJSONSerializable now accepts null without reading null.buffer, and the tests cover null as serializable. The change also keeps undefined non-serializable and a…
Out of Scope Changes check ✅ Passed The changed source and tests remain within isJSONSerializable behavior. The FormData, URLSearchParams, and null-prototype handling supports the function's type checks and does not demonstrate un…
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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