Remove assignability cases in getNarrowedType + an isArray improvement for readonly arrays #39258

orta · 2020-06-25T16:28:15Z

There are two pieces to this PR:

The fix for 'instanceof' changes type outside of 'if' statement #31155 which came from Fix #31155 - Remove assignability cases in getNarrowedType #32443
Unblocking Array.isArray type narrows to any[] for ReadonlyArray<T> #17002 which stopped Fix #31155 - Remove assignability cases in getNarrowedType #32443

The 2nd was blocked on :

TLDR: any[] is assignable to IAction | ReadonlyArray, but it's not a subtype. The issue here is that readonly arrays are not subtypes of mutable arrays so we end up falling back to the intersection case.

Where :

interface IAction {
    x: string;
}

declare const arg: IAction | ReadonlyArray<IAction>;
const x = Array.isArray(arg) ? arg : [arg];

Is now legal due to changes in the isArray definition which takes into account readonly arrays.

/cc @jack-williams

RyanCavanaugh · 2020-06-25T16:45:01Z

@typescript-bot test this
@typescript-bot user test this

typescript-bot · 2020-06-25T16:45:04Z

Heya @RyanCavanaugh, I've started to run the parallelized community code test suite on this PR at 855fd84. You can monitor the build here.

typescript-bot · 2020-06-25T16:45:04Z

Heya @RyanCavanaugh, I've started to run the extended test suite on this PR at 855fd84. You can monitor the build here.

typescript-bot · 2020-06-25T17:11:57Z

The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master.

RyanCavanaugh · 2020-06-25T17:21:45Z

@typescript-bot test this
@typescript-bot user test this

typescript-bot · 2020-06-25T17:21:49Z

Heya @RyanCavanaugh, I've started to run the parallelized community code test suite on this PR at ed01085. You can monitor the build here.

typescript-bot · 2020-06-25T17:21:49Z

Heya @RyanCavanaugh, I've started to run the extended test suite on this PR at ed01085. You can monitor the build here.

RyanCavanaugh

I don't think this is going to work. See https://mseng.visualstudio.com/Typescript/_git/Typescript/pullrequest/560730?_a=files for RWC breaks

jack-williams · 2020-06-26T08:47:31Z

Seems to be tripping up when the guarded value has type any and it picks both parts of the conditional when it should really pick the false branch. Could something like the following work?

declare function isArray<T>(arg: T | {}): arg is T extends readonly (infer U)[] ? GroundArray<T,U>: any[];
type GroundArray<T extends readonly unknown[], U> = [T] extends [U[]] ? any[] : T;

weswigham · 2020-07-01T17:50:18Z

isArray<T>(arg: T | {}): arg is T extends readonly any[] ? (unknown extends T ? never : readonly any[]) : any[];

should also work to filter out any from the true branch.

orta · 2020-07-08T16:56:40Z

@typescript-bot test this
@typescript-bot user test this

typescript-bot · 2020-07-08T16:56:43Z

Heya @orta, I've started to run the parallelized community code test suite on this PR at c69b255. You can monitor the build here.

typescript-bot · 2020-07-08T16:56:43Z

Heya @orta, I've started to run the extended test suite on this PR at c69b255. You can monitor the build here.

orta · 2020-07-13T13:38:39Z

OK, so that does clean up most of the errors in the RWC in the azure side - it looks like just a few rxjs issues and they all stem to their own custom isArray function which looks like:

export const isArray = (() => Array.isArray || (<T>(x: any): x is T[] => x && typeof x.length === 'number'))();

We're still seeing issues in this PR: typescript-bot#53

[XX:XX:XX] Error: /vscode/src/vs/base/browser/ui/actionbar/actionbar.ts(628,9): Type '(readonly IAction[] & any[]) | (IAction & any[]) | (IAction | readonly IAction[])[]' is not assignable to type 'readonly IAction[]'.
   Type '(IAction | readonly IAction[])[]' is not assignable to type 'readonly IAction[]'.
     Type 'IAction | readonly IAction[]' is not assignable to type 'IAction'.
       Type 'readonly IAction[]' is missing the following properties from type 'IAction': id, label, tooltip, class, and 4 more

this could be because they have their own isArray in vscode which wouldn't propagate the readonly:

export function isArray(array: any): array is any[] {
	return Array.isArray(array);
}

I could update the vscode definition to the new definition to see if that fixes it

weswigham · 2020-07-13T15:25:01Z

Yeah, the custom isArray functions would need to adopt the same type, both to be assignment compatible and to behave the same way.

orta · 2020-07-13T19:17:29Z

Looks like I need a build to verify rxjs

@typescript-bot pack this

typescript-bot · 2020-07-13T19:17:33Z

Heya @orta, I've started to run the tarball bundle task on this PR at c69b255. You can monitor the build here.

typescript-bot · 2020-07-13T19:35:52Z

Hey @orta, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/79666/artifacts?artifactName=tgz&fileId=4DD38BEE4F055DD0797EA952DE69E613AD7160224DB562CEDFF4A4AC7DFDA0FA02&fileName=/typescript-4.0.0-insiders.20200713.tgz"
    }
}

and then running npm install.

There is also a playground for this build.

orta · 2020-07-30T12:50:14Z

@typescript-bot pack this

orta · 2020-09-03T15:26:52Z

I'll merge on tuesday when I'm off DT rotation 👍🏻

orta · 2020-09-08T18:43:42Z

Today is that tuesday, I'm going to merge 👍🏻

Typescript 4.1 will have [stricter rules for type predicates](microsoft/TypeScript#39258) like ```ts export const isObject = (val: unknown): val is Record<any, any> => val !== null && typeof val === 'object' ``` As a result of these rules, an expression in collectionHandlers.ts in the reactivity package doesn't get narrowed to the type it did in TS 4.0 and below. Instead it gets an intersection type, which doesn't work with subsequent code. I restored the old type using a cast, since that's essentially what the old version of Typescript was doing here.

RyanCavanaugh · 2020-10-19T15:57:37Z

Example for blog post

declare const p: string | ReadonlyArray<string>;
if (Array.isArray(p)) {
  // 4.0: p: any[]
  // 4.1: p: readonly string[];

  // 4.0: OK
  // 4.1: Error
  p.push(0);
}

laughinghan · 2020-11-02T21:18:00Z

FWIW, possibly a better example for the blogpost is one where TypeScript narrowed the type incorrectly and unsafely, rather than merely providing a weaker-than-desired type, e.g.:

declare var foo: string[] | readonly number[] | null

if (foo instanceof Array) {
    foo; // at runtime, this may be string[] or readonly number[],
         // however, TypeScript <4.1 would narrow to just string[]

    foo.push('str'); // incorrect and unsafe, but allowed by <4.1, now errors in Nightly
} else {
    foo; // at runtime, strictly null, however TypeScript <4.1 would
         // narrow to readonly number[] | null
}

(Playground Link)

felixfbecker · 2020-11-20T10:57:36Z

@orta I think this broke passing Array.isArray to other type-guard-accepting functions, e.g. array.filter(): #41610

victorgarciaesgi · 2020-11-20T11:02:23Z

@orta I think this broke passing Array.isArray to other type-guard-accepting functions, e.g. array.filter(): #41610

It brokes a lot de array.map for me too. Only happens on recursive types for me #41609

MartinJohns · 2020-11-20T15:50:47Z

@orta / @RyanCavanaugh This PR has the Breaking Change label, but I don't see any mention of it on the "Breaking Changes" documentation:

https://github.com/Microsoft/TypeScript/wiki/Breaking-Changes#typescript-41

yseymour · 2020-11-20T23:32:08Z

This is broken for generics and other nontrivial types:

declare function f1<T extends any[]>(array: T): void;
function f2<T>(thing: T) {
    if (Array.isArray(thing)) {
        f1(thing);  // Argument of type 'T & (T extends readonly any[] ? unknown extends T ? never : readonly any[] : any[])' is not assignable to parameter of type 'any[]'.
    }
}

In 4.0, the type of thing was narrowed to T & any[] which was OK.

MartinJohns · 2020-11-28T18:32:52Z

Another issue caused by the improvement: #41714

orta added 2 commits June 24, 2020 14:07

Explore using a different isArray declaration

636bc9d

Add tests and the new isArray definition

e81d845

orta requested review from sandersn and sheetalkamat June 25, 2020 16:28

orta self-assigned this Jun 25, 2020

typescript-bot added the Author: Team label Jun 25, 2020

Baseline updates

ed01085

orta force-pushed the 31155_2 branch from 855fd84 to ed01085 Compare June 25, 2020 16:52

typescript-bot mentioned this pull request Jun 25, 2020

🤖 User test baselines have changed for 31155_2 typescript-bot/TypeScript#53

Open

RyanCavanaugh requested changes Jun 25, 2020

View reviewed changes

Upda the isArray type

c69b255

orta mentioned this pull request Jul 13, 2020

Support the new 4.0 definition of isArray microsoft/vscode#102413

Merged

orta merged commit fa89ce6 into microsoft:master Sep 8, 2020

orta mentioned this pull request Sep 10, 2020

Array#map() is not callable on type-guarded array #40463

Closed

This was referenced Sep 23, 2020

Fix Typescript 4.1 compile error vuejs/core#2219

Merged

🤖 User test baselines have changed for refs/heads/master #40156

Merged

RyanCavanaugh added the Breaking Change Would introduce errors in existing code label Oct 19, 2020

cyrilletuzi mentioned this pull request Oct 19, 2020

Accept readonly arrays in schema options cyrilletuzi/angular-async-local-storage#470

Closed

3 tasks

felixfbecker mentioned this pull request Nov 20, 2020

Can no longer pass Array.isArray to function accepting type guard to narrow type, e.g. array.filter(Array.isArray) #41610

Closed

RyanCavanaugh mentioned this pull request Dec 3, 2020

4.1 and 4.2-dev fails on "TS7053: Element implicitly has an 'any' type" #41658

Closed

orta added a commit to orta/TypeScript that referenced this pull request Dec 7, 2020

Undo microsoft#39258 (isArray changes) see overview at microsoft#41808

d2a52c5

orta mentioned this pull request Dec 7, 2020

[release-4.1] Undo #39258 (isArray changes) - see overview at #41808 #41849

Merged

orta added a commit to orta/TypeScript that referenced this pull request Dec 7, 2020

Undo microsoft#39258 (isArray changes) see overview at microsoft#41808

1312509

orta pushed a commit that referenced this pull request Dec 7, 2020

Undo #39258 (isArray changes) see overview at #41808 (#41849)

abf8815

orta pushed a commit that referenced this pull request Dec 7, 2020

Undo #39258 (isArray changes) see overview at #41808 (#41851)

71e881a

This was referenced Jan 6, 2021

Revert assignability cases in getNarrowedType #42231

Merged

A second attempt at readonly array support persisting through isArray #42316

Closed

xiaoxiangmoe mentioned this pull request Jan 27, 2021

feat: use ReadonlyArray in props react-component/table#574

Merged

dependabot bot mentioned this pull request Mar 14, 2021

Bump typescript from 4.0.7 to 4.1.5 in /frontend DhruvGheewala/Decoder#122

Merged

tom-sherman mentioned this pull request May 24, 2021

feat: Accept ReadonlyArray<T> for enum members graphql-nexus/nexus#916

Merged

IllusionMH mentioned this pull request Apr 21, 2022

TS looses previously determined type #48901

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove assignability cases in getNarrowedType + an isArray improvement for readonly arrays #39258

Remove assignability cases in getNarrowedType + an isArray improvement for readonly arrays #39258

orta commented Jun 25, 2020

RyanCavanaugh commented Jun 25, 2020

typescript-bot commented Jun 25, 2020 •

edited

Loading

typescript-bot commented Jun 25, 2020 •

edited

Loading

typescript-bot commented Jun 25, 2020

RyanCavanaugh commented Jun 25, 2020

typescript-bot commented Jun 25, 2020 •

edited

Loading

typescript-bot commented Jun 25, 2020 •

edited

Loading

RyanCavanaugh left a comment

jack-williams commented Jun 26, 2020 •

edited

Loading

weswigham commented Jul 1, 2020

orta commented Jul 8, 2020

typescript-bot commented Jul 8, 2020 •

edited

Loading

typescript-bot commented Jul 8, 2020 •

edited

Loading

orta commented Jul 13, 2020

weswigham commented Jul 13, 2020

orta commented Jul 13, 2020

typescript-bot commented Jul 13, 2020 •

edited

Loading

typescript-bot commented Jul 13, 2020 •

edited

Loading

orta commented Jul 30, 2020

orta commented Sep 3, 2020

orta commented Sep 8, 2020

RyanCavanaugh commented Oct 19, 2020

laughinghan commented Nov 2, 2020

felixfbecker commented Nov 20, 2020

victorgarciaesgi commented Nov 20, 2020

MartinJohns commented Nov 20, 2020 •

edited

Loading

yseymour commented Nov 20, 2020

MartinJohns commented Nov 28, 2020

Remove assignability cases in getNarrowedType + an isArray improvement for readonly arrays #39258

Remove assignability cases in getNarrowedType + an isArray improvement for readonly arrays #39258

Conversation

orta commented Jun 25, 2020

RyanCavanaugh commented Jun 25, 2020

typescript-bot commented Jun 25, 2020 • edited Loading

typescript-bot commented Jun 25, 2020 • edited Loading

typescript-bot commented Jun 25, 2020

RyanCavanaugh commented Jun 25, 2020

typescript-bot commented Jun 25, 2020 • edited Loading

typescript-bot commented Jun 25, 2020 • edited Loading

RyanCavanaugh left a comment

Choose a reason for hiding this comment

jack-williams commented Jun 26, 2020 • edited Loading

weswigham commented Jul 1, 2020

orta commented Jul 8, 2020

typescript-bot commented Jul 8, 2020 • edited Loading

typescript-bot commented Jul 8, 2020 • edited Loading

orta commented Jul 13, 2020

weswigham commented Jul 13, 2020

orta commented Jul 13, 2020

typescript-bot commented Jul 13, 2020 • edited Loading

typescript-bot commented Jul 13, 2020 • edited Loading

orta commented Jul 30, 2020

orta commented Sep 3, 2020

orta commented Sep 8, 2020

RyanCavanaugh commented Oct 19, 2020

laughinghan commented Nov 2, 2020

felixfbecker commented Nov 20, 2020

victorgarciaesgi commented Nov 20, 2020

MartinJohns commented Nov 20, 2020 • edited Loading

yseymour commented Nov 20, 2020

MartinJohns commented Nov 28, 2020

typescript-bot commented Jun 25, 2020 •

edited

Loading

typescript-bot commented Jun 25, 2020 •

edited

Loading

typescript-bot commented Jun 25, 2020 •

edited

Loading

typescript-bot commented Jun 25, 2020 •

edited

Loading

jack-williams commented Jun 26, 2020 •

edited

Loading

typescript-bot commented Jul 8, 2020 •

edited

Loading

typescript-bot commented Jul 8, 2020 •

edited

Loading

typescript-bot commented Jul 13, 2020 •

edited

Loading

typescript-bot commented Jul 13, 2020 •

edited

Loading

MartinJohns commented Nov 20, 2020 •

edited

Loading