Skip to content

Type inference ignores explicit type annotation #55330

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

Closed
belyakov-am opened this issue Aug 10, 2023 · 17 comments
Closed

Type inference ignores explicit type annotation #55330

belyakov-am opened this issue Aug 10, 2023 · 17 comments

Comments

@belyakov-am
Copy link

πŸ”Ž Search Terms

type inference
explicit type
narrowing
type annotation

πŸ•— Version & Regression Information

TS version 5.1.6

⏯ Playground Link

https://www.typescriptlang.org/play?#code/IYZwngdgxgBAZgV2gFwJYHsI1JKBBEEVAcwgFsBTCZACggC4YIEyAjCgJwEpGAFD9GVQgKAHmZtOAPhgBvAFAwlMADYVkMZBQAeyRiGQdUEYjAA+TBCpUwAvJesBueYuUQKAdxj9BwijRoOChB0FQA3Ci47GQVlOM0dDXsAchDKBN1k53jlIJDw-yQAEwo4YwoirlclAF8q6uwPYFQNdy8fIREAvNCIqNsZEWQAFVRKdARaHoKAGhgARgAGZcWuLmcG1DgYGi1dO1t7Zmso2JyYIOQEDixFhpqXOMvrrD3kADo1E2QAC2caoA

πŸ’» Code

async function asyncAssignment(n: number): Promise<number> {
    let text: string | null = null;

    new Promise((resolve) => {
        text = 'some text';
        resolve(undefined)
    })

    await new Promise((resolve) => setTimeout(resolve, 10000));

    if (text === null) {
        return 0
    }

    return text.length;
}

πŸ™ Actual behavior

Typescript narrows text type to null and not to string | null

πŸ™‚ Expected behavior

Expecting to have string | null type for text before explicit type narrowing (if (text === null)) and string type after explicit type narrowing

@MartinJohns
Copy link
Contributor

This is working as intended. It's partially a duplicate of #9998 (modification in closure is not considered), and the intentional behavior that assignment narrows the variable (that's why text is typed null).

You can work around this by using a type assertion: let text = null as string | null;

@TmLev
Copy link

TmLev commented Aug 10, 2023

This is working as intended.

IMHO, this is suboptimal and not the behaviour anyone expects from the explicit type annotation.

@MartinJohns
Copy link
Contributor

The behaviour is what most people expect. When they assign a string value to a variable annotated string | null they don't expect it to be suddenly typed string | null, they expect it to be typed string. You're assigning null, so the variable is typed null.

The only reason this is "unexpected" is because of #9998.

@TmLev
Copy link

TmLev commented Aug 10, 2023

When they assign a string value to a variable annotated string | null they don't expect it to be suddenly typed string | null, they expect it to be typed string.

This logic should only apply to const variables, not let variables. let variables can be changed at any point in time – why initial assignment dictates the type for the whole lifetime of the variable?

I find it frustratingly unintuitive.

@MartinJohns
Copy link
Contributor

MartinJohns commented Aug 10, 2023

Consider this example:

let text: string | null = someFunctionReturningStringNotNull();
text.substring(1);

// Later in code:
text = null;

Should this code error? You're proposing yes, most people (including me) expect no.

why initial assignment dictates the type for the whole lifetime of the variable?

It doesn't. It dictates the type until the next assignment. The assignment in OPs example is just not seen due to #9998.


Regardless, this has come up many times already, and the position of the team is clear on this.

@TmLev
Copy link

TmLev commented Aug 10, 2023

Should this code error?

With explicit type annotation? Yes.
Without it (inferring to string)? No.

Regardless, this has come up many times already, and the position of the team is clear on this.

Closing as duplicate?

@MartinJohns
Copy link
Contributor

Without it (inferring to string)? No.

Without a type annotation the variable is typed string, and it's not possible to assign null to it.

@TmLev
Copy link

TmLev commented Aug 10, 2023

Without a type annotation the variable is typed string, and it's not possible to assign null to it.

But the code is explicitly stating possible types: string or null.

It's nice to have Typescript infer the initial type on the first assignment, but ignoring explicit annotation in cases where there are no more visible to Typescript assignments in the following code is beyond my understanding.

In my opinion, inference should never overrule explicit type annotations. Inference should help, not roadblock.

@MartinJohns
Copy link
Contributor

Given my example, your suggestion would result in an error on text.substring(1) with a type annotation (because it's typed string | null), and on text = null without a type annotation (because the type of the variable is inferred to be string, not string | null). Can't win either way.

See also this comment: #51487 (comment)

This used to not be the case, and people found that behavior very annoying and inconsistent, so we changed to this other behavior which is apparently also very annoying and inconsistent.

@TmLev
Copy link

TmLev commented Aug 10, 2023

My point is: if a variable has an explicit type annotation, then this annotation should have higher priority then the inferred type of the first assignment.

In your example, text.substring(1) should be inside an explicit type narrowing if, since variable text has string | null type. There's nothing to win, it's just how Typescript works in all other scenarios (consider function handle(text: string | null)).

@MartinJohns
Copy link
Contributor

MartinJohns commented Aug 10, 2023

In your example, text.substring(1) should be inside an explicit type narrowing if, since variable text has string | null type.

And I argue: It should be typed string, because the function always returns a string, it never returns null (return type of the function is string, not string | null). Why should I check for null, when the type system knows it can never be the case? This will surprise a lot more people.

The culprit in the end is still the missed assignments due to #9998. In all other cases this behaviour is what most people would expect (but maybe not you :-p).

But alas, we're moving in circles.

Another comment by the team: #50444 (comment)

Initialization does perform narrowing. Not doing this opens you up to stuff that just looks broken as heck; this used to not happen and everyone hated it.

@TmLev
Copy link

TmLev commented Aug 10, 2023

Why should I check for null, when the type system knows it can never be the case?

Because the code explicitly states string | null. Don't want to check for null? Remove explicit annotation. Want to widen possible types? Create a new variable.

Initialization does perform narrowing. Not doing this opens you up to stuff that just looks broken as heck; this used to not happen and everyone hated it.

I'm not saying "don't perform narrowing on initialisation". Do it, but respect the explicit type annotation – it's there for a reason.

@MartinJohns
Copy link
Contributor

MartinJohns commented Aug 10, 2023

respect the explicit type annotation – it's there for a reason.

Yep, it is. To limit what values can be assigned to it. That's the purpose of the type annotation, and that's still happening. :-D

It's working as intended, and the suggested change would be a massive breaking change and be so unnecessarily annoying in a lot of code.

Anyway, just circling and rehashing arguments, so I'm out.

@fatcerberus
Copy link

You're proposing that I should have to write:

let x: string | null = "foo";
if (x !== null) doStringThing(x);
x = null;

Just because I might later assign it a null value. That's nonsense.

@TmLev
Copy link

TmLev commented Aug 10, 2023

You're proposing that I should have to write: ...

Write this:

const x = "foo";
doStringThing(x);
const maybeX = condition ? x : null;

@MartinJohns
Copy link
Contributor

MartinJohns commented Aug 10, 2023

Write this:

And the variable has a different scope. So you possible need to declare the variable much earlier, before it has a meaningful value assigned, so it needs to be undefined in the meantime, which is yet another possible value. And null is just one case, if the union has more types in it I need to declare additions variables, for different types and type combinations, depending on the location where the variable gets modified. When using it I need to make sure to include all possible variables. Where does this stop?!

All that mess and extra code for the small corner case of #9998, which can easily be worked around with a type assertion as demonstrated.

@RyanCavanaugh
Copy link
Member

We don't have to guess which behavior is better. The annotated type used to take precedence over the initializer type, and everyone rightly complained that this example didn't work. It also breaks syntactic consistency with having the initialization on the subsequent line. Complaints about the inconsistency you're describing are much rarer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants