-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Relate control flow to conditional types in return types #33912
Comments
FWIW: this is exactly the same problem faced here in #33014.
Can users not just use an overload to emulate this today? type SomeConditionalType<T> = T extends string ? 1 : -1;
function fn<T>(arg: T): SomeConditionalType<T>;
function fn<T>(arg: T): 1 | -1 {
if (typeof arg === "string") {
return 1;
} else {
return -1;
}
} Is it possible to cleanly handle the following, without rebuilding the CFG on demand in the checker? function fn<T>(arg: T): SomeConditionalType<T> {
return typeof arg === "string" ? 1 : -1;
} |
If generic type parameters could be narrowed via control flow analysis then perhaps this would also address #13995? |
I don't think this is really a solution to type parameter narrowing in general. There are ways that this could be unsound because of the previously discussed points where a type guard doesn't provide sufficient information to narrow a type-variable. Two examples: // Example 1.
type HasX = { x: number }
function hasX(value: unknown): value is HasX {
return typeof value === "object" && value !== null && typeof (<any>value).x === "number"
}
function foo<T>(point: T): T extends HasX ? number : boolean {
if (hasX(point)) {
return point.x
}
return false;
}
const point: { x: number | boolean } = { x: 3 };
const b: boolean = foo(point);
// Example 2.
type SomeConditionalType<T> = T extends string ? 1 : -1;
function fn<T>(arg: T): SomeConditionalType<T> {
if (typeof arg === "string") {
return 1;
} else {
return -1;
}
}
const shouldBe1: -1 = fn("a string" as unknown);
const isOne: 1 = fn("a string");
const isOneMaybe: 1 | - 1 = fn("a string" as string | number); I think the solution is relying on two points: the conditional type is distributive, and the constraint of the check type is 'filterable' with respect to the extends type in each conditional type. That is: Given a type parameter
|
Cross-linking to #22735, the design limitation addressed by this suggestion |
Just an SO question as another reference. |
As proposed, I think this goes too far in the direction of unsoundness. Knowing that a value I think a good starting point would be an example function that is self-consistent in terms of the relationship between its argument types and return type. If How about something like:
Here each conditional specializes the output for a specialized input, like a type-safe overload. Analysis should proceed starting from the return type. If |
@dgreensp I don't believe the proposal in the OP introduces any novel unsoundness, specifically because the logic is only applied to |
I think my examples here show unsound calls. |
@RyanCavanaugh In your example:
Substituting
Or substituting
So I think the compiler should not allow the example. It would be great to design this feature so that all valid substitutions for T produce valid typings, first, and only be more lenient if that proves too limiting. Fundamentally, all the compiler can prove at the site of In my example, the same substitutions are correct, for example substituting
A safe alternative proposal would be... I think, first allow returning a value that's assignable to all the branches of the conditional return value (or the intersection of the branches). So in a function whose return value is In other words, the contextual return type is So in the original example:
Code that is crafted to be "correct," like my example, will work, as follows:
|
My comment neglects the fact that conditionals distribute over unions. The
string | number example should be replaced by one using a different super
type/subtype relationship that isn’t a union, like superclass/subclass, and
the proposal may need to be modified. The bulk of the argument remains the
same.
…On Thu, Jan 9, 2020 at 11:35 AM Jack Williams ***@***.***> wrote:
I think my examples here
<#33912 (comment)>
show unsound calls.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#33912?email_source=notifications&email_token=AABGBJSYR4ETSZH23IENQ3LQ4536NA5CNFSM4I7ERYTKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIRQDJI#issuecomment-572719525>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABGBJRKJTQIE7VLLFALVCLQ4536NANCNFSM4I7ERYTA>
.
|
Overloads work for the original example: function getAddress(obj: Website): URL;
function getAddress(obj: Person): string;
function getAddress(obj: Website | Person) {
if (isWebsite(obj)) {
return obj.url; // no error, error on the overload if the type is changed here
} else if (isPerson(obj)) {
return obj.address; // same
}
throw new Error('oops');
} What are they doing different? |
Here is another use case where it seems to be the same issue: Here a conditional type bound to a method argument and return value are unable to handle the valid cast
|
My workaround (in a simpler example from the official documentation) is explicitly casting to the expected type. Even if it is not an elegant solution and requires an intermediate step with type IdLabel = { id: number };
type NameLabel = { name: string };
type NameOrIdType<T extends number | string> = T extends number
? IdLabel
: NameLabel;
function createLabel<T extends number | string>(
paramNameOrId: T,
): NameOrIdType<T> {
if (typeof paramNameOrId === 'string') {
- return { name: paramNameOrId };
+ return <NameOrIdType<T>>(<unknown>{ name: paramNameOrId });
} else {
- return { id: paramNameOrId };
+ return <NameOrIdType<T>>(<unknown>{ id: paramNameOrId });
}
}
let a = createLabel('typescript'); // { name: 'typescript' }
let b = createLabel(4.1); // { id: 4.1 } |
From the documentation specifically on conditional types, it looks like something like this not only should be possible, but a main use case. The same documentation uses this as a replacement for overloads but, from this issue, it looks like there's no other way. In the documented use case, how would one implement |
Overloads (like in @simonbuchan example) is exactly what I would like to do, but with arrow functions - but unfortunately it seems you can't use overloads with arrow functions, at least what I have found so far, without casting the implementation return type to type Overload = {
(obj: Website): URL;
(obj: Person): string;
}
const getAddress: Overload = (obj): any /* any required here :-( */ => {
if (isWebsite(obj)) {
return obj.url;
} else if (isPerson(obj)) {
return obj.address;
}
throw new Error('oops');
} |
I don't see how the unwieldy inferred return types are a problem. Couldn't you just infer a naïve type but check if an explicit return type annotation exists? Also, I don't think it's necessary to change the inferred type of the return value, but to, when checking the return type, evaluate the conditional type given the known guards in the control flow. |
There are no "external types", only types that are in scope: // same issue
function getAddress<T extends Person | Website>(obj: T) {
return function inner() {
return function inner2() {
return function inner3() {
return function inner4(): T extends Person ? string : URL {
I completely agree that all of the different cases you're bringing up are different cases. :-) All conditional return types are overloads, but not all overloads have conditional return types. You're bringing up a lot of the second case, and those aren't related to this issue. You opened an issue about overloads, cited my comment as support for it, and then came here to say that it's not the same case as overloads. For the record, I don't care at all about the semantics of the existing overload syntax. It's an anti-feature like
You've said "easy to implement" a dozen times or so. When people who actively maintain the compiler on a daily basis are telling you otherwise, you're going to need to produce the PR for anyone to believe you. I don't want to pollute this thread anymore. If you disagree, you can post it in this gist, but you really need to enumerate your cases, because pointing out that there are a lot of different cases without listing them is a poor preface to calling something easy. |
Hey @gabritto any update on this? Honestly one of my biggest pains working w/ TypeScript has been these sorts of weird edge cases in the type system. Also, for this example in the docs, it is still not possible to write an implementation that does not require type assertions. |
@margaretdax I am slowly working on it. I'm close to having a working prototype, but it's been tricky to make it work in a mostly sound way. |
Hello! I think I have a relevant example here: |
Thanks for the example, it has a couple things that don't currently work with my in-progress prototype, but that I hope I can make work. |
Here's another, similar to something I recently did around RabbitMQ. Depending on the type of the exchange, I want the signature of a publish function to be different (Rabbit just being an example, so mocking it here): type FauxAmqpChannel = {
publish(exchange: string, routingKey: string, content: string, options?: Record<string, any>): Promise<void>
}
type DirectExchange = { type: 'direct'; name: string }
type DirectPublisher<T> = (routingKey: string, message: T) => Promise<void>
type FanoutExchange = { type: 'fanout'; name: string }
type FanoutPublisher<T> = (message: T) => Promise<void>
type CreatePublisherFn = {
<T>(channel: FauxAmqpChannel, directExchange: DirectExchange): DirectPublisher<T>
<T>(channel: FauxAmqpChannel, fanoutExchange: FanoutExchange): FanoutPublisher<T>
}
// Type '(routingKey: string, message: T) => Promise<void>' is not assignable to type 'FanoutPublisher<any>'.
// Target signature provides too few arguments. Expected 2 or more, but got 1. [2322]
const createPublisher: CreatePublisherFn = (channel, exchange) => {
if (exchange.type === 'direct') {
return async (routingKey, message) => {
await channel.publish(exchange.name, routingKey, JSON.stringify(message))
}
} else if (exchange.type === 'fanout') {
return async (message) => {
await channel.publish(exchange.name, '', JSON.stringify(message))
}
}
throw new TypeError(`Unknown exchange type: ${exchange}`)
}
declare const channel: FauxAmqpChannel
const directPublisher = createPublisher<string>(channel, { type: 'direct', name: 'direct-exchange' })
const fanoutPublisher = createPublisher<string>(channel, { type: 'fanout', name: 'fanout-exchange' }) The problem is further compounded if you want varying numbers of generic parameters. RabbitMQ has a "topic exchange" that uses structured strings as keys, so it would be nice to be able to strongly type those. The compiler loses all ability to infer input or output types at that point. type FauxAmqpChannel = {
publish(exchange: string, routingKey: string, content: string, options?: Record<string, any>): Promise<void>
}
type DirectExchange = { type: 'direct'; name: string }
type DirectPublisher<T> = (routingKey: string, message: T) => Promise<void>
type FanoutExchange = { type: 'fanout'; name: string }
type FanoutPublisher<T> = (message: T) => Promise<void>
export type TopicExchange<K> = { type: 'topic'; name: string; createKey: (key: K) => string }
export type TopicPublisher<T, K> = (key: K, message: T) => Promise<void>
export type CreatePublisherFn = {
<T>(channel: FauxAmqpChannel, directExchange: DirectExchange): DirectPublisher<T>
<T>(channel: FauxAmqpChannel, fanoutExchange: FanoutExchange): FanoutPublisher<T>
<T, K>(channel: FauxAmqpChannel, topicExchange: TopicExchange<K>): TopicPublisher<T, K>
}
const createPublisher: CreatePublisherFn = (channel, exchange) => {
if (exchange.type === 'direct') {
return async (routingKey, message) => {
await channel.publish(exchange.name, routingKey, JSON.stringify(message))
}
} else if (exchange.type === 'fanout') {
return async (message) => {
await channel.publish(exchange.name, '', JSON.stringify(message))
}
} else if (exchange.type === 'topic') {
return async (key, message) => {
const routingKey = exchange.createKey(key)
await channel.publish(exchange.name, routingKey, JSON.stringify(message))
}
}
throw new TypeError(`Unknown exchange type: ${exchange}`)
}
declare const channel: FauxAmqpChannel
const directPublisher = createPublisher<string>(channel, { type: 'direct', name: 'direct-exchange' })
const fanoutPublisher = createPublisher<string>(channel, { type: 'fanout', name: 'fanout-exchange' }) That case probably makes this more difficult, but it is covered with standard overloading. This is perfectly valid: type VariablyGeneric = {
<T>(values: T[]): string
<K extends string, T>(values: Record<K, T>): string
}
const variablyGeneric: VariablyGeneric = (values: any) => {
if (Array.isArray(values)) {
return values.join(', ')
}
return Object.values(values).join(', ')
} |
Another overloading without usage of type Person = { name: string; address: string; };
type Website = { name: string; url: URL };
declare function isWebsite(w: any): w is Website;
declare function isPerson(p: any): p is Person;
function getAddress<T extends ( Person | Website)>(obj: T): T extends Person ? string : URL
function getAddress<T>(obj: T) {
if (isWebsite(obj))
return obj.url
else if (isPerson(obj))
return obj.address
throw new Error('oops');
} |
Search Terms
control flow conditional return type cannot assign extends
Suggestion
Developers are eager to use conditional types in functions, but this is unergonomic:
The errors here originate in the basic logic:
By some mechanism, this function should not have an error.
Dead Ends
The current logic is that all function return expressions must be assignable to the explicit return type annotation (if one exists), otherwise an error occurs.
A tempting idea is to change the logic to "Collect the return type (using control flow to generate conditional types) and compare that to the annotated return type". This would be a bad idea because the function implementation would effectively reappear in the return type:
For more complex implementation bodies, you could imagine extremely large conditional types being generated. This would be Bad; in most cases functions don't intend to reveal large logic graphs to outside callers or guarantee that that is their implementation.
Proposal Sketch
The basic idea is to modify the contextual typing logic for
return
expressions:Normally
return 1;
would evaluate1
's type to the simple literal type1
, which in turn is not assignable toSomeConditionalType<T>
. Instead, in the presence of a conditional contextual type, TS should examine the control flow graph to find narrowings ofT
and see if it can determine which branch of the conditional type should be chosen (naturally this should occur recursively).In this case,
return 1
would produce the expression typeT extends string ? 1 : never
andreturn -1
would produce the expression typeT extends string ? never : -1
; these two types would both be assignable to the declared return type and the function would check successfully.Challenges
Control flow analysis currently computes the type of an expression given some node in the graph. This process would be different: The type
1
does not have any clear relation toT
. CFA would need to be capable of "looking for"T
s to determine which narrowings are in play that impact the check type of the conditional.Limitations
Like other approaches from contextual typing, this would not work with certain indirections:
Open question: Maybe this isn't specific to
return
expressions? Perhaps this logic should be in play for all contextual typing, not justreturn
statements:Fallbacks
The proposed behavior would have the benefit that TS would be able to detect "flipped branch" scenarios where the developer accidently inverted the conditional (returning
a
when they should have returnedb
and vice versa).That said, if we can't make this work, it's tempting to just change assignability rules specifically for
return
to allowreturn
s that correspond to either side of the conditional - the status quo of requiring very unsafe casts everywhere is not great. We'd miss the directionality detection but that'd be a step up from having totally unsound casts on all branches.Use Cases / Examples
TODO: Many issues have been filed on this already; link them
Workarounds
Checklist
My suggestion meets these guidelines:
The text was updated successfully, but these errors were encountered: