Repository navigation
Add support for custom objects that will correctly type to DataType - #192
Conversation
WalkthroughAdds version 0.0.150, updates JsonPrimitive typing to accept arbitrary valid custom objects as JSON, and introduces a comprehensive test suite validating DataType and JSON object acceptance. No changes to runtime control flow. Changes
Sequence Diagram(s)Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/types.ts (1)
196-203: Consider consolidating redundant type definitions.Adding
{ [key: string]: JsonPrimitive }toJsonPrimitivecreates logical redundancy withJsonObject(line 218), which already permits string keys viaJsonKey. While this addition may improve TypeScript's type inference for plain object literals, it introduces maintenance overhead and potential confusion.Consider these alternatives:
- Simplify by relying solely on
JsonObjectif type inference proves sufficient after testing- Document why both forms are needed if TypeScript inference genuinely requires the explicit index signature
- Ensure future changes keep both definitions synchronized
Alternative approach (if TypeScript inference works without the extra union member):
export type JsonPrimitive = | string | number | boolean | null | JsonArray - | JsonObject - | { [key: string]: JsonPrimitive }; + | JsonObject;If the explicit index signature is necessary for type inference, add a comment explaining why:
export type JsonPrimitive = | string | number | boolean | null | JsonArray | JsonObject + // Explicit index signature improves TypeScript inference for plain object literals | { [key: string]: JsonPrimitive };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
CHANGELOG.md(1 hunks)package.json(1 hunks)src/types.ts(1 hunks)test/types/datatype.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
{src,test}/**/!(*.d).ts
📄 CodeRabbit inference engine (AGENT.md)
{src,test}/**/!(*.d).ts: Use strict TypeScript and prefer unknown over any
Use ESM import/export syntax; avoid CommonJS require/module.exports
Use relative imports for internal modules
Keep imports organized (sorted, no unused imports)
Use tabs with a visual width of 2 spaces
Limit lines to a maximum of 80 characters
Use single quotes for strings
Use proper Error types; do not throw strings
Prefer template literals over string concatenation
Files:
src/types.tstest/types/datatype.test.ts
test/**
📄 CodeRabbit inference engine (AGENT.md)
Tests must mirror the source structure under the test/ directory
Files:
test/types/datatype.test.ts
🧬 Code graph analysis (1)
test/types/datatype.test.ts (1)
src/types.ts (3)
isDataType(167-191)DataType(144-152)isJsonObject(233-256)
🔇 Additional comments (3)
package.json (1)
3-3: LGTM: Version bump is consistent.The version bump to 0.0.150 aligns with the CHANGELOG entry documenting the JsonPrimitive type fix.
test/types/datatype.test.ts (1)
1-206: Excellent test coverage!The test suite is comprehensive and well-structured, covering:
- All DataType variants (Buffer, Uint8Array, ArrayBuffer, Blob, ReadableStream, strings)
- Custom object shapes including nested structures, mixed primitives, and arrays
- Edge cases like empty objects/arrays and deeply nested structures
- Data objects with contentType property
- Proper rejection of null/undefined
The tests effectively validate the expanded JsonPrimitive type and ensure DataType accepts arbitrary valid JSON objects.
CHANGELOG.md (1)
3-7: LGTM: Changelog entry is accurate.The changelog clearly documents the patch change for version 0.0.150, accurately describing the fix to support arbitrary valid custom objects as Json in DataType.
Summary by CodeRabbit