Add P.object and P.object.empty#233
Conversation
gitsunmin
changed the title
gvergnaud
left a comment
gvergnaud
left a comment
There was a problem hiding this comment.
Thank you for contributing! I have a few comments but that's a good start :)
| pattern: pattern | ||
| ): ObjectChainable<pattern> => | ||
| Object.assign(chainable(pattern), { | ||
| empty: () => objectChainable(intersection(pattern, emptyObject())), |
There was a problem hiding this comment.
Seems like chainable would be enough here, since you can't chain empty several times
There was a problem hiding this comment.
Yes, I'll change it to "chainable." If it's wrong, please "comment."
| | 'or' | ||
| | 'and' | ||
| | 'array' | ||
| | 'object' |
There was a problem hiding this comment.
I'm not sure a new pattern type is necessary here because both patterns you added are implemented with guards
| GuardP<unknown, null | undefined>, | ||
| never | ||
| >; | ||
|
|
| * match(value) | ||
| * .with(P.object.empty(), () => 'empty object') | ||
| */ | ||
| empty<input>(): ObjectChainable< |
There was a problem hiding this comment.
It should just be Chainable here as well
| type t = Expect<Equal<typeof obj, { str: 'world' }>>; | ||
| return obj.str; | ||
| }) | ||
| .with(P.object, (obj) => { |
There was a problem hiding this comment.
Could you add test covering how P.object behaves with more inputs:
- Functions
- Primitive values
- Null
It should catch all values that are assignable to the object type, and type narrowing and exhaustive should both work
There was a problem hiding this comment.
I've added test. If you need more, please comment
There was a problem hiding this comment.
Actually, functions and arrays are part of the object type in TypeScript: Playground
I think P.object should catch anything assignable to object to stay close to typescript semantics and play nicely with exhaustiveness checking.
I'm going to update your PR shortly
- Comment: Seems like chainable would be enough here, since you can't chain empty several times - Comment: I think we could make this more efficient by using a for in loop instead of Object.keys and breaking the loop by returning false if an object own property is encountered. - Comment: It should just be Chainable here as well - Comment: I'm not sure a new pattern type is necessary here because both patterns you added are implemented with guards - Comment: Could you add test covering how P.object behaves with more inputs: Functions, Primitive values, Null. It should catch all values that are assignable to the object type, and type narrowing and exhaustive should both work - Comment: Could you remove this diff?
|
I wanted to make a few changes and realized I can't push to your repository so I had to create a new branch that includes your changes. Closing this PR in favor of #234 |
2 participants
Add P.object and P.object.empty
Add a pattern to match all 'Object' and 'Empty Object'.
P.objectP.object.emptyIssue number: #230