Type inference/narrowing lost after assignment

[sgoll]

TypeScript Version: 3.1

Search Terms: type inference, type guard, narrowing, lost, assignment

Code

let a: unknown = 'x';

if (typeof a === 'string') {
  // a inferred as `string`
  a = a.substr(0, 5);  // (method) String.substr(from: number, length?: number): string
  // a inferred as `unknown`
  a.length;            // Failure: Object is of type 'unknown'.
}

Expected behavior: This should compile without an error.

Actual behavior: Line 7 fails with: Object is of type 'unknown'.

Playground Link: https://www.typescriptlang.org/play/index.html#src=let%20a%3A%20unknown%20%3D%20'x'%3B%0D%0A%0D%0Aif%20(typeof%20a%20%3D%3D%3D%20'string')%20%7B%0D%0A%20%20%2F%2F%20a%20inferred%20as%20%60string%60%0D%0A%20%20a%20%3D%20a.substr(0%2C%205)%3B%0D%0A%20%20%2F%2F%20a%20inferred%20as%20%60unknown%60%0D%0A%20%20a.length%3B%0D%0A%7D%0D%0A

Related Issues: #18840, #19955, #26673

[weswigham]

@DanielRosenwasser We do not currently narrow non-union types on assignment. For example, given

declare class Foo {
    x: string;
}

declare class Bar extends Foo {
    y: string;
}

declare var x: Foo;

if (x instanceof Bar) {
    x.y;
    x = new Bar();
    x.y;
}

we error on the second x.y as well.

Did we want to change this?

[aleclarson]

👍 Can we expect this in 3.3.0, or no guarantees yet? Here's my use case.

[danielrentz]

Another related simple example:

function reverse1(a: ArrayLike<number>): number[] {
    return Array.from(a).reverse(); // works!
}

function reverse2(a: ArrayLike<number>): number[] {
    a = Array.from(a); // makes "a" a real Array
    return a.reverse(); // error; TS thinks "a" is still of type ArrayLike
}

[hab25]

@DanielRosenwasser We do not currently narrow non-union types on assignment. For example, given

declare class Foo {
    x: string;
}

declare class Bar extends Foo {
    y: string;
}

declare var x: Foo;

if (x instanceof Bar) {
    x.y;
    x = new Bar();
    x.y;
}

we error on the second x.y as well.

Did we want to change this?

Another example, which to me is quite unintuitive, is with objects that contain union typed properties:

// compiles
let myVar: string | number;
myVar = '5';
console.log(myVar.length);

// compiles
let myObj1: {myProp: string | number} = {myProp: 5};
myObj1.myProp = '5';
console.log(myObj1.myProp.length);

// does not compile, emitting the following error:
// TSError: ⨯ Unable to compile TypeScript:
// myt.ts(63,26): error TS2339: Property 'length' does not exist on type 'string | number'.
//   Property 'length' does not exist on type 'number'.
let myObj2: {myProp: string | number} = {myProp: 5};
myObj2 = {myProp: '5'};
console.log(myObj2.myProp.length);

[AlCalzone]

I just stumbled into this aswell, albeit with number instead of string. I can't see any logical reason why assigning a number to a value we know for sure to be a number should require us to narrow again:

declare function takesNumber(value: number): void;

function test(value: unknown) {
  if (typeof value !== "number") return;
  takesNumber(value); // ok
  value = 1; // assign the same type!
  takesNumber(value); // error (value is unknown, although we narrowed it to number before)
}

I would really love to have an unknown type that is less cumbersome to work with. It has been really awkward since it was introduced almost 3 years ago and not much has changed :(.

[justingrant]

According to #45870 (comment), "That an unknown variable does not narrow on assignment is intentional." I assume it was intended for a good reason. What's the reason?

This issue has admittedly been painful for us while porting the TC39 Temporal polyfill from JS to TS. The original JS polyfill does a lot of mutation of parameters, usually for type coercion. For example, a function may take a parameter which can either be a string or an object, and if it's an object then it's coerced to a string in the first line of the function. Later code knows that it's always a string, like old-school JS's runtime equivalent of a TS type guard function.

To port this kind of code to TS, we have many bad alternatives:

1. Add type assertions after the assignment (which may be 10s of lines in a long function!). This partly undoes the benefit of TS while making the code hard to read and harder to refactor.
2. Create new variables instead of mutating types. This requires a delicate manual (can't use TS IDE refactor) bisection to rename all usage after the refactor. It's easy to mess up sometimes, esp. when assignments are in if blocks or assignments that use the pre-assignment value in the RHS. Also, reusing temporary variables is prone to unexpected mutation.
3. Retain any for many functions, which negates much of the value of TS.
4. Rewrite in idiomatic TS (e.g. remove most let and enforce no parameter mutation). The JS polyfill is still being changed as browser implementers find bugs, so we want to diverge the codebases as little as possible. Also we're lazy and don't want to rewrite working code.

For a real-world example, today I screwed up a bisection refactor. Here's original JS:

function TotalDurationNanoseconds(days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds, offsetShift ) {
  if (days !== 0) nanoseconds = bigInt(nanoseconds).subtract(offsetShift);
  hours = bigInt(hours).add(bigInt(days).multiply(24));
  minutes = bigInt(minutes).add(hours.multiply(60));
  seconds = bigInt(seconds).add(minutes.multiply(60));
  milliseconds = bigInt(milliseconds).add(seconds.multiply(1000));
  microseconds = bigInt(microseconds).add(milliseconds.multiply(1000));
  return bigInt(nanoseconds).add(microseconds.multiply(1000));
}

If assignments worked like type guard functions, then AFAICT I wouldn't have had to make any changes to the body of the JS function at all. Below is my naive expectation of how types should change after each assignments.

Note that the big-integer functions accept either number or bigInt.BigInteger parameters, but always return a bigInt.BigInteger.

function TotalDurationNanoseconds(days: number, hours: number, minutes: number, seconds: number, milliseconds: number, microseconds: number, nanoseconds: number, offsetShift: number) {
  if (days !== 0) nanoseconds = bigInt(nanoseconds).subtract(offsetShift);
  // => nanoseconds: number | bigInt.BigInteger
  hours = bigInt(hours).add(bigInt(days).multiply(24));
  // => hours: bigInt.BigInteger
  minutes = bigInt(minutes).add(hours.multiply(60));
  // => minutes: bigInt.BigInteger
  seconds = bigInt(seconds).add(minutes.multiply(60));
  // => seconds: bigInt.BigInteger
  milliseconds = bigInt(milliseconds).add(seconds.multiply(1000));
  // => milliseconds: bigInt.BigInteger
  microseconds = bigInt(microseconds).add(milliseconds.multiply(1000));
  // => microseconds: bigInt.BigInteger
  return bigInt(nanoseconds).add(microseconds.multiply(1000));
  // => return type: bigInt.BigInteger
}

But because assignment doesn't act like type guards, I had to do a non-trivial refactor:

• 7 different variables to bisect
• Must declare nanoseconds outside the if statement
• Need to individually examine each use of a variable to decide on the old or new name. Can't simply search-n-replace post-assignment, because each assignment uses the original variable as input.
• There's a helpful pattern for 5 of the variables, but the pattern may make it easier to miss or mess up on the other two.
• We wanted a small diff to avoid bugs, to make it easier to review, to stay closer to the spec, and to avoid unnecessary variation from the original JS.

Here's my buggy initial port. Keep in mind this was just one of hundreds I ported that day. 😄

export function TotalDurationNanoseconds(
  daysParam: number,
  hoursParam: number,
  minutesParam: number,
  secondsParam: number,
  millisecondsParam: number,
  microsecondsParam: number,
  nanosecondsParam: number,
  offsetShift: number
) {
  let days: bigInt.BigInteger, nanoseconds: bigInt.BigInteger;
  if (daysParam !== 0) nanoseconds = bigInt(nanosecondsParam).subtract(offsetShift);
  let hours = bigInt(hoursParam).add(bigInt(days).multiply(24));
  let minutes = bigInt(minutesParam).add(hours.multiply(60));
  let seconds = bigInt(secondsParam).add(minutes.multiply(60));
  let milliseconds = bigInt(millisecondsParam).add(seconds.multiply(1000));
  let microseconds = bigInt(microsecondsParam).add(milliseconds.multiply(1000));
  return bigInt(nanoseconds).add(microseconds.multiply(1000));
}

The bug is forgetting to initialize days = daysParam and nanoseconds = nanosecondsParam. This is sloppy, but also seems like a completely unnecessary bug because IMHO I shouldn't have had to make any code changes at all beyond typing the parameters.

Why can't assignment act like a type guard function? Is the current behavior designed this way because it has big benefits in some use cases? Or is it a technical limitation, e.g. types can be narrowed but not expanded/changed?

cc @12wrigja

[corymharper]

I also ran into this issue today while trying to use unknown. It's rather fine that assignment itself doesn't narrow types, that would just be nice. However, the unintuitive part for me is that it actually broadens an already narrowed type. So if I have type checked that I am working with an array, if at any point I do a reassignment on that variable, I'll lose the narrowing that has already been done and have unknown again. I'm left with the option of creating a new variable after doing the type narrowing and assigning it to the now narrowed variable, then the type for that variable will be inferred as the correct type, which is just really awkward and creates unneeded variables.

For arrays in Javascript this is particularly noticeable if you have broken down multiple method calls onto separate lines using reassignments, you just can't do it if the variable was originally of type unknown.

[titouandk]

Any news on the subject guys? 😶‍🌫️

The number of duplicates seems to indicate that there is a real need for this feature 👀

Thanks :)

[sparecycles]

Just came to report this myself.

Search terms "type narrowing lost after assignment".

Kudos to OP for naming the issue perfectly.

[rChaoz]

Worth noting this also happens with {}, not just unknown