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: Add isPartialRecord guard #50

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

kkirsche
Copy link
Contributor

@kkirsche kkirsche commented Jan 20, 2023

This begins to address #4 but is no longer a full solution.

@kkirsche kkirsche changed the title feat: Support isRecord feat: Support isRecord Jan 20, 2023
@kkirsche kkirsche changed the title feat: Support isRecord feat: Add isRecord guard (fixes #4) Jan 20, 2023
@lazarljubenovic
Copy link
Owner

This doesn't work correctly. The simplest example is for an empty object: you'll always return true, because the array of key/value pairs you're checking is empty. In TypeScript, Record is exhaustive: the object of type Record<'a' | 'b' | 'c', number> required all three keys to exist, but you function will return true as long as each existing satisfies the predicate. For example, {} will pass, but it should not.

Unfortunately, I don't think there's an easy to fix it properly. The only thing we can do is assert that an object is of type Partial<Record<K, V>>, but this should be properly documented and reflected in the function name to avoid confusion.

@kkirsche
Copy link
Contributor Author

kkirsche commented Mar 8, 2023

I'm happy to either close this due to this limitation or update it to be isPartialRecord, do you have a preference which direction is gone?

@lazarljubenovic
Copy link
Owner

I guess leaving it but renaming it is the way to go. At least we'll have a function, which is better than nothing!

@kkirsche
Copy link
Contributor Author

kkirsche commented Mar 8, 2023

Rebased against master and I've renamed the function to isPartialRecord, updated the guard and the comment

@kkirsche
Copy link
Contributor Author

kkirsche commented Mar 8, 2023

😄 well, at least the github actions workflows are protecting things

@kkirsche kkirsche changed the title feat: Add isRecord guard (fixes #4) feat: Add isPartialRecord guard Mar 8, 2023
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