Skip to content
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

TS3.6/3.7 regression: template arguments used in unions no longer resolve/narrow #34754

Closed
mprobst opened this issue Oct 26, 2019 · 14 comments
Closed
Assignees
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@mprobst
Copy link
Contributor

mprobst commented Oct 26, 2019

TypeScript Version: 3.7.1-rc

Search Terms: template argument union

Code

declare class Data {
  string<T>(defaultValue?: string|T): string|T;
}

declare const str: string|undefined;
const x = new Data().string(str);

Expected behavior:

I'd argue string, at least that's what the user intended here.

Actual behavior:

In TypeScript version:

  • 3.5, typeof x === string
  • 3.6, typeof x === unknown
  • 3.7, typeof x === string|undefined

Playground Link: n/a

@mprobst
Copy link
Contributor Author

mprobst commented Oct 26, 2019

I presume this is an intentional change (?) but I could find an existing issue.

@AlCalzone
Copy link
Contributor

IMO the inferred type is the only logical one here. You pass a variable of type string|undefined to an argument expecting string|T, so T is inferred as undefined. The method returns string|T, so I expect the return type to be string|undefined.

@mprobst
Copy link
Contributor Author

mprobst commented Oct 29, 2019

@AlCalzone note the ? in defaultValue?. The parameter is optional, so its real type is string|T|undefined. We're passing string|undefined into string|undefined|T, so T should/could be never, I guess?

@RyanCavanaugh RyanCavanaugh added the Bug A bug in TypeScript label Oct 29, 2019
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 3.8.0 milestone Oct 29, 2019
@RyanCavanaugh
Copy link
Member

I think this is a bug; undefined should indeed be part of the union for the purposes of inference. So T = never and x should be string. Maybe this is the smallest code snippet that can produce different behavior in 4 consecutive versions of TypeScript 😶

@fatcerberus
Copy link

If there's no way to infer T then shouldn't it default to its constraint of unknown (the TS 3.6 behavior), rather than becoming never?

@ahejlsberg
Copy link
Member

The core issue here is what, if anything, to infer for T when inferring from a source type X to a target type X | T?

  1. Make no inference. Problem here is if we make no other inferences for T, we'll end up with the constraint, which typically is the default unknown. That's a terrible outcome because it is less specific than what we started with.
  2. Infer never. This doesn't work if we're inferring for a function that returns just T since it would suddenly imply the function never returns.
  3. Infer X. This is pretty much the only meaningful thing to do.

In 3.5 we'd pick option 3 for a non-union type X and option 2 for a union type X. That was obviously inconsistent. In 3.6 we started out with option 1 and then, prompted by issues, switched to option 3 in the 3.6.3 release. Nothing has changed in 3.7 and we're still doing option 3. I continue to think this is the only meaningful option.

@ahejlsberg
Copy link
Member

See #32460, #32558, and #33252 for more context.

@ahejlsberg ahejlsberg added Working as Intended The behavior described is the intended behavior; this is not a bug and removed Bug A bug in TypeScript labels Oct 30, 2019
@ahejlsberg ahejlsberg removed this from the TypeScript 3.8.0 milestone Oct 30, 2019
@fatcerberus
Copy link

That's a terrible outcome because it is less specific than what we started with.

Maybe, but if this is really the benchmark: doesn't the same thing already happen any time you infer from a source Y to target X | T? X | Y is necessarily less specific than Y.

@ahejlsberg
Copy link
Member

ahejlsberg commented Oct 30, 2019

Maybe, but if this is really the benchmark: doesn't the same thing already happen any time you infer from a source Y to target X | T? X | Y is necessarily less specific than Y.

Yes, but there's no reason an inference from X to X | T should produce a final type that is less specific than X. For example, when inferring from string to string | T, if we infer unknown then the final type ends up being unknown, meaning you could pass an argument of any type.

@fatcerberus
Copy link

I see what you're saying, but from a type-theoretical standpoint I view it as the same kind of question as "Given an empty array [], what is the T in Array<T>"--and in such case we can say T is anything we want because an empty array is always a valid input regardless of what T is.

@nreid260
Copy link

In the Closure compiler we would arrive at the same inference at TS3.7 (`string|undefined') which I believe to be the correct one.

Effectively the question being asked is "what should T be inferred as?". When the inference is against a union containing T the only consistent thing to do is to distribute over all the union elements. That is T is inferred to be the entire type of the passed argument.

As a human, in this case, you probably imagine some kind of "type subtraction" where first we remove the common elements of the argument and parameter unions, before inferring T. However, for cases that don't involve finite types, "subtraction" is not well defined. In this example even, what's the correct result if str was 'foo'|undefined?

@fatcerberus
Copy link

fatcerberus commented Oct 30, 2019

I point back to my empty array example: Given an empty array, what is the T in T[]? And the question itself is nonsensical because no example of T exists; however, practically speaking, we can say T is anything we want without introducing any unsoundness.

@ahejlsberg
Copy link
Member

When someone writes string | T it is a strong indication that T is intended to be disjoint with string. For this reason unknown is a bad inference as it encompasses string. And never is a bad inference for the reason previously outlined. That leaves string, which is what we currently infer.

@typescript-bot
Copy link
Collaborator

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

7 participants