Skip to content

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

Draft
JUSTIVE wants to merge 5 commits intogvergnaud:gitsunmin/mainfrom
JUSTIVE:gitsunmin/main
Draft

feat: added docs and test for P.object.exact(..)#244
JUSTIVE wants to merge 5 commits intogvergnaud:gitsunmin/mainfrom
JUSTIVE:gitsunmin/main

Conversation

@JUSTIVE
Copy link
Copy Markdown
Contributor

@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
Copy Markdown
Contributor Author

JUSTIVE commented Apr 7, 2024

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

@JUSTIVE
Copy link
Copy Markdown
Contributor Author

JUSTIVE commented Apr 7, 2024

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

Copy link
Copy Markdown
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

Comment thread README.md Outdated
Comment thread src/patterns.ts
Comment thread src/types/Pattern.ts
Comment on lines +174 to +176
export type ObjectExactPattern<a> = {
readonly [k in keyof a]: Pattern<a[k]>;
};
Copy link
Copy Markdown
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?

Comment thread tests/object.test.ts Outdated
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
Copy Markdown
Contributor 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
Copy Markdown
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
Copy Markdown
Contributor 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
Copy Markdown
Contributor 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.

2 participants