-
-
Notifications
You must be signed in to change notification settings - Fork 601
Add ExtractStrict
type
#1119
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?
Add ExtractStrict
type
#1119
Conversation
@@ -175,3 +175,6 @@ export type {LastArrayElement} from './source/last-array-element'; | |||
export type {GlobalThis} from './source/global-this'; | |||
export type {PackageJson} from './source/package-json'; | |||
export type {TsConfigJson} from './source/tsconfig-json'; | |||
|
|||
// Improved Built-in |
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.
As per this comment: #291 (comment), I have added this new section/category.
d0134f1
to
e946bbd
Compare
e946bbd
to
800bec3
Compare
union member of `Union` is only allowed to be a subset | ||
of some union member of `Type`. | ||
|
||
Constraint: ∀ U ∈ Union, U ⊆ T, where T ∈ Type |
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.
I can remove if this overcomplicates things, but I found this mathematical notation pretty useful
fields in the given union type `Union`, where each | ||
union member of `Union` is only allowed to be a subset | ||
of some union member of `Type`. |
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.
where each
union member ofUnion
is only allowed to be a subset
of some union member ofType
.
This is not true; consider the following example:
type T = ExtractStrict<{foo: 1; bar: 1} | {baz: 1}, {foo: 1}>;
The above instantiation of Extract
is valid, but {foo: 1}
is neither a subset of {foo: 1; bar: 1}
nor a subset of {baz: 1}
. It is infact a superset of {foo: 1; bar: 1}
.
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.
On the other hand, for something like:
type T = ExtractStrict<string | number, 'foo'>;
// ^? type T = never
'foo'
is now a subset of string
.
If I understand the ask correctly, 'foo'
should ideally not be allowed here, because it doesn't do any extraction.
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.
Thank you for the detailed review!
The above instantiation of Extract is valid, but {foo: 1} is neither a subset of {foo: 1; bar: 1} nor a subset of {baz: 1}. It is infact a superset of {foo: 1; bar: 1}.
You're right here in terms of the type, but I'm open to amending the language to whatever you think makes sense to most users. I was thinking a subset of fields, not of the type. Let U
be a union member of Union
, T
be a union member of Type
. For a type, U
should be a superset of T
(because U
is more loose than T
), but in terms of fields of U
, the fields are a subset of the fields of T
(basically, a Partial
). I can try to make either approach more clear, or am open to whatever language you think makes sense.
If I understand the ask correctly, 'foo' should ideally not be allowed here, because it doesn't do any extraction.
100%. This is a great test case for me to add and calls for a better extends
condition for Union
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.
IMO, it'll be difficult to clearly explain this using technical terms, so it'd be better if we keep the text simple and explain it with examples. Like:
/**
A stricter version of {@link Extract} that ensures every member of `Union` can successfully extract something from `Type`.
For e.g., `StrictExtract<string | number | boolean, number | bigint>` will error because `bigint` cannot extract anything from `string | number | boolean`.
@example
```
// Valid Examples
type Example1 = ExtractStrict<{status: 'success'; data: string[]} | {status: 'error'; error: string}, {status: 'success'}>;
//=> {status: 'success'; data: string[]}
type Example2 = ExtractStrict<'xs' | 's' | 'm' | 'l' | 'xl', 'xs' | 's'>;
//=> 'xs' | 's'
type Example3 = ExtractStrict<{x: number; y: number} | [number, number], unknown[]>;
//=> [number, number]
```
@example
```
// Invalid Examples
type Example1 = ExtractStrict<'xs' | 's' | 'm' | 'l' | 'xl', 'xl' | 'xxl'>;
// ~~~~~~~~~~~~
//=> Error: Type "'xl' | 'xxl'" does not satisfy the constraint 'never'.
type Example2 = ExtractStrict<{x: number; y: number} | {x: string; y: string}, unknown[]>;
// ~~~~~~~~~
//=> Error: Type 'unknown[]' does not satisfy the constraint 'never'.
```
@category Improved Builtin
*/
* is only allowed to be a subset of some union | ||
* member of `Type`. | ||
*/ | ||
Union extends Partial<Exact<Type, Union>>, |
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.
Also, is ExtractStrict
not meant to be used in cases like these:
type Line =
| {x1: number; y1: number; x2: number; y2: number}
| [[x1: number, y1: number], [x2: number, y2: number]]
| [x1: number, y1: number, x2: number, y2: number];
type LineArrayNotations = ExtractStrict<Line, unknown[]>; // Errors
// ^? type LineArrayNotations = [[x1: number, y1: number], [x2: number, y2: number]] | [x1: number, y1: number, x2: number, y2: number];
The above instantiation of ExtractStrict
errors even though LineArrayNotations
is not never
.
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.
IMO, the strictness be simplified to just ensure if Union
is able to successfully extract something from Type
, like:
type ExtractStrict<
Type,
Union extends Extract<Type, Union> extends never ? never : unknown,
> = Extract<Type, Union>;
type T1 = ExtractStrict<1 | 2 | 3 | 4, 1 | 2 | 3>;
// ^? type T1 = 1 | 2 | 3
// @ts-expect-error
type T2 = ExtractStrict<1 | 2 | 3 | 4, "one" | "two">;
// But something like this will also pass
type T3 = ExtractStrict<1 | 2 | 3 | 4, 1 | 2 | "three">;
// ^? type T3 = 1 | 2
Playground: https://tsplay.dev/wQDG1W
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.
Or maybe we can ensure that every member of Union
is able to do some extraction from Type
, like:
type ExtractStrict<
Type,
Union extends [Union] extends [
Union extends unknown ? (Extract<Type, Union> extends never ? never : Union) : never,
]
? unknown
: never,
> = Extract<Type, Union>;
type T1 = ExtractStrict<1 | 2 | 3 | 4, 1 | 2 | 3>;
// ^? type T1 = 1 | 2 | 3
// @ts-expect-error
type T2 = ExtractStrict<1 | 2 | 3 | 4, "one" | "two">;
// @ts-expect-error
type T3 = ExtractStrict<1 | 2 | 3 | 4, 1 | 2 | "three">;
Playground: https://tsplay.dev/m0kXqN
So, both T2
and T3
error now. This also passes all the existing tests.
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.
I like your idea of additionally checking Extract<Type, Union> extends never
. Let me add that in and see what works best. Thanks!
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.
@nigeltroy I'd rather prefer the second approach that I suggested.
Adds
ExtractStrict
type as per issue #222.How it differs from PR #291:
Union
wasUnion extends Type
. This meant that you always had to supply the entire type when extracting, which seemed cumbersome.I have fixed this by slightly modifying the type condition and have added a bunch of tests.
Note on
tsd/expectError()
: Using this doesn't work because it conflicts withtsc
. Adding suppressive comments also don't work becausetsd/expectError()
is not compatible with suppressive comments. Just adding the suppressive comments seems sufficient astsc
will error out if we use a suppressive comment where there is no error.