Skip to content

Commit

Permalink
Fix multiple issues based on PR review feedback
Browse files Browse the repository at this point in the history
- 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?
  • Loading branch information
gitsunmin committed Mar 28, 2024
1 parent ea7a937 commit 1831e29
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 6 deletions.
9 changes: 7 additions & 2 deletions src/patterns.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1128,14 +1128,19 @@ export function shape(pattern: UnknownPattern) {
* .with(P.object.empty(), () => 'will match on empty objects')
*/
const emptyObject = <input>(): GuardExcludeP<input, object, never> => when(
(value) => value && typeof value === 'object' && Object.keys(value).length === 0,
(value) => {
if (!isObject(value)) return false;

for (var prop in value) return false;
return true;
},
);

const objectChainable = <pattern extends Matcher<any, any, any, any, any>>(
pattern: pattern
): ObjectChainable<pattern> =>
Object.assign(chainable(pattern), {
empty: () => objectChainable(intersection(pattern, emptyObject())),
empty: () => chainable(intersection(pattern, emptyObject())),
}) as any;

/**
Expand Down
6 changes: 3 additions & 3 deletions src/types/Pattern.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ export type MatcherType =
| 'or'
| 'and'
| 'array'
| 'object'
| 'map'
| 'set'
| 'select'
Expand Down Expand Up @@ -98,7 +97,7 @@ export type CustomP<input, pattern, narrowedOrFn> = Matcher<

export type ArrayP<input, p> = Matcher<input, p, 'array'>;

export type ObjectP<input, p> = Matcher<input, p, 'object'>;
export type ObjectP<input, p> = Matcher<input, p>;

export type OptionalP<input, p> = Matcher<input, p, 'optional'>;

Expand Down Expand Up @@ -194,6 +193,7 @@ export type NullishPattern = Chainable<
GuardP<unknown, null | undefined>,
never
>;

type MergeGuards<input, guard1, guard2> = [guard1, guard2] extends [
GuardExcludeP<any, infer narrowed1, infer excluded1>,
GuardExcludeP<any, infer narrowed2, infer excluded2>
Expand Down Expand Up @@ -676,7 +676,7 @@ export type ObjectChainable<
* match(value)
* .with(P.object.empty(), () => 'empty object')
*/
empty<input>(): ObjectChainable<
empty<input>(): Chainable<
ObjectP<input, Record<string, never>>,
omitted | 'empty'
>;
Expand Down
63 changes: 62 additions & 1 deletion tests/object.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,66 @@ describe('Object', () => {
expect(res).toEqual('not found');
});

it('when input is a Function, it should not match as an exact object', () => {
const fn = () => () => {};

const res = match(fn())
.with(P.object, (obj) => {
type t = Expect<Equal<typeof obj, () => void>>;
return 'not found';
})
.otherwise(() => 'not found');
expect(res).toEqual('not found');
})

it('when input is a Number (a primitive value), it should not be matched as an exact object', () => {
const fn = () => 1_000_000;

const res = match(fn())
.with(P.object, (obj) => {
type t = Expect<Equal<typeof obj, number>>;
return 'not found';
})
.otherwise(() => 'not found');
expect(res).toEqual('not found');
})

it('when input is a String (a primitive value), it should not be matched as an exact object', () => {
const fn = () => 'hello';

const res = match(fn())
.with(P.object, (obj) => {
type t = Expect<Equal<typeof obj, string>>;
return 'not found';
})
.otherwise(() => 'not found');
expect(res).toEqual('not found');
})

it('when input is a Boolean (a primitive value), it should not be matched as an exact object', () => {
const fn = () => true;

const res = match(fn())
.with(P.object, (obj) => {
type t = Expect<Equal<typeof obj, boolean>>;
return 'not found';
})
.otherwise(() => 'not found');
expect(res).toEqual('not found');
})

it('when input is Null, it should not be matched as an exact object', () => {
const fn = () => null;

const res = match(fn())
.with(P.object, (obj) => {
type t = Expect<Equal<typeof obj, null>>;
return 'not found';
})
.otherwise(() => 'not found');
expect(res).toEqual('not found');
})

it('should match object with nested objects', () => {
const res = match({ x: { y: 1 } })
.with({ x: { y: 1 } }, (obj) => {
Expand All @@ -45,6 +105,7 @@ describe('Object', () => {
return 'no';
})
.exhaustive();

expect(res).toEqual('yes');
});

Expand All @@ -64,7 +125,7 @@ describe('Object', () => {
expect(res).toEqual('yes');
});

it('should match object with optional properties', () => {
it('should properly match an object against the P.object pattern, even with optional properties', () => {
const res = match({ x: 1 })
.with(P.object.empty(), (obj) => {
type t = Expect<Equal<typeof obj, { readonly x: 1; }>>;
Expand Down

0 comments on commit 1831e29

Please sign in to comment.