fix(collections): stricter enforcement on generics#6961
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves TypeScript type inference for the pick function by using the const type parameter modifier and restructuring the generic signature to ensure proper type narrowing when picking object properties.
Changes:
- Updated generic signature to use
const K extends readonly (keyof T)[]instead ofK extends keyof T - Changed return type from
Pick<T, K>toPick<T, K[number]>to correctly extract union type from the array
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6961 +/- ##
==========================================
+ Coverage 94.08% 94.21% +0.12%
==========================================
Files 600 614 +14
Lines 43540 47791 +4251
Branches 6991 8303 +1312
==========================================
+ Hits 40965 45026 +4061
- Misses 2520 2698 +178
- Partials 55 67 +12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
would need some help to fix test-bun 😄 i am not sure how to make it aware of |
kt3k
left a comment
There was a problem hiding this comment.
Good catch. Thanks for the contribution!
Please see the following reproduction:
https://www.typescriptlang.org/play/?noUncheckedIndexedAccess=true#code/KYDwDg9gTgLgBAMwK4DsDGMCWEVzJtAawH0IAbAEwB4AVOUGYFCgZzggCMArYDAGjgBpeiEbM2hYAE8ICODQB8ACgBQcdtwBccAErAAhhRxkptBXzVxJUltqgGjKE0IDaAXQsBKbQAUChWgFBBTgAb0s0HBZ4exYkMngAXjCAXzh9Nj8iQKEFAG5LBGg4JUiUaKtpdjlrFk84TDklawbcTi562PiYF2s3OGT23uk3AvV7GCQoXC6EgpSVFVBIWERUDGxcfGy6BiZWDR5+ODKK4T3xOHtDYykS61l5T3dlS3btPRunU0ULdVrtIIvL5-DlBC4UEgALYcYBQNwhcLqU4xYBxBIDVLpTKgmhBCHQ2Hw-KFYqlKLwFqPWr1Rr3KqYNrcTpo7rDKT9QbcdmjSwTKYzVlzFQLJbgaDwZDoLA4PD+Yj6BK0ERiA7tXgwAQooQq-YSaSPRSqdTvXQOW5mP6VGx2c3fVweFTeOBZAJ43JhCIUq5CpJYjIu3FBEnqIpQEraqk1aR1BpNFqMw4s9E9PqYoZ9MY+ybTH0p+aLbVyZLKi4HPSRKDUaJQRkAcwEqEIKAgAHcUAplDBtDR6okQjACtrW1AcHWFeVW3DMQglNsSOQKEpQnAUPoocBtAByLgZHBbgT6OubuAAZgAjHAUgIXFu1xut25PJ4h97K-YMBOWFPw8lZ-Pl1XdcTx3PcUAPdJj20C8rxvO9gMfZ9X3KeB3w1L8fwVDE-zneVFRgQD7xA3cWH3Q8oLPS9rzgW8iMQl8VCAA
As you can see above, the old typing will sometimes fail to narrow the second generic to fit the provided argument (in this example the second generic is inferred as
"name" | "age"even the provided array is just['name']. The new typing fixes the issue and also make the generics more aligned with the argument type.I have also included an alternative fix for this but I feel like what I am proposing is better since it aligns the generics to the arguments better.
Update: my proposed approach is breaking test when the input object is
any. I have updated to use the alternative solution and added a test to cover this case.Side note: It is surprising to me that
omitdoesn't suffer from the same issue.