1

I am a bit puzzled about something I have found and I would like to know why TypeScript is behaving this way or what is the reason.

I have this piece of code

class A {
    constructor(public name : String, public x = 0, public y = 0) { }
}

class B extends A {
    constructor(public name : String, public x = 0, public y = 0, public quantity = 1) {
        super(name,x,y)
    }
}

class Foo {
    array1 = [] as A[]
    array2 = [] as A[]
    array3 = [] as A[]
    array4 = [] as B[]

    private arrays : A[][] = [this.array1, this.array2, this.array3]

    foobar (index : number, x : number, y : number) {
        let array = (index < this.arrays.length) ? this.arrays[index] : this.array4
        let AB = (index < this.arrays.length) ? A : B

        //error: let [array, AB] = (index < this.arrays.length) ? [ this.arrays[index], A ] : [ this.array4, B ]

        array.push(new AB(`${AB.toString()}${array.length}`, x, y))
    }
}

If I use the commented part instead of the one before, it will give me a type error for array.push(new AB(…)) saying that an "object of type A is not assignable to type B" and it would miss the quantity property. I would rather expect this type-checking case to succeed.

No other dependency or package is used and I cannot really explain what happens here and whether this is intended behaviour.

ChrisoLosoph
  • 459
  • 4
  • 8
  • 1
    It's a bad idea to try to push onto a union of array types, because they end up getting weird overloads for their methods. Probably you should just annotate `[array, AB]` as `[A[], typeof A]` since those types are compatible, as shown [here](https://tsplay.dev/WGBB0m). Although I'm still a little concerned about that `quantity` property since the compiler can't quite tell that `B` instances will go with the `B` constructor, but ‍♂️. Let me know if you want this written up as an answer. – jcalz Jul 07 '23 at 16:35
  • I would not say, it's generally a bad idea. The types are valid. I am using the concerned function as a function to a higher-level function. That means it could seen as a simple loop body. I don't know a better way to avoid code duplication without putting both array types into one code piece. (I initially placed the other array inside the array of arrays which required additional hardcoding.) I tried multiple ways but your proposed solution somehow works . Maybe, this could work as a minimum solution or answer to the question. – ChrisoLosoph Jul 09 '23 at 10:59
  • 1
    TypeScript really doesn't handle pushing into a union of arrays well; that's why it's a bad idea. TypeScript can't really model what I call *correlated unions* very well, see [ms/TS#30581](https://github.com/microsoft/TypeScript/issues/30581). You can easily write valid code that TS can't properly check. The workaround/approach for correlated unions is usually to change to generics, as described in [ms/TS#47109](https://github.com/microsoft/TypeScript/pull/47109). In your specific example it's possible to just widen to a common tuple type instead of using generics. – jcalz Jul 09 '23 at 15:31

1 Answers1

1

TypeScript can't really model "correlated unions" as described in microsoft/TypeScript#30581.

In your code, the type of array and the type of AB are related to each other in such a way that it's always appropriate to call array.push(new AB(⋯)). But the compiler can't see it. We can look at declare const [array, AB]: [A[], typeof A] | [B[], typeof B] and understand that there are two union members corresponding to two possibilities... and since each possibility works with array.push(new AB(⋯)), the whole thing should work. But TypeScript cannot analyze that single line of code multiple times. It tries to analyze it "at once", and therefore array is of the union type A[] | B[] and AB is of the union type typeof A | typeof B, and it loses the correlation between them, and then worries that maybe you'll push an A onto a B[], which is unsafe.

The general solution to correlated unions is to refactor to use generics instead, as described in detail in microsoft/TypeScript#47109.

But for your example code we don't need to do much refactoring.


We know that B extends A by the definition of B. It also turns out that typeof B extends typeof A. That is, you can use the B constructor as if it were an A constructor, because it accepts the same constructor arguments as A (the quantity parameter is optional), and because it constructs instances that are valid A instances (since B extends A). And TypeScript has covariant arrays (for better or worse, see Why are TypeScript arrays covariant?), so B[] extends A[], and therefore [B[], typeof B] extends [A[], typeof A].

So a value of type [A[], typeof A] | [B[], typeof B] can be treated as a value of type [A[], typeof A]. And that means there's no union anymore. You can therefore write:

let [array, AB]: [A[], typeof A] =
  (index < this.arrays.length) ? 
  [this.arrays[index], A] : [this.array4, B]

and it compiles, and then

array.push(new AB(
  `${AB.toString()}${array.length}`, x, y))

also complies. No union of arrays, so no correlated union issues.

Indeed when you wrote the code in two lines, the compiler automatically collapsed A[] | B[] to A[] via a heuristic rule for determining the best common type. So the problem didn't show up that way because there was no union of arrays.

Hooray, we're done, right? This code compiles, and it's safe, right?


Well, sort of. Yes, what you're doing is safe: you are never passing an A-which-is-not-a-B into an array of B elements. Yes, the compiler accepts it as safe, so that's a good sign.

But no, the compiler isn't really verifying it properly. I could change the push line to this:

array.push(new A("oops", 1, 2)); // okay, no compiler error

and the compiler doesn't complain. And then you can write this and the compiler also won't complain:

const f = new Foo()
f.foobar(100, 0, 0);
f.array4.map(x => x.quantity.toFixed())

But that's a runtime error! We pushed a non-B instance of A onto an array of type B[], and then later accessed its nonexistent quantity property and dereferenced undefined. So even though the compiler is happy with this code, it's a mistake.

TypeScript is intentionally not fully sound, so there are places where unsafe things are allowed.

Here the main issue is that arrays and object properties are related covariantly, which is safe for reading but can be unsafe for writing. Widening B to A or B[] to A[] will never give you a problem if you just read, but can be dangerous if you write. Pushing a value onto an array of type A[] | B[] is inherently problematic in TypeScript for that reason, because pushing is essentially writing to the array.

So you'll have to be careful when writing to types that may have been widened. We can rewrite the code to suppress the error, but we can't just assume the lack of error means everything is definitely fine.

Playground link to code

jcalz
  • 264,269
  • 27
  • 359
  • 360