fix(utils): move instanceof guards before value.buffer in isJSONSerializable (#580)

[MohammadYusif]

Summary

• isJSONSerializable checked value.buffer before instanceof FormData || instanceof URLSearchParams, making those guards unreachable for objects that expose a .buffer property
• In Bun, FormData has a non-standard toJSON method; if it exposes buffer, the explicit exclusion would silently fail
• Accessing value.buffer on certain host objects can also throw before reaching the instanceof checks

Changes

• src/utils.ts: moved instanceof FormData || instanceof URLSearchParams before value.buffer check to ensure correct early rejection regardless of internal structure
• test/index.test.ts: added 10 unit tests for isJSONSerializable covering FormData, URLSearchParams, ArrayBuffer, TypedArray, plain objects, arrays, toJSON objects, and undefined

Fixes #580

Summary by CodeRabbit

• Tests

  • Added comprehensive test coverage for JSON-serialization validation, covering plain objects/arrays, objects with custom serialization, empty/non-empty form-like and binary inputs, and undefined.

• Refactor

  • Reordered internal validation checks for JSON-serializability to improve consistency without changing public behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ca489a5f-a744-406e-90af-6ca235e259d8

📥 Commits

Reviewing files that changed from the base of the PR and between 3b369fb and 5c988e7.

📒 Files selected for processing (2)

• src/utils.ts
• test/index.test.ts

✅ Files skipped from review due to trivial changes (1)

• src/utils.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• test/index.test.ts

---

📝 Walkthrough

Walkthrough

The isJSONSerializable checks were reordered so FormData/URLSearchParams instanceof exclusions run before accessing value.buffer. A Vitest suite was added to assert expected true/false results across FormData, URLSearchParams, ArrayBuffer, Uint8Array, plain objects/arrays, objects with toJSON, and undefined.

Changes

JSON Serialization Safety and Testing

Layer / File(s)	Summary
Reordered FormData/URLSearchParams checks src/utils.ts	The isJSONSerializable function moves its FormData/URLSearchParams instanceof checks before the value.buffer property access to prevent errors on edge cases.
isJSONSerializable test suite test/index.test.ts	Comprehensive test cases validate the function returns false for FormData, URLSearchParams, ArrayBuffer, Uint8Array, and undefined, and true for plain objects, arrays, and objects with toJSON methods.

🎯 2 (Simple) | ⏱️ ~10 minutes

🐰 A reorder done with care,
Tests that prove the function's fair,
FormData and URLSearchParams now safe,
Before buffer's risky waif,
Edge cases handled with a flair!

🚥 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 accurately describes the main change: moving instanceof guards before the value.buffer check in isJSONSerializable.
Linked Issues check	✅ Passed	The PR implementation directly addresses #580 by moving instanceof FormData/URLSearchParams checks before value.buffer access, and comprehensive tests validate the fix.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing issue #580: reordering checks in isJSONSerializable and adding corresponding unit tests.

_{✏️ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

src/utils.ts (1)

530-572: ⚡ Quick win

Consider adding tests for primitive types for completeness.

The test suite thoroughly validates the fix for issue #580 and covers the main object/buffer-backed types. For complete coverage of the function's contract (lines 22-24 in src/utils.ts), consider adding test cases for primitive types that are JSON-serializable: null, strings, numbers, and booleans.

✅ Suggested additional test cases

  it("returns true for null", () => {
    expect(isJSONSerializable(null)).toBe(true);
  });

  it("returns true for strings", () => {
    expect(isJSONSerializable("hello")).toBe(true);
  });

  it("returns true for numbers", () => {
    expect(isJSONSerializable(42)).toBe(true);
  });

  it("returns true for booleans", () => {
    expect(isJSONSerializable(true)).toBe(true);
  });

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils.ts` around lines 530 - 572, Add unit tests for primitive
JSON-serializable values to cover the isJSONSerializable contract in
src/utils.ts: create tests that call isJSONSerializable with null, a string
(e.g., "hello"), a number (e.g., 42), and a boolean (e.g., true) and assert they
return true; place these alongside the existing tests for object/buffer-backed
types so the suite verifies both object and primitive branches of
isJSONSerializable.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/utils.ts`:
- Around line 530-572: Add unit tests for primitive JSON-serializable values to
cover the isJSONSerializable contract in src/utils.ts: create tests that call
isJSONSerializable with null, a string (e.g., "hello"), a number (e.g., 42), and
a boolean (e.g., true) and assert they return true; place these alongside the
existing tests for object/buffer-backed types so the suite verifies both object
and primitive branches of isJSONSerializable.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: eabf71f1-7319-4bd6-9efe-eb219075c3d7

📥 Commits

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

📒 Files selected for processing (2)

• src/utils.ts
• test/index.test.ts