Array.isArray type narrows to any[] for ReadonlyArray<T>

[jinder]

trafficstars

TypeScript Version: 2.4.1

Code

declare interface T {
  foo: string;
}

const immutable: T | ReadonlyArray<T> = [];

if (Array.isArray(immutable)) {
  const x = immutable; // Any[]  - Should be ReadonlyArray<T>
} 

const mutable: T | Array<T> = [];

if (Array.isArray(mutable)) {
  const x = mutable; // T[]
} 

Expected behavior: Should type narrow to ReadonlyArray<T>, or at the very least T[].

Actual behavior: Narrows to any[]. Doesn't trigger warnings in noImplicitAny mode either.

[jcalz]

If you add the following declaration to overload the declaration of isArray():

interface ArrayConstructor {
    isArray(arg: ReadonlyArray<any> | any): arg is ReadonlyArray<any>    
}

the post-checked mutable is still narrowed to T[] and the post-checked immutable is narrowed to ReadonlyArray<T> as desired.

I'm not sure if this or similar would be an acceptable addition to the standard typing libraries or not, but you can at least use it yourself.

[jinder]

@jcalz yes, I think these overloads should be in the standard library. There are a few other scenarios where ReadonlyArray isn't accepted where it should be (for example Array.concat).

[vidartf]

The same narrowing problem also exists for this check:

if (immutable instanceof Array) {
  const x = immutable; // Any[]  - Should be ReadonlyArray<T>
} 

I'm not sure if there exists a similar workaround for this.

[jcalz]

I would probably do this if I wanted a workaround:

interface ReadonlyArrayConstructor  {
    new(arrayLength?: number): ReadonlyArray<any>;
    new <T>(arrayLength: number): ReadonlyArray<T>;
    new <T>(...items: T[]): ReadonlyArray<T>;
    (arrayLength?: number): ReadonlyArray<any>;
    <T>(arrayLength: number): ReadonlyArray<T>;
    <T>(...items: T[]): ReadonlyArray<T>;
    isArray(arg: any): arg is ReadonlyArray<any>;
    readonly prototype: ReadonlyArray<any>;
}
const ReadonlyArray = Array as ReadonlyArrayConstructor;

And then later

if (ReadonlyArray.isArray(immutable)) {
  const x = immutable; // ReadonlyArray<T>
} 
if (immutable instanceof ReadonlyArray) {
  const x = immutable; //  ReadonlyArray<T>
} 

but of course, since at runtime there's no way to tell the difference between ReadonlyArray and Array, you would have to be careful to use the right one in the right places in your code. ¯\_(ツ)_/¯

[jinder]

@vidartf As instanceof works at runtime, I'm not sure that should narrow to ReadonlyArray. You're asking if the runtime representation of that is an array, which is true. So I'd expect it to narrow to Array<T>, not ReadonlyArray<T>.

[vidartf]

@jinder I didn't state it explicitly, but my code was meant to be based on yours (same variables and types), so it should already know that it was T | ReadonlyArray<T>. As such, instanceof should narrow it to ReadonlyArray<T>.

[NN---]

What is the best workaround here ? I just ended up with ugly type assertion:

function g(x: number) {}

function f(x: number | ReadonlyArray<number>) {
    if (!Array.isArray(x)) { 
        g(x as number); // :(
    }
}

[adrianheine]

I think this is fixed in 3.0.3.

[jinder]

@adrianheine doesn't seem to be for me.

[adrianheine]

Oh, yeah, I was expecting the code in the original issue to not compile, bu that's not even the issue.

[bluelovers]

let command: readonly string[] | string;

let cached_command: Record<string, any>;

if (Array.isArray(command))
{

}
else
{
	cached_command[command] = 1;
	// => Error: TS2538: Type 'readonly string[]' cannot be used as an index type.
}

[mamiu]

Temporary solution from @aleksey-l (on stackoverflow) until the bug is fixed:

declare global {
    interface ArrayConstructor {
        isArray(arg: ReadonlyArray<any> | any): arg is ReadonlyArray<any>
    }
}

[lll000111]

It is not only ReadonlyArray: #33700

[jwalton]

Here's a concrete example of ReadonlyArray<string> behaving differently than Array. The !Array.isArray() case fails to eliminate the ReadonlyArray<string> when narrowing.

[kambing86]

it should be like this

interface ArrayConstructor {
  isArray(arg: unknown): arg is unknown[] | readonly unknown[];
}

and test it in typescript

const a = ['a', 'b', 'c'];
if (Array.isArray(a)) {
  console.log(a); // a is string[]
} else {
  console.log(a); // a is never
}

const b: readonly string[] = ['1', '2', '3']

if (Array.isArray(b)) {
  console.log(b); // b is readonly string[]
} else {
  console.log(b); // b is never
}

function c(val: string | string[]) {
  if (Array.isArray(val)) {
    console.log(val); // val is string[]
  }
  else {
    console.log(val); // val is string
  }
}

function d(val: string | readonly string[]) {
  if (Array.isArray(val)) {
    console.log(val); // val is readonly string[]
  }
  else {
    console.log(val); // val is string
  }
}

function e(val: string | string[] | readonly string[]) {
  if (Array.isArray(val)) {
    console.log(val); // val is string[] | readonly string[]
  }
  else {
    console.log(val); // val is string
  }
}

[MicahZoltu]

Would a PR that adds the appropriate overload to Array.isArray be reasonable at this point? This issue is marked as In Discussion, but I'm not sure what discussion is necessary. This just feels like a mistake in the definition files, which should be an easy fix that any contributor can submit.

Proposed addition to built-in TS libraries:

interface ArrayConstructor {
    isArray(arg: ReadonlyArray<any> | any): arg is ReadonlyArray<any>    
}

[kg-currenxie]

Any news here? :|

[falsandtru]

Removed ReadonlyArray<any> | any hack

interface ArrayConstructor {
  isArray(arg: any[]): arg is any[];
  isArray(arg: any): arg is readonly any[];
}

[ExE-Boss]

You could also do:

interface ArrayConstructor {
	 isArray(arg: any): arg is any[] | readonly any[];
}

This will work correctly for union types that contain arrays or readonly arrays, and as long as you don’t mutate the result, will even work for any or unknown.

[falsandtru]

It widens readonly any[] to any[] | readonly any[].

[thw0rted]

I'm not sure if this is the same root cause but I'm seeing this now with readonly tuples.

Playground Link

[orta]

This got reverted, re-opening

[valerii15298]

I do not understand why we cannot accept the solution from @kambing86:

interface ArrayConstructor {
  isArray(arg: unknown): arg is unknown[] | readonly unknown[];
}

Why do we need to use any instead of unknown? When I use isArray() then it loses all type information. Solution with unknown preserves types so we can benefit from it, have nice autocomplete, type safety, etc... Tell me please if I am wrong, and why do not use this nice solution?

Really this issue is open since 2017 and seems to be very simple, people even provided ready solutions, just implement. But still type defs for isArray remains the same: isArray(arg: any): arg is any[]; So maybe I do not understand something or this issue is hanging with 86 likes for now:) Or maybe there are some complexities using it with unknown?

[octogonz]

+1

I encountered this problem today with code like this:

// The error goes away if you delete "readonly":
type T = readonly string[] | { color: 'red' }

function f(t:T): void {
  if (!Array.isArray(t)) {
    // Property 'color' does not exist on type 'readonly string[]'
    console.log(t.color);
  }  
}

The above code compiles without errors with @valerii15298's solution.

Barring some unforeseen consequence, this seems like a one-line fix in lib.es5.d.ts.

[RyanCavanaugh]

Barring some unforeseen consequence

I have incredibly bad news... 😅. Every time we touch this thing something breaks. Happy to take a PR to try, though.

[sisp]

The suggested change leads to a backwards incompatibility and (I think) inconsistency though:

function f(v: unknown) {
  if (Array.isArray(v)) {
    // BEFORE:
    // `v` is `any[]`
    //
    // AFTER:
    // `v` is `unknown[] | readonly unknown[]`
  }
}

Currently the narrowed type is a mutable array and the suggested change would make it either mutable or immutable when the variable type is unknown or any.

But I agree that Array.isArray's return type should be unknown[] instead of any[]. And I agree with the problem shown in the original post, I just don't know a solution.

[jablko]

@RyanCavanaugh Would you try this PR? https://github.com/microsoft/TypeScript/pull/48228

I modified https://github.com/microsoft/TypeScript/pull/42316#discussion_r825141259 with the following overloads, to fix this issue and https://github.com/microsoft/TypeScript/issues/33700, without changing the current behavior of any and unknown:

    isArray<T>(arg: ArrayLike<T>): arg is readonly T[];
    isArray<T>(arg: Iterable<T>): arg is readonly T[];

[bluelovers]

export type ITSArrayListMaybeReadonly<T> = T[] | readonly T[];

declare global
{
	interface ArrayConstructor
	{
		isArray<T extends ITSArrayListMaybeReadonly<any>>(arg: T | unknown): arg is T
	}
}

export function isArray<T extends ITSArrayListMaybeReadonly<any>>(value: T | unknown): value is T
{
	return Array.isArray(value)
}

[huan]

[email protected]

  let a: string[] | number = 42

  if (Array.isArray(a)) {
    console.info(a)
    // let a: number & any[]
  }

  a = []

It's really wired that we got a number & any[] in the if (Array.isArray(a)) guard block.

ts(2358)

  // ERROR: The left-hand side of an 'instanceof' expression must be of type 'any', an object type or a type parameter.ts(2358)
  if (a instanceof Array) {
    // ...
  }

[jcalz]

That's just control flow analysis; the type of a has already been narrowed to number upon the assignment of 42. Control flow can't possibly make it into that console.info(a) block unless, somehow, a were both a number and an array, and hence number & any[].

Hmm, you keep editing that comment. Let me know when it stabilizes.