Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: added docs and test for P.object.exact(..) #244

Draft
wants to merge 5 commits into
base: gitsunmin/main
Choose a base branch
from

Conversation

JUSTIVE
Copy link

@JUSTIVE JUSTIVE commented Apr 7, 2024

adds docs, and tests for P.object.exact

Todos:

  • write docs for P.object.exact
  • write test for P.object.exact

@JUSTIVE JUSTIVE marked this pull request as draft April 7, 2024 09:10
@JUSTIVE
Copy link
Author

JUSTIVE commented Apr 7, 2024

currently, {} could be caught by P.object.exact({a: P.any}) pattern.

@JUSTIVE
Copy link
Author

JUSTIVE commented Apr 7, 2024

fixed {} caught by {a:P.any}, but nested object pattern should be tested

Copy link
Owner

@gvergnaud gvergnaud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for contributing! A few comments

README.md Outdated Show resolved Hide resolved
src/patterns.ts Show resolved Hide resolved
Comment on lines +174 to +176
export type ObjectExactPattern<a> = {
readonly [k in keyof a]: Pattern<a[k]>;
};
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it to force people to include any key that exist's on an object? I think that makes a lot of sense, but how does it behave when the input has type unknown and any? Could you add tests for that?

tests/object.test.ts Outdated Show resolved Hide resolved
JUSTIVE and others added 2 commits April 8, 2024 01:30
Co-authored-by: Gabriel Vergnaud <9265418+gvergnaud@users.noreply.github.com>
@JUSTIVE
Copy link
Author

JUSTIVE commented Apr 8, 2024

during the implementation of this feature, I'm concerned about the following scenarios.

  1. nested object, but an exact match for the outer side, and less strict matching inside. (the other way, less strict matching outside and strict inside makes sense, but not this one.)
  2. recursive objects.

maybe we need to consider those cases first before implementing it.

@gvergnaud
Copy link
Owner

gvergnaud commented Apr 8, 2024

I was think that exact({ ... }) would only apply to the current level of nesting. If you want everything to be exact, then you need to nest exacts as well:

P.object.exact({
   // this object should only have a single `a` key
   a: {
      // this object must have at least a `b` key that's a string.
      b: P.string
   }
})

If you want everything to be exact you'd do:

 P.object.exact({
   a: P.object.exact({
      b: P.string
   })
})

Do you find a problem with that? What are the scenarios that worry you?

@JUSTIVE
Copy link
Author

JUSTIVE commented Apr 8, 2024

I think that behavior could easily cause misunderstanding. I thought exact should catch 'exact' object matching, not 'exact matching, but only single depth'. It's a bit ambiguous.

@JUSTIVE
Copy link
Author

JUSTIVE commented Apr 13, 2024

I'll keep implementing with single-depth match version(because it's a sub-spec of full-depth matching), but still not sure it's good design or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants