isArray type guards still causing issues in latest nightly build

[mjbvz]

Bug Report

🔎 Search Terms

• isArray

🕗 Version & Regression Information

Tested in 4.3.0-dev.20210211 which

💻 Code

In the VS Code codebase, we are still seeing the isArray type guards causing problems with the latest TS 4.3 nightly build.

Here's an example from this file

The type of DocumentSelector is string | vscode.DocumentFilter | readonly (string | vscode.DocumentFilter)[]

	export function from(selector: vscode.DocumentSelector | undefined): languageSelector.LanguageSelector | undefined {
		if (!selector) {
			return undefined;
		} else if (Array.isArray(selector)) {
			return <languageSelector.LanguageSelector>selector.map(from);
		} else if (typeof selector === 'string') {
			return selector;
		} else {
			return <languageSelector.LanguageFilter>{
				language: selector.language,
				scheme: selector.scheme,
				pattern: typeof selector.pattern === 'undefined' ? undefined : GlobPattern.from(selector.pattern),
				exclusive: selector.exclusive
			};
		}
	}

The error in the else block on the line language: selector.language:

[watch-client    ] [13:45:27] Error: /Users/matb/projects/vscode/src/vs/workbench/api/common/extHostTypeConverters.ts(1284,24): Property 'language' does not exist on type 'DocumentFilter | readonly (string | DocumentFilter)[]'.
[watch-client    ]   Property 'language' does not exist on type 'readonly (string | DocumentFilter)[]'.

These errors did not show up with typescript@4.2.0-dev.20201207.

Here are the other errors:

[watch-client    ] [13:45:27] Error: /Users/matb/projects/vscode/src/vs/workbench/services/extensions/common/abstractExtensionService.ts(545,58): Argument of type 'ILocalization[] | IAuthenticationContribution[] | IDebugger[] | { [location: string]: IView[]; } | ISnippet[] | IConfiguration | ... 12 more ... | undefined' is not assignable to parameter of type 'T'.
[watch-client    ]   'T' could be instantiated with an arbitrary type which could be unrelated to 'ILocalization[] | IAuthenticationContribution[] | IDebugger[] | { [location: string]: IView[]; } | ISnippet[] | IConfiguration | ... 12 more ... | undefined'.
[watch-client    ]     Type 'undefined' is not assignable to type 'T'.
[watch-client    ]       'T' could be instantiated with an arbitrary type which could be unrelated to 'undefined'.
[watch-client    ] [13:45:27] Error: /Users/matb/projects/vscode/src/vs/workbench/services/extensions/common/abstractExtensionService.ts(700,6): Type 'ILocalization[] | IAuthenticationContribution[] | IDebugger[] | { [location: string]: IView[]; } | ISnippet[] | IConfiguration | ... 12 more ... | undefined' is not assignable to type 'T'.
[watch-client    ]   'T' could be instantiated with an arbitrary type which could be unrelated to 'ILocalization[] | IAuthenticationContribution[] | IDebugger[] | { [location: string]: IView[]; } | ISnippet[] | IConfiguration | ... 12 more ... | undefined'.
[watch-client    ]     Type 'undefined' is not assignable to type 'T'.
[watch-client    ]       'T' could be instantiated with an arbitrary type which could be unrelated to 'undefined'.
[watch-client    ] [13:45:27] Error: /Users/matb/projects/vscode/src/vs/base/browser/ui/dropdown/dropdownActionViewItem.ts(175,99): Property 'getActions' does not exist on type 'readonly IAction[] | IActionProvider'.
[watch-client    ]   Property 'getActions' does not exist on type 'readonly IAction[]'.
[watch-client    ] [13:45:27] Error: /Users/matb/projects/vscode/src/vs/workbench/api/common/extHostTypeConverters.ts(1284,24): Property 'language' does not exist on type 'DocumentFilter | readonly (string | DocumentFilter)[]'.
[watch-client    ]   Property 'language' does not exist on type 'readonly (string | DocumentFilter)[]'.
[watch-client    ] [13:45:27] Error: /Users/matb/projects/vscode/src/vs/workbench/api/common/extHostTypeConverters.ts(1285,22): Property 'scheme' does not exist on type 'DocumentFilter | readonly (string | DocumentFilter)[]'.
[watch-client    ]   Property 'scheme' does not exist on type 'readonly (string | DocumentFilter)[]'.
[watch-client    ] [13:45:27] Error: /Users/matb/projects/vscode/src/vs/workbench/api/common/extHostTypeConverters.ts(1286,30): Property 'pattern' does not exist on type 'DocumentFilter | readonly (string | DocumentFilter)[]'.
[watch-client    ]   Property 'pattern' does not exist on type 'readonly (string | DocumentFilter)[]'.
[watch-client    ] [13:45:27] Error: /Users/matb/projects/vscode/src/vs/workbench/api/common/extHostTypeConverters.ts(1286,94): Property 'pattern' does not exist on type 'DocumentFilter | readonly (string | DocumentFilter)[]'.
[watch-client    ]   Property 'pattern' does not exist on type 'readonly (string | DocumentFilter)[]'.
[watch-client    ] [13:45:27] Error: /Users/matb/projects/vscode/src/vs/workbench/api/common/extHostTypeConverters.ts(1287,25): Property 'exclusive' does not exist on type 'DocumentFilter | readonly (string | DocumentFilter)[]'.
[watch-client    ]   Property 'exclusive' does not exist on type 'readonly (string | DocumentFilter)[]'.
[watch-client    ] [13:45:27] Error: /Users/matb/projects/vscode/src/vs/workbench/api/common/extHost.api.impl.ts(201,26): Property 'scheme' does not exist on type 'DocumentFilter | readonly (string | DocumentFilter)[]'.
[watch-client    ]   Property 'scheme' does not exist on type 'readonly (string | DocumentFilter)[]'.
[watch-client    ] [13:45:27] Error: /Users/matb/projects/vscode/src/vs/workbench/api/common/extHost.api.impl.ts(204,58): Property 'exclusive' does not exist on type 'DocumentFilter | readonly (string | DocumentFilter)[]'.
[watch-client    ]   Property 'exclusive' does not exist on type 'readonly (string | DocumentFilter)[]'.
[watch-client    ] [13:45:27] Error: /Users/matb/projects/vscode/src/vs/editor/common/modes/languageSelector.ts(58,11): Property 'language' does not exist on type 'LanguageFilter | readonly (string | LanguageFilter)[]'.
[watch-client    ] [13:45:27] Error: /Users/matb/projects/vscode/src/vs/editor/common/modes/languageSelector.ts(58,21): Property 'pattern' does not exist on type 'LanguageFilter | readonly (string | LanguageFilter)[]'.
[watch-client    ] [13:45:27] Error: /Users/matb/projects/vscode/src/vs/editor/common/modes/languageSelector.ts(58,30): Property 'scheme' does not exist on type 'LanguageFilter | readonly (string | LanguageFilter)[]'.
[watch-client    ] [13:45:27] Error: /Users/matb/projects/vscode/src/vs/editor/common/modes/languageSelector.ts(58,38): Property 'hasAccessToAllModels' does not exist on type 'LanguageFilter | readonly (string | LanguageFilter)[]'.
[watch-client    ] [13:45:27] Error: /Users/matb/projects/vscode/src/vs/editor/common/modes/languageFeatureRegistry.ts(28,21): Property 'exclusive' does not exist on type 'LanguageFilter | readonly (string | LanguageFilter)[]'.
[watch-client    ]   Property 'exclusive' does not exist on type 'readonly (string | LanguageFilter)[]'.

@orta @RyanCavanaugh I believed that #41851 was supposed to fix these issues. Can you please take a look at our errors and me know if we need to change anything on the VS Code side or if these are unexpected errors

[orta]

That's likely the result of microsoft/vscode#102413 which used to rely on some of the changes - I've shipped microsoft/vscode#116571 which reverts those changes too

[mjbvz]

Thanks for taking a look. After merging that commit however, I'm still seeing these errors. I tested with both the latest nightly and the 4.2-rc.

It looks like this location for example is using the standard Array.isArray function: https://github.com/microsoft/vscode/blob/868fb4c39f3e07899d7e88317510fcdab13b2849/src/vs/workbench/api/common/extHostTypeConverters.ts#L1338

[orta]

It looks like this code only worked on nightly builds of 4.2 while the isArray narrowing was still in and before it got pulled out prior to the RC. Correctly narrowing via this check isArray seems to be a lot of trade-offs which we've concluded broke too much prod code. I've re-opened #31155 which is the underlaying issue (switch 'instanceof' with 'Array.isArray' for this explanation #31155 (comment)

You can try this playground with any of the prod builds in the versions (and then try 4.2.0-dev.20201207)

IMO, this probably needs code changes on the vscode side, perhaps shuffling around the narrowing fields (which I don't think is too satisfying)

[mjbvz]

Thanks. The function itself hasn't changed but we did make one of the type ReadonlyArray instead of a normal Array. I don't fully understand the typing discussion going on. I guess a minimal example of the problem would be:

declare const x: string | readonly string[]

if (!Array.isArray(x)) {
	x.toLocaleLowerCase() // Property 'toLocaleLowerCase' does not exist on type 'string | readonly string[]'.
}

I'll look into adding casts or something to workaround this

[orta]

Roughly what's happening: a readonly string[] is a subset of string[] (because the mutable bits are removed in the type system by being marked readonly) which means that it doesn't pass the narrowing how you'd think/expect it to, but it passes a different part of the narrowing check which says 'keep this type' because of the any[] result in Array.isArray meaning the narrowing doesn't happen

I agree its a bug though, you'd expect that to work which was why we originally tried to propagate the readonly through