Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cross-file Typescript support in vscode-web #169311

Merged
merged 63 commits into from
Jan 12, 2023
Merged
Show file tree
Hide file tree
Changes from 39 commits
Commits
Show all changes
63 commits
Select commit Hold shift + click to select a range
bd9e555
recreate logging from other machine
sandersn Sep 16, 2022
052c6a8
Merge branch 'main' into wasm-typescript
sandersn Sep 16, 2022
ea42f84
comment out openSystemBrowser
sandersn Sep 16, 2022
0102053
Add vscode-wasm-typescript dep
sandersn Sep 16, 2022
f79188b
remove unused reference to module
sandersn Sep 16, 2022
bdee414
use require reference that linter allows
sandersn Sep 16, 2022
4f0ece1
Add vscode-wasm-typescript to tsserver.web.js
sandersn Sep 26, 2022
6f6c491
Update vscode-wasm-typescript dependency
sandersn Sep 27, 2022
21eac11
Fix minor syntax in webpack hack
sandersn Sep 27, 2022
2afbf36
Fix another typo in webpack hack!
sandersn Sep 27, 2022
80d2729
Fix provided typescript path
sandersn Sep 27, 2022
9d88cc9
Try to improve module.exports handling in webpack hac
sandersn Sep 27, 2022
7db6575
tsserver.web.js comes from local builds
sandersn Sep 29, 2022
f67a08e
First attempt to set up server-side support
sandersn Sep 30, 2022
092c565
Remove auto-imported identifier
sandersn Sep 30, 2022
bd93d21
Move sync-api setup code to serverProcess.browser.ts
sandersn Oct 4, 2022
293b6e3
Reorder webpack hack and clean up unused logging
sandersn Oct 6, 2022
124acc8
Update vscode-wasm/vscode-wasm-typescript dependencies
sandersn Oct 11, 2022
40a2626
Add file watching
sandersn Oct 13, 2022
060d671
Extract webpack hack
sandersn Oct 17, 2022
fac861b
Remove manual verbose logging
sandersn Oct 21, 2022
65b8c46
Merge branch 'main' into wasm-typescript
sandersn Oct 21, 2022
d8f495b
Add vscode-test-web to semantic-supported schemes
sandersn Oct 24, 2022
7a1eb58
Also update the webpack-hack-only build
sandersn Oct 26, 2022
4ee41e2
Switch to tsserverlibrary
sandersn Nov 4, 2022
72a972c
Merge branch 'main' into wasm-typescript
sandersn Nov 4, 2022
6509c48
Remove bogus auto-import and unneeded (?) dep
sandersn Nov 4, 2022
2c32ab4
Merge branch 'main' into wasm-typescript
sandersn Dec 1, 2022
6c56e69
remove webpack-like hack
sandersn Dec 1, 2022
673869c
move code from vscode-wasm-typescript
sandersn Dec 2, 2022
741e3a8
Initial prototype of cancellation
sandersn Dec 2, 2022
a24ae56
Switch tsserver to separate MessageChannel
sandersn Dec 7, 2022
6f1415e
Move watches to a separate MessagePort
sandersn Dec 7, 2022
4b19dee
switch vscode-web from in-memory to real filesystem
sandersn Dec 8, 2022
c17bf33
Make toResource translate / -> vscode-test-web
sandersn Dec 8, 2022
6fade7c
Encode scheme and authority in TS filenames
sandersn Dec 15, 2022
68047c7
Lift parseUri outside createServerHost
sandersn Dec 15, 2022
9ffdb8e
Special-case URI of lib*d.ts in webServer.toResource
sandersn Dec 15, 2022
629804c
Merge branch 'main' into wasm-typescript
sandersn Dec 15, 2022
0f0e155
Improve cancellation
sandersn Dec 16, 2022
675401e
Pass in current request instead of waiting for a fresh one.
sandersn Dec 16, 2022
41be877
Address initial PR comments
sandersn Dec 19, 2022
8639b4b
Add cancellation bit to each (cancellable) request, locally fix an is…
jrieken Dec 22, 2022
0138958
Switch to per-file/directory watches
sandersn Dec 22, 2022
a8774f3
Parse --serverMode partialSemantic in webServer
sandersn Dec 27, 2022
52506b6
Simplify logging code
sandersn Dec 27, 2022
6f1966f
Merge branch 'main' into wasm-typescript
sandersn Dec 27, 2022
a07f98c
Cleanup in webServer
sandersn Dec 29, 2022
0288f0f
Switch to markdown extension's FileWatcherManager
sandersn Jan 6, 2023
1c52093
Clean up host methods
sandersn Jan 9, 2023
f48e065
More logging/TODO cleanup
sandersn Jan 9, 2023
f5dade7
Remove duplicate dependency
mjbvz Jan 10, 2023
cde1ffc
Add setting to enable/disable semantic mode on web
mjbvz Jan 10, 2023
8f130c0
Re-order and re-arrange code to minimise PR diff
sandersn Jan 10, 2023
4952d5d
Copy fileWatchingManager to typescript extension
sandersn Jan 10, 2023
8a72e7d
Fix linting of webServer
mjbvz Jan 10, 2023
98ae48e
Align formatting of catch / else
mjbvz Jan 10, 2023
9069220
Extract isProjectWideIntellisenseOnWebEnabled and keep using in-memor…
mjbvz Jan 11, 2023
6057abc
Make sure we still work if SharedArrayBuffers aren't supported
mjbvz Jan 12, 2023
e36a6db
Remove symlink support and fix typo
sandersn Jan 12, 2023
0b2a13c
Merge branch 'main' into wasm-typescript
sandersn Jan 12, 2023
aad4f87
Merge branch 'main' into wasm-typescript
sandersn Jan 12, 2023
6a1b2c3
Fix compile errors
mjbvz Jan 12, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions extensions/typescript-language-features/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@
"jsonc-parser": "^3.2.0",
"semver": "5.5.1",
"vscode-tas-client": "^0.1.63",
"vscode-uri": "^3.0.3",
"@vscode/sync-api-client": "^0.7.2",
"@vscode/sync-api-common": "^0.7.2",
"@vscode/sync-api-service": "^0.7.3",
"typescript": "latest",
sandersn marked this conversation as resolved.
Show resolved Hide resolved
"vscode-uri": "^3.0.3"
},
"devDependencies": {
Expand Down
11 changes: 9 additions & 2 deletions extensions/typescript-language-features/src/tsServer/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*--------------------------------------------------------------------------------------------*/

import * as vscode from 'vscode';
import { Cancellation } from '@vscode/sync-api-common/lib/common/messageCancellation';
import type * as Proto from '../protocol';
import { EventName } from '../protocol.const';
import { CallbackMap } from '../tsServer/callbackMap';
Expand Down Expand Up @@ -64,6 +65,7 @@ export interface TsServerProcessFactory {
kind: TsServerProcessKind,
configuration: TypeScriptServiceConfiguration,
versionManager: TypeScriptVersionManager,
extensionUri: vscode.Uri,
): TsServerProcess;
}

Expand Down Expand Up @@ -167,6 +169,7 @@ export class ProcessBasedTsServer extends Disposable implements ITypeScriptServe
throw new Error(`Unknown message type ${message.type} received`);
}
} finally {
// TODO: Also need to make requests cancellable that came in while handling responses, but I don't have a token
this.sendNextRequests();
}
}
Expand Down Expand Up @@ -220,6 +223,7 @@ export class ProcessBasedTsServer extends Disposable implements ITypeScriptServe
result = new Promise<ServerResponse.Response<Proto.Response>>((resolve, reject) => {
this._callbacks.add(request.seq, { onSuccess: resolve as () => ServerResponse.Response<Proto.Response> | undefined, onError: reject, queuingStartTime: Date.now(), isAsync: executeInfo.isAsync }, executeInfo.isAsync);

// TODO: Need to decide whether to old-cancel or new-cancel (although for prototype purposes, doing both might not crash)
if (executeInfo.token) {
executeInfo.token.onCancellationRequested(() => {
this.tryCancelRequest(request.seq, command);
Expand All @@ -246,16 +250,19 @@ export class ProcessBasedTsServer extends Disposable implements ITypeScriptServe
}

this._requestQueue.enqueue(requestInfo);
this.sendNextRequests();
this.sendNextRequests(executeInfo.expectsResult && executeInfo.token);

return [result];
}

private sendNextRequests(): void {
private sendNextRequests(cancelToken?: vscode.CancellationToken | false): void {
while (this._pendingResponses.size === 0 && this._requestQueue.length > 0) {
const item = this._requestQueue.dequeue();
if (item) {
this.sendRequest(item);
if (cancelToken) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you put this logic in this.tryCancelRequets instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It fits quite nicely there, but I'm not sure it will work: Cancellation.addData needs to add information to a request that will be posted to tsserver's webworker. If there is no pending request the code will have to wait until there is one.

For now I put in peek method on RequestQueue and a busy wait in tryCancelRequests, but it seems wrong. Could be better than what I had before, though.

@jrieken can you provide advice here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The latest commit provides the current request to tryCancelRequest so that . However, it might actually be worse, since I believe that request will have been sent to tsserver already, and that Cancellation.addData won't actually transmit the request for cancellation. (again, @jrieken will know how this should work, I think)

I haven't worked out how to test this, so advice on that would be helpful as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cancellation.addData must be added before sending the request and the function that it returns need to be invoked in case of cancellation. I have some local changes on top of this PR that I am attempting to push. There was also an issue on the retrieving end. For that I for now inlines the implementation with a fix

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

8639b4b it is. It would be nicer if this could be done with serverProcess.browser.ts but I couldn't find a neat/minimal way to doing that

cancelToken.onCancellationRequested(Cancellation.addData(item.request));
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,16 @@
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

/// <reference lib='dom' />
sandersn marked this conversation as resolved.
Show resolved Hide resolved
import * as vscode from 'vscode';
import type * as Proto from '../protocol';
import { TypeScriptServiceConfiguration } from '../utils/configuration';
import { memoize } from '../utils/memoize';
import { TsServerProcess, TsServerProcessKind } from './server';
import { TypeScriptVersion } from './versionProvider';



declare const Worker: any;
declare type Worker = any;
import { ServiceConnection } from '@vscode/sync-api-common/browser';
import { Requests, ApiService } from '@vscode/sync-api-service';
import { TypeScriptVersionManager } from './versionManager';

export class WorkerServerProcess implements TsServerProcess {

Expand All @@ -22,10 +20,12 @@ export class WorkerServerProcess implements TsServerProcess {
args: readonly string[],
_kind: TsServerProcessKind,
_configuration: TypeScriptServiceConfiguration,
_versionManager: TypeScriptVersionManager,
extensionUri: vscode.Uri,
) {
const tsServerPath = version.tsServerPath;
const worker = new Worker(tsServerPath);
return new WorkerServerProcess(worker, [
return new WorkerServerProcess(worker, extensionUri, [
...args,

// Explicitly give TS Server its path so it can
Expand All @@ -37,27 +37,72 @@ export class WorkerServerProcess implements TsServerProcess {
private readonly _onDataHandlers = new Set<(data: Proto.Response) => void>();
private readonly _onErrorHandlers = new Set<(err: Error) => void>();
private readonly _onExitHandlers = new Set<(code: number | null, signal: string | null) => void>();
/** For communicating with TS server synchronously */
private readonly tsserver: MessagePort;
/** For communicating watches asynchronously */
private readonly watcher: MessagePort;
/** For communicating with filesystem synchronously */
private readonly syncFs: MessagePort;

public constructor(
private readonly worker: Worker,
/** For logging and initial setup */
private readonly mainChannel: Worker,
extensionUri: vscode.Uri,
args: readonly string[],
) {
worker.addEventListener('message', (msg: any) => {
if (msg.data.type === 'log') {
this.output.append(msg.data.body);
const tsserverChannel = new MessageChannel();
const watcherChannel = new MessageChannel();
const syncChannel = new MessageChannel();
this.tsserver = tsserverChannel.port2;
this.watcher = watcherChannel.port2;
this.syncFs = syncChannel.port1;
sandersn marked this conversation as resolved.
Show resolved Hide resolved
this.tsserver.onmessage = (event) => {
if (event.data.type === 'log') {
console.error(`unexpected log message on tsserver channel: ${JSON.stringify(event)}`);
return;
}

for (const handler of this._onDataHandlers) {
handler(msg.data);
handler(event.data);
}
});
worker.onerror = (err: Error) => {
};
this.watcher.onmessage = (event) => {
console.error(`unexpected message on watcher channel: ${JSON.stringify(event)}`);
};
mainChannel.onmessage = (msg: any) => {
// for logging only
if (msg.data.type === 'log') {
this.output.append(msg.data.body);
return;
}
console.error(`unexpected message on main channel: ${JSON.stringify(msg)}`);
// TODO: A multi-watcher message would look something like this:
// if (msg.data.type === 'dispose-watcher') {
// watchers.get(msg.data.path).dispose()
// }
};
mainChannel.onerror = (err: ErrorEvent) => {
this.output.append(JSON.stringify('error! ' + JSON.stringify(err)) + '\n');
for (const handler of this._onErrorHandlers) {
handler(err);
// TODO: The ErrorEvent type might be wrong; previously this was typed as Error and didn't have the property access.
handler(err.error);
}
};
worker.postMessage(args);
// TODO: For prototyping, create one watcher ahead of time and send messages using the worker.
// For the real thing, it makes more sense to create a third MessageChannel and listen on it; the host
// can then send messages to tell the extension to create a new filesystemwatcher for each file/directory
const fsWatcher = vscode.workspace.createFileSystemWatcher('**/*');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may be able to reuse the markdown watcher manager for this:

fsWatcher.onDidChange(e => this.watcher.postMessage({ type: 'watch', event: 'change', path: e.path }));
fsWatcher.onDidCreate(e => this.watcher.postMessage({ type: 'watch', event: 'create', path: e.path }));
fsWatcher.onDidDelete(e => this.watcher.postMessage({ type: 'watch', event: 'delete', path: e.path }));
this.output.append('creating new MessageChannel and posting its port2 + args: ' + args.join(' '));
mainChannel.postMessage(
{ args, extensionUri: { scheme: extensionUri.scheme, authority: extensionUri.authority, path: extensionUri.path } },
[syncChannel.port2, tsserverChannel.port1, watcherChannel.port1]
);
const connection = new ServiceConnection<Requests>(syncChannel.port1);
new ApiService('vscode-wasm-typescript', connection);
connection.signalReady();
this.output.append('done constructing WorkerServerProcess\n');
}

@memoize
Expand All @@ -66,7 +111,7 @@ export class WorkerServerProcess implements TsServerProcess {
}

write(serverRequest: Proto.Request): void {
this.worker.postMessage(serverRequest);
this.tsserver.postMessage(serverRequest);
}

onData(handler: (response: Proto.Response) => void): void {
Expand All @@ -83,6 +128,10 @@ export class WorkerServerProcess implements TsServerProcess {
}

kill(): void {
this.worker.terminate();
this.mainChannel.terminate();
this.tsserver.close();
this.watcher.close();
this.syncFs.close();
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export class TypeScriptServerSpawner {
private readonly _telemetryReporter: TelemetryReporter,
private readonly _tracer: Tracer,
private readonly _factory: TsServerProcessFactory,
private readonly _extensionUri: vscode.Uri,
) { }

public spawn(
Expand Down Expand Up @@ -152,7 +153,7 @@ export class TypeScriptServerSpawner {
}

this._logger.info(`<${kind}> Forking...`);
const process = this._factory.fork(version, args, kind, configuration, this._versionManager);
const process = this._factory.fork(version, args, kind, configuration, this._versionManager, this._extensionUri);
this._logger.info(`<${kind}> Starting...`);

return new ProcessBasedTsServer(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ export default class TypeScriptServiceClient extends Disposable implements IType
return this.apiVersion.fullVersionString;
});

this.typescriptServerSpawner = new TypeScriptServerSpawner(this.versionProvider, this._versionManager, this.logDirectoryProvider, this.pluginPathsProvider, this.logger, this.telemetryReporter, this.tracer, this.processFactory);
this.typescriptServerSpawner = new TypeScriptServerSpawner(this.versionProvider, this._versionManager, this.logDirectoryProvider, this.pluginPathsProvider, this.logger, this.telemetryReporter, this.tracer, this.processFactory, context.extensionUri);

this._register(this.pluginManager.onDidUpdateConfig(update => {
this.configurePlugin(update.pluginId, update.config);
Expand All @@ -235,7 +235,8 @@ export default class TypeScriptServiceClient extends Disposable implements IType
if (isWeb()) {
return new ClientCapabilities(
ClientCapability.Syntax,
ClientCapability.EnhancedSyntax);
ClientCapability.EnhancedSyntax,
ClientCapability.Semantic);
}

if (this.apiVersion.gte(API.v400)) {
Expand Down Expand Up @@ -678,7 +679,7 @@ export default class TypeScriptServiceClient extends Disposable implements IType
}
default:
{
return this.inMemoryResourcePrefix
return (isWeb() ? '' : this.inMemoryResourcePrefix)
+ '/' + resource.scheme
+ '/' + (resource.authority || this.emptyAuthority)
+ (resource.path.startsWith('/') ? resource.path : '/' + resource.path)
Expand Down Expand Up @@ -722,9 +723,16 @@ export default class TypeScriptServiceClient extends Disposable implements IType
public toResource(filepath: string): vscode.Uri {
if (isWeb()) {
// On web, the stdlib paths that TS return look like: '/lib.es2015.collection.d.ts'
// TODO: Find out what extensionUri is when testing (should be http://localhost:8080/static/sources/extensions/typescript-language-features/)
// TODO: make sure that this code path is getting hit
if (filepath.startsWith('/lib.') && filepath.endsWith('.d.ts')) {
return vscode.Uri.joinPath(this.context.extensionUri, 'dist', 'browser', 'typescript', filepath.slice(1));
}
const parts = filepath.match(/^\/([^\/]+)\/([^\/]*)\/(.+)$/);
if (parts) {
const resource = vscode.Uri.parse(parts[1] + '://' + (parts[2] === this.emptyAuthority ? '' : parts[2]) + '/' + parts[3]);
return this.bufferSyncSupport.toVsCodeResource(resource);
}
}

if (filepath.startsWith(this.inMemoryResourcePrefix)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,14 @@ export const vscodeNotebookCell = 'vscode-notebook-cell';
export const memFs = 'memfs';
export const vscodeVfs = 'vscode-vfs';
export const officeScript = 'office-script';
export const vscodeWeb = 'vscode-test-web'; // TODO: Should probably reuse vscodeVfs eventually, since vscode-test-web is the local test web server
sandersn marked this conversation as resolved.
Show resolved Hide resolved

export const semanticSupportedSchemes = [
file,
untitled,
walkThroughSnippet,
vscodeNotebookCell,
vscodeWeb,
];

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,5 @@
import * as vscode from 'vscode';

export function isWeb(): boolean {
// @ts-expect-error
return typeof navigator !== 'undefined' && vscode.env.uiKind === vscode.UIKind.Web;
}
Loading