Bug: KeysByType includes undefined #106
Labels
Comments
andnp
added a commit
that referenced
this issue
Jun 14, 2019
Because status modifiers are attached to keys, `KeysByType` gives back an `undefined` key when the original object contains optional keys; even if the optional key is not of the given type. This change fixes that by removing the optional status modifier from the object while iterating over the keys. The downside to this change is that it does not perfectly purify the object. So if other status modifiers are added in the future, we will not necessarily be robust to that change. A more "pure" fix (pun intended) would be to work with a purified object. The downside is that the only way I could figure out to do that would require much uglier code and/or a helper function that would need to be exported. In favor of minimizing "private" exports, I opted for the simple solution. Also a downside here is that we cannot filter by `undefined` if a key is optional. This was already an issue before this commit, so I'm punting on it for now because the fix seems non-trivial. Closes #106
andnp
added a commit
that referenced
this issue
Jun 14, 2019
The regression test makes sure that this PR actually solves the problem that was brought up in issue #106. It also adds a failing test case for a bug that I noticed while making the fix. Since my ad hoc testing solution obviously does not allow for known failures (which I guess is, on its own, a known failure..), I annotated this with a comment. There is obviously a lot of work to be done on the testing side of this library.
Merged
|
Thanks for pointing this out! I've added a PR (#108) that should fix this issue. It also alerted me to another issue with interface Obj { a: string; b?: number; }
// should be `b`, but is `never`
type got = KeysByType<Obj, undefined>; |
andnp
added a commit
that referenced
this issue
Jun 25, 2019
The regression test makes sure that this PR actually solves the problem that was brought up in issue #106. It also adds a failing test case for a bug that I noticed while making the fix. Since my ad hoc testing solution obviously does not allow for known failures (which I guess is, on its own, a known failure..), I annotated this with a comment. There is obviously a lot of work to be done on the testing side of this library.
|
🎉 This issue has been resolved in version 3.2.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Looks like for some reason
undefinedis getting included in the result for KeysByTypeThe text was updated successfully, but these errors were encountered: