👍 Add isObjectOf modify functions (isStrictOf, isPartialOf, isPickOf, and isOmitOf)
#56
Conversation
- Define `toString` for later use - Use `Set` instead of `Array.includes`
|
Warning Rate Limit Exceeded@lambdalisue has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 5 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. WalkthroughThis update introduces utility types for flattening nested objects and converting unions to intersections, alongside expanding the Changes
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #56 +/- ##
==========================================
- Coverage 99.55% 98.66% -0.90%
==========================================
Files 3 4 +1
Lines 454 524 +70
Branches 46 55 +9
==========================================
+ Hits 452 517 +65
- Misses 2 7 +5 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 4
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
__snapshots__/is_test.ts.snapis excluded by:!**/*.snap
Files selected for processing (5)
- _typeutil.ts (1 hunks)
- is.ts (23 hunks)
- is_bench.ts (2 hunks)
- is_test.ts (6 hunks)
- metadata.ts (1 hunks)
Additional comments: 20
_typeutil.ts (1)
- 1-8: The type utilities
FlatType<T>andUnionToIntersection<U>are correctly implemented and follow TypeScript best practices.metadata.ts (1)
- 1-46: The implementation of metadata functionality for predicate functions in
metadata.tsis correctly done, enhancing the library's capabilities for sophisticated type validation scenarios.is_bench.ts (1)
- 375-416: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [332-413]
The benchmarks for the newly introduced predicate functions
is.StrictOf,is.PartialOf,is.PickOf, andis.OmitOfare correctly implemented, ensuring their performance is tested.is.ts (11)
- 946-962:
isPickOfcorrectly filters the properties based on the provided keys array. However, ensure that the keys array is validated to contain only keys that exist inTto prevent runtime errors or unexpected behavior.- 986-1002:
isOmitOfcorrectly filters out the specified keys. Similar toisPickOf, ensure that the keys array is validated to contain only keys that exist inTto prevent runtime errors or unexpected behavior.- 1021-1040:
isOptionalOfuses a property check to avoid duplicate wrapping withOptional. Consider adding a comment or documentation to clarify this behavior for future maintainers or users of the library.- 1085-1085:
isSyncFunctionusestoString.call(x)for type checking. This is a reliable method, but ensure consistency across similar checks, such asisAsyncFunction, for maintainability.- 1104-1104:
isAsyncFunctionis consistent withisSyncFunctionin its implementation. This consistency is good for maintainability.- 1126-1131:
isInstanceOfcorrectly uses metadata and provides a type-safe way to check for class instances. Ensure that the metadata functionality is thoroughly tested, especially for edge cases with inheritance.- 1222-1231: The
primitiveSetis used inisPrimitivefor checking primitive types. This is an efficient way to handle multiple type checks in one function. Ensure that all relevant primitive types are included.- 1258-1265:
isLiteralOffunction is correctly implemented with metadata. This function provides a type-safe way to check for specific literal values.- 1292-1298:
isLiteralOneOfuses aSetfor efficient checking against multiple literals. This is a good practice for performance. Ensure that the literals array is properly validated to contain only primitives.- 1342-1347:
isOneOfcorrectly implements a type predicate for union types. The use ofsomefor checking predicates is efficient. Ensure comprehensive testing for complex union types.- 1401-1406:
isAllOfcorrectly implements a type predicate for intersection types. The use ofeveryfor checking predicates is efficient. Ensure comprehensive testing for complex intersection types.is_test.ts (6)
- 27-39: The import statements for
isOmitOf,isPartialOf,isPickOf, andisStrictOfare correctly added. Ensure these functions are properly exported from./is.ts.- 825-842: The test for
isObjectOfwith optional properties correctly checks for both presence and absence of optional properties. However, ensure that the test cases cover scenarios where optional properties are explicitly set toundefinedand when they are omitted entirely.- 846-916: The
isStrictOftests correctly validate the strict object validation, including the handling of unknown properties. Ensure that the test cases adequately cover objects with extra properties not defined in the predicate, as this is a key aspect of strict validation.- 996-1029: The
isPartialOftests are well-structured to validate partial object validation. It's important to ensure that the test cases cover scenarios where some properties are omitted and others are present, including cases where properties are explicitly set toundefined.- 1032-1074: The
isPickOftests correctly validate the functionality of picking specific properties from an object for validation. Ensure that the test cases cover various combinations of picked properties, including cases where picked properties are missing or have incorrect types.- 1077-1119: The
isOmitOftests correctly validate the functionality of omitting specific properties from an object for validation. Ensure that the test cases cover various combinations of omitted properties, including cases where omitted properties are present with incorrect types.
| // Nesting does nothing | ||
| await assertSnapshot(t, isOptionalOf(isOptionalOf(isNumber)).name); |
There was a problem hiding this comment.
The nested use of isOptionalOf in the test setup is redundant since nesting isOptionalOf does not change its behavior. Consider simplifying the test by removing the nested usage or clarifying the intent if it's meant to test a specific edge case.
- await assertSnapshot(t, isOptionalOf(isOptionalOf(isNumber)).name);Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| // Nesting does nothing | |
| await assertSnapshot(t, isOptionalOf(isOptionalOf(isNumber)).name); |
| export function isObjectOf< | ||
| T extends Record<PropertyKey, Predicate<unknown>>, | ||
| >( | ||
| predObj: T, | ||
| { strict }: { strict?: boolean } = {}, | ||
| ): Predicate<ObjectOf<T>> & WithMetadata<IsObjectOfMetadata> { | ||
| if (strict) { | ||
| // For backward compatibility | ||
| // deno-lint-ignore no-explicit-any | ||
| return isStrictOf(isObjectOf(predObj)) as any; | ||
| } | ||
| return setPredicateMetadata( | ||
| (x: unknown): x is ObjectOf<T> => { | ||
| if (!isRecord(x)) return false; | ||
| for (const k in predObj) { | ||
| if (!predObj[k](x[k])) return false; | ||
| } | ||
| return true; | ||
| }, | ||
| { name: "isObjectOf", args: [predObj] }, | ||
| ); |
There was a problem hiding this comment.
The isObjectOf function includes a deprecated strict option. Consider removing deprecated features to clean up the API and encourage the use of the new, more explicit functions like isStrictOf.
- { strict }: { strict?: boolean } = {},
- if (strict) {
- // For backward compatibility
- // deno-lint-ignore no-explicit-any
- return isStrictOf(isObjectOf(predObj)) as any;
- }Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| export function isObjectOf< | |
| T extends Record<PropertyKey, Predicate<unknown>>, | |
| >( | |
| predObj: T, | |
| { strict }: { strict?: boolean } = {}, | |
| ): Predicate<ObjectOf<T>> & WithMetadata<IsObjectOfMetadata> { | |
| if (strict) { | |
| // For backward compatibility | |
| // deno-lint-ignore no-explicit-any | |
| return isStrictOf(isObjectOf(predObj)) as any; | |
| } | |
| return setPredicateMetadata( | |
| (x: unknown): x is ObjectOf<T> => { | |
| if (!isRecord(x)) return false; | |
| for (const k in predObj) { | |
| if (!predObj[k](x[k])) return false; | |
| } | |
| return true; | |
| }, | |
| { name: "isObjectOf", args: [predObj] }, | |
| ); | |
| export function isObjectOf< | |
| T extends Record<PropertyKey, Predicate<unknown>>, | |
| >( | |
| predObj: T, | |
| ): Predicate<ObjectOf<T>> & WithMetadata<IsObjectOfMetadata> { | |
| return setPredicateMetadata( | |
| (x: unknown): x is ObjectOf<T> => { | |
| if (!isRecord(x)) return false; | |
| for (const k in predObj) { | |
| if (!predObj[k](x[k])) return false; | |
| } | |
| return true; | |
| }, | |
| { name: "isObjectOf", args: [predObj] }, | |
| ); |
| export function isStrictOf<T extends Record<PropertyKey, unknown>>( | ||
| pred: | ||
| & Predicate<T> | ||
| & WithMetadata<IsObjectOfMetadata>, | ||
| ): | ||
| & Predicate<T> | ||
| & WithMetadata<IsStrictOfMetadata> { | ||
| const { args } = getPredicateMetadata(pred); | ||
| const s = new Set(Object.keys(args[0])); | ||
| return setPredicateMetadata( | ||
| (x: unknown): x is T => { | ||
| if (!pred(x)) return false; | ||
| // deno-lint-ignore no-explicit-any | ||
| const ks = Object.keys(x as any); | ||
| return ks.length <= s.size && ks.every((k) => s.has(k)); | ||
| }, | ||
| { name: "isStrictOf", args: [pred] }, | ||
| ); |
There was a problem hiding this comment.
In isStrictOf, the check ks.length <= s.size allows objects with fewer keys than specified in predObj to pass the predicate, which contradicts the strictness requirement. It should ensure the number of keys in x exactly matches the number of keys in predObj.
- return ks.length <= s.size && ks.every((k) => s.has(k));
+ return ks.length === s.size && ks.every((k) => s.has(k));Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| export function isStrictOf<T extends Record<PropertyKey, unknown>>( | |
| pred: | |
| & Predicate<T> | |
| & WithMetadata<IsObjectOfMetadata>, | |
| ): | |
| & Predicate<T> | |
| & WithMetadata<IsStrictOfMetadata> { | |
| const { args } = getPredicateMetadata(pred); | |
| const s = new Set(Object.keys(args[0])); | |
| return setPredicateMetadata( | |
| (x: unknown): x is T => { | |
| if (!pred(x)) return false; | |
| // deno-lint-ignore no-explicit-any | |
| const ks = Object.keys(x as any); | |
| return ks.length <= s.size && ks.every((k) => s.has(k)); | |
| }, | |
| { name: "isStrictOf", args: [pred] }, | |
| ); | |
| export function isStrictOf<T extends Record<PropertyKey, unknown>>( | |
| pred: | |
| & Predicate<T> | |
| & WithMetadata<IsObjectOfMetadata>, | |
| ): | |
| & Predicate<T> | |
| & WithMetadata<IsStrictOfMetadata> { | |
| const { args } = getPredicateMetadata(pred); | |
| const s = new Set(Object.keys(args[0])); | |
| return setPredicateMetadata( | |
| (x: unknown): x is T => { | |
| if (!pred(x)) return false; | |
| // deno-lint-ignore no-explicit-any | |
| const ks = Object.keys(x as any); | |
| return ks.length === s.size && ks.every((k) => s.has(k)); | |
| }, | |
| { name: "isStrictOf", args: [pred] }, | |
| ); |
| T extends Record<PropertyKey, unknown>, | ||
| >( | ||
| pred: Predicate<T> & WithMetadata<IsObjectOfMetadata>, | ||
| ): | ||
| & Predicate<FlatType<Partial<T>>> | ||
| & WithMetadata<IsObjectOfMetadata> { | ||
| const { args } = getPredicateMetadata(pred); | ||
| const predObj = Object.fromEntries( | ||
| Object.entries(args[0]).map(([k, v]) => [k, isOptionalOf(v)]), | ||
| ); | ||
| return isObjectOf(predObj) as | ||
| & Predicate<FlatType<Partial<T>>> | ||
| & WithMetadata<IsObjectOfMetadata>; | ||
| } |
There was a problem hiding this comment.
isPartialOf uses isOptionalOf internally for all properties, which might not be necessary for already optional properties. This could lead to redundant checks and slightly impact performance. Consider optimizing this behavior for properties that are already optional.
Close #52
Example code
Summary by CodeRabbit