Add moar helper types to wonder-stuff-core #905
Conversation
🦋 Changeset detectedLatest commit: f9d211f The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
kevinbarabash
requested review from
lillialexis and
somewhatabstract
and removed request for
a team
| /** | ||
| * Remove the keys in `R` from `C`. | ||
| */ | ||
| export type Without<C, R> = Omit<C, keyof R>; |
There was a problem hiding this comment.
I'm kind of questioning the usefulness of this type. It's used primarily in components that use HOCs in webapp. We only had a helper type before because Flow didn't have Omit. I kind of think we should just replace Without with Omit.
There was a problem hiding this comment.
I don't mind either way. It's a nice, clear utility that drops the need for the extra keyof and saves 2 keystrokes. However, if we didn't have it, I wouldn't mind either - but then you have to go find replace all the uses. Is that worth it? I don't know.
I don't think it does any harm existing.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #905 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 97 97
Lines 1393 1390 -3
Branches 359 343 -16
=========================================
- Hits 1393 1390 -3 see 3 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
|
Size Change: 0 B Total Size: 4.67 kB ℹ️ View Unchanged
|
| /** | ||
| * Remove the keys in `R` from `C`. | ||
| */ | ||
| export type Without<C, R> = Omit<C, keyof R>; |
There was a problem hiding this comment.
I don't mind either way. It's a nice, clear utility that drops the need for the extra keyof and saves 2 keystrokes. However, if we didn't have it, I wouldn't mind either - but then you have to go find replace all the uses. Is that worth it? I don't know.
I don't think it does any harm existing.
| @@ -37,3 +37,28 @@ export type Secrets = {readonly [key: string]: SecretString}; | |||
| * Make a read-only type mutable. | |||
| */ | |||
| export type Mutable<T> = {-readonly [P in keyof T]: T[P]}; | |||
|
|
|||
| /** | |||
| * Remove the keys in `R` from `C`. | |||
There was a problem hiding this comment.
suggestion: This feels a little inexact - perhaps:
| * Remove the keys in `R` from `C`. | |
| * Remove from `C` properties that have identical keys to those in `R`. |
There was a problem hiding this comment.
suggestion[change-requested]: Can you add types tests to verify these types are working as expected?
Summary:
A number of repos are defining the same or similar utility types. Let's bring them into wonder-stuff-core so that they're consistent across projects.
Test Plan:
N/A