microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
163.8k stars 29.12k forks source link

Deal with build failures related to FileSystemHandle changes with TS@next #141908

Closed mjbvz closed 2 years ago

mjbvz commented 2 years ago

Repro Build VS Code using the latest TS nightly:

This produced the errors:

[watch-client    ] [16:28:53] Error: /Users/matb/projects/vscode/src/vs/workbench/services/search/browser/searchService.ts(94,11): Type 'FileSystemHandle | undefined' is not assignable to type 'SearchWorkerFileSystemHandle | undefined'.
[watch-client    ]   Type 'FileSystemHandle' is not assignable to type 'SearchWorkerFileSystemHandle | undefined'.
[watch-client    ]     Property 'getFile' is missing in type 'FileSystemHandle' but required in type 'ISearchWorkerFileSystemFileHandle'.
[watch-client    ] [16:28:53] Error: /Users/matb/projects/vscode/src/vs/workbench/services/search/browser/searchService.ts(147,11): Type 'FileSystemHandle | undefined' is not assignable to type 'SearchWorkerFileSystemHandle | undefined'.
[watch-client    ] [16:28:53] Error: /Users/matb/projects/vscode/src/vs/workbench/services/dialogs/browser/fileDialogService.ts(65,33): Property 'showOpenFilePicker' does not exist on type 'Window & typeof globalThis'.
[watch-client    ] [16:28:53] Error: /Users/matb/projects/vscode/src/vs/workbench/services/dialogs/browser/fileDialogService.ts(70,58): Argument of type 'FileSystemHandle | undefined' is not assignable to parameter of type 'FileSystemFileHandle'.
[watch-client    ]   Type 'undefined' is not assignable to type 'FileSystemFileHandle'.
[watch-client    ] [16:28:53] Error: /Users/matb/projects/vscode/src/vs/workbench/services/dialogs/browser/fileDialogService.ts(120,30): Property 'showSaveFilePicker' does not exist on type 'Window & typeof globalThis'.
[watch-client    ] [16:28:53] Error: /Users/matb/projects/vscode/src/vs/workbench/services/dialogs/browser/fileDialogService.ts(125,53): Argument of type 'FileSystemHandle | undefined' is not assignable to parameter of type 'FileSystemFileHandle'.
[watch-client    ]   Type 'undefined' is not assignable to type 'FileSystemFileHandle'.
[watch-client    ] [16:28:53] Error: /Users/matb/projects/vscode/src/vs/workbench/services/dialogs/browser/fileDialogService.ts(128,54): Cannot find name 'FilePickerAcceptType'.
[watch-client    ] [16:28:53] Error: /Users/matb/projects/vscode/src/vs/workbench/services/dialogs/browser/fileDialogService.ts(157,30): Property 'showSaveFilePicker' does not exist on type 'Window & typeof globalThis'.
[watch-client    ] [16:28:53] Error: /Users/matb/projects/vscode/src/vs/workbench/services/dialogs/browser/fileDialogService.ts(162,53): Argument of type 'FileSystemHandle | undefined' is not assignable to parameter of type 'FileSystemFileHandle'.
[watch-client    ]   Type 'undefined' is not assignable to type 'FileSystemFileHandle'.
[watch-client    ] [16:28:53] Error: /Users/matb/projects/vscode/src/vs/workbench/services/dialogs/browser/fileDialogService.ts(181,33): Property 'showOpenFilePicker' does not exist on type 'Window & typeof globalThis'.
[watch-client    ] [16:28:53] Error: /Users/matb/projects/vscode/src/vs/workbench/services/dialogs/browser/fileDialogService.ts(186,33): Property 'showDirectoryPicker' does not exist on type 'Window & typeof globalThis'.
[watch-client    ] [16:28:53] Error: /Users/matb/projects/vscode/src/vs/workbench/contrib/files/browser/fileImportExport.ts(635,66): Property 'showDirectoryPicker' does not exist on type 'Window & typeof globalThis'.
[watch-client    ] [16:28:53] Error: /Users/matb/projects/vscode/src/vs/workbench/contrib/files/browser/fileImportExport.ts(676,67): Cannot find name 'FileSystemWritableFileStream'.
[watch-client    ] [16:28:53] Error: /Users/matb/projects/vscode/src/vs/workbench/contrib/files/browser/fileImportExport.ts(711,69): Cannot find name 'FileSystemWritableFileStream'.
[watch-client    ] [16:28:53] Error: /Users/matb/projects/vscode/src/vs/workbench/contrib/files/browser/fileImportExport.ts(730,45): Property 'createWritable' does not exist on type 'FileSystemFileHandle'.
[watch-client    ] [16:28:53] Error: /Users/matb/projects/vscode/src/vs/platform/files/browser/htmlFileSystemProvider.ts(59,31): Property 'getFile' does not exist on type 'FileSystemHandle'.
[watch-client    ] [16:28:53] Error: /Users/matb/projects/vscode/src/vs/platform/files/browser/htmlFileSystemProvider.ts(89,38): Type 'FileSystemDirectoryHandle' must have a '[Symbol.asyncIterator]()' method that returns an async iterator.
[watch-client    ] [16:28:53] Error: /Users/matb/projects/vscode/src/vs/platform/files/browser/htmlFileSystemProvider.ts(212,34): Property 'createWritable' does not exist on type 'FileSystemFileHandle'.

Cause I believe that TS recently added their own typings for some of the file system types. This seems to be causing issues with our typings

I took a look at the errors but wasn't sure what the proper fix for them was

Also tracked upstream by https://github.com/microsoft/TypeScript/issues/47568

/cc @sandersn

JacksonKearl commented 2 years ago

[watch-client ] [16:28:53] Error: /Users/matb/projects/vscode/src/vs/platform/files/browser/htmlFileSystemProvider.ts(89,38): Type 'FileSystemDirectoryHandle' must have a '[Symbol.asyncIterator]()' method that returns an async iterator.

Based on the spec here, it should indeed expose an async iterator. I think this is an issue with the typings.

[watch-client ] [16:28:53] Error: /Users/matb/projects/vscode/src/vs/platform/files/browser/htmlFileSystemProvider.ts(59,31): Property 'getFile' does not exist on type 'FileSystemHandle'.

It'd be nice if checking that #.kind === 'file' would narrow the type to a FileSystemFileHandle https://github.com/microsoft/vscode/blob/16d0a319b28caa4b6cf4e6801fd508282b7533e0/src/vs/platform/files/browser/htmlFileSystemProvider.ts#L58-L59

[watch-client ] [16:28:53] Error: /Users/matb/projects/vscode/src/vs/workbench/contrib/files/browser/fileImportExport.ts(730,45): Property 'createWritable' does not exist on type 'FileSystemFileHandle'.

Again, spec sats that createWritable should exist on a FileSystemFileHandle.

JacksonKearl commented 2 years ago

A couple were easy, put up on a branch (cc @bpasero). Rest seem to be a conflict between the spec/reality and the typings.

bpasero commented 2 years ago

I would wait to have TS fix their typings issue before jumping onto anything.

sandersn commented 2 years ago

I checked out the 3 failures. To make sure I got them:

  1. FileSystemFileHandle.createWritable should exist.
  2. FileSystemDirectoryHandle.[Symbol.asyncIterator] should exist.
  3. checking FileSystemHandle.kind should narrow to the appropriate subtype.

Typescript ships DOM types that are in 2 or more browser engines [1]. From poking at FileSystemHandle in Safari, and looking at MDN's web-compat-data on caniuse.com, it looks like Safari has actually implemented only half of the spec. I think VS Code should keep using @types/wicg-file-system-access. The latest version should extend the 4.6 DOM types with what's supported by Chrome and should build without errors.

That should fix (1) and (2). As for (3), unfortunately, the DOM types use the name FileSystemHandle for the actual base type, so I create an alias type FileSystemHandleUnion = FileSystemFileHandle | FileSystemDirectoryHandle in @types/wicg-file-system-access to retain the narrowing behaviour.

[1] This is more automated than in the past, so the rest of the API should arrive in Typescript's DOM types when Safari starts supporting it (and MDN records the support).

JacksonKearl commented 2 years ago

Cool, thanks for looking into this!

mjbvz commented 2 years ago

I tested with the latest TS nightly and also bumping @types/wicg-file-system-access but still see the following six failures:

[watch-client    ] [08:23:28] Error: /Users/matb/projects/vscode/src/vs/workbench/services/search/browser/searchService.ts(95,11): Type 'FileSystemHandle | undefined' is not assignable to type 'SearchWorkerFileSystemHandle | undefined'.
[watch-client    ]   Type 'FileSystemHandle' is not assignable to type 'SearchWorkerFileSystemHandle | undefined'.
[watch-client    ]     Property 'getFile' is missing in type 'FileSystemHandle' but required in type 'ISearchWorkerFileSystemFileHandle'.
[watch-client    ] [08:23:28] Error: /Users/matb/projects/vscode/src/vs/workbench/services/search/browser/searchService.ts(149,11): Type 'FileSystemHandle | undefined' is not assignable to type 'SearchWorkerFileSystemHandle | undefined'.
[watch-client    ] [08:23:28] Error: /Users/matb/projects/vscode/src/vs/workbench/services/dialogs/browser/fileDialogService.ts(70,58): Argument of type 'FileSystemHandle' is not assignable to parameter of type 'FileSystemFileHandle'.
[watch-client    ]   Type 'FileSystemHandle' is missing the following properties from type 'FileSystemFileHandle': getFile, createWritable
[watch-client    ] [08:23:28] Error: /Users/matb/projects/vscode/src/vs/workbench/services/dialogs/browser/fileDialogService.ts(125,53): Argument of type 'FileSystemHandle' is not assignable to parameter of type 'FileSystemFileHandle'.
[watch-client    ] [08:23:28] Error: /Users/matb/projects/vscode/src/vs/workbench/services/dialogs/browser/fileDialogService.ts(162,53): Argument of type 'FileSystemHandle' is not assignable to parameter of type 'FileSystemFileHandle'.
[watch-client    ] [08:23:28] Error: /Users/matb/projects/vscode/src/vs/platform/files/browser/htmlFileSystemProvider.ts(59,31): Property 'getFile' does not exist on type 'FileSystemHandle'.
FossPrime commented 2 years ago

[1] This is more automated than in the past, so the rest of the API should arrive in Typescript's DOM types when Safari starts supporting it (and MDN records the support).

Safari released support 15 days before this statement... caniuse.com/mdn-api_filesystemhandle

I'm not seeing FileSystemHandle API support in vscode PWA at all... should I file a ticket here similar to: https://github.com/Felx-B/vscode-web/issues/7

bpasero commented 2 years ago

I am not sure what Safari supports is the same as what we need: https://webkit.org/blog/12257/the-file-system-access-api-with-origin-private-file-system/

FossPrime commented 2 years ago

I am not sure what Safari supports is the same as what we need: webkit.org/blog/12257/the-file-system-access-api-with-origin-private-file-system

I think you're right. I don't have access to any Safari-ish environments right now, but the following code is what you'd need to test for it. I've filed the issue with caniuse.

if ('launchQueue' in window && 'files' in LaunchParams.prototype) {
  // The File Handling API is supported.
}
bpasero commented 2 years ago

Here is our check today:

https://github.com/microsoft/vscode/blob/36ed074f8f6a4da708775a27c036117e09fcced0/src/vs/platform/files/browser/webFileSystemAccess.ts#L13-L19

We expect a browser to allow the user to pick a folder or file to open.