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

Allow type narrowing of non-primitive expressions #44899

Closed
4 tasks done
antoinep92 opened this issue Jul 5, 2021 · 5 comments
Closed
4 tasks done

Allow type narrowing of non-primitive expressions #44899

antoinep92 opened this issue Jul 5, 2021 · 5 comments
Labels
Duplicate An existing issue was already created

Comments

@antoinep92
Copy link

antoinep92 commented Jul 5, 2021

Suggestion

πŸ” Search Terms

narrow type assertion expression

N.B. I looked at ~20 open requests and didn't find a duplicate, but there were many issues related to narrowing/assertions so I might have missed it

βœ… Viability Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
  • [ x This feature would agree with the rest of TypeScript's Design Goals.

⭐ Suggestion

Currently, a variable's type is set when it is declared/initialized, but can be further refined or narrowed within a branch of the code.
Like so:

function generate(): string | undefined { return "test" }
let value : string | undefined = generate(); // at this point, value is string or undefined
if(value) { // (string | undefined) can only be considered true for string, so narrow to string
  console.log(value.length); // ok, because here value is a string
  value = undefined; // invalidate narrowing and recompute value's type
  console.log(value.length); // fail because we know it's undefined
}
console.log(value.length) // fails because there are no active narrowing here

As far as I can tell, this mechanism only applies to single variables, not to more complex expressions.

I suggest we consider extending this to all expressions, with any change to any term of the expression invalidating the assumption.

πŸ“ƒ Motivating Example

Building from the previous toy exemple, consider we now assign to an object instead of a plain variable

const obj : Record<string, string | undefined> = { a: undefined, b: "string" }
for(let key of ["a", "b", "c"]) {
  if(obj[key]) {
    console.log(obj[key].length) // fails
  }
}

This fails because typescript seems unable to propagate type knowledge about expressions in the same way it does with plain variables. The code can be made to work either by adding a manual cast/assertion, or by using a proxy variable / alias:

for(let key of ["a", "b", "c"]) {
  const value = obj[key];
  if(value) {
    console.log(value.length) // ok
  }
}

}

It would be cool if typescript could keep track of types even for expressions. Like this:

  • Every time a condition allows to narrow the type of an expression, register the expression and its narrowing in a map associated with the code linked with that condition.
  • Analyze every statement within that code block, if it writes to any term of the expression, loose the associated assumption below this point.

Notes:

  • If we have a non-primitive expression where all/some terms are narrowed, this narrowing is used to infer the type of the expression ; but this is not the same. My proposal is about narrowing the expression as a whole, and keeping track of that.
  • If the expression includes function calls (either free functions, methods, or properties), then these would have to invalidate the narrowing, at least until we have something like TypeScript#7770 (pure functions).
  • If the expression includes accessing an object, then any other expression involving that may change the object (most of them, except plain field reads, or methods marked with method(this: Readonly<Class>)) should invalidate our assumptions.

πŸ’» Use Cases

This is non critical as the simple workaround is to assign the expression to a variable and narrowing will then work on that variable. The other option is to manually annotate the code with type assertions, although it's error-prone.

From a user point of view there is nothing different between a variable and an expression with regards to type, and it makes sense that they would be treated the same w.r.t. type narrowing. There are many use-cases where it would be convenient. Like when iterating over arrays or objects whose values have a broad static type, but usage indicates a more narrow type. Another simple code example below:

const cache : Record<string, number | undefined> = {};
function impl(x: string): number { return x.length }

function f(x: string): number {
    if(cache[x] === undefined) cache[x] = impl(x);
    return cache[x] as number // hopefuly this cast should not be needed
}

From a compiler point of view, I understand this is not the same thing, as there are infinite ways to build expressions, and it's harder to know when it changes (which is why I suggest invalidating the compound-narrowing as soon as any term changes as a conservative approximation)

@RyanCavanaugh
Copy link
Member

#10530 ?

@antoinep92
Copy link
Author

Thanks. I'm not 100% sure it's the same, there is a const-key vs dynamic-key ambiguity here:

  • The original issue reported in #10530 and also for example this case is about const keys.

  • This issue is about dynamic keys, and I can see that some comments at #10530 also mention this, but there is no feedback on weather it really is the same issue. I guess it really depends on how this will be implemented.

So if these are the same, indeed we can close this, and maybe clarify that #10530 is about both cases. On the other hand, if the scope #10530 is only const-keys, I think we can keep this open (and I'll edit the description to clarify the situation)

@fatcerberus
Copy link

fatcerberus commented Jul 10, 2021

A problem with your example that jumps out at me immediately is that obj[key] may end up calling a getter that mutates the object (or just returns a different value each time). In general, TS doesn't distinguish between plain properties and accessors at the type level.

JS is so dynamic that it's hard to know, in the case of a complex expression, whether or not it will produce observable side effects. When you assign the result of the expression to a local variable, there's much more confidence that its value won't change between accesses (this can still happen in the case of closures but see #9998 for that).

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jul 22, 2021

If key isn't const, then we really can't and shouldn't narrow, because in practice we have no idea when mutations to key might happen. This is enough of a problem for regular variables that it would certainly be a problem for object keys.

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Jul 23, 2021
@RyanCavanaugh
Copy link
Member

Going to treat this as a duplicate of #10530 since the non-const case would not be something we'd handle until the const case was working well anyway

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

3 participants