-
Notifications
You must be signed in to change notification settings - Fork 664
fix(collections): stricter enforcement on generics #6961
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.08% -0.01%
==========================================
Files 600 600
Lines 43540 43540
Branches 6991 6991
==========================================
- Hits 40965 40964 -1
- Misses 2520 2521 +1
Partials 55 55 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 |
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.