From 3ba8565b798949f61232789ed07f6db9130763d2 Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Mon, 18 Oct 2021 15:47:21 -0700 Subject: [PATCH 1/6] Use node ipc for TS Server For #46417 This lets us use Node's built-in ipc for passing messages to/from the typescript server instead of using stdio --- src/tsserver/nodeServer.ts | 40 +++++++++++++++++++++++++++++++------- 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/src/tsserver/nodeServer.ts b/src/tsserver/nodeServer.ts index e8e42042d5523..e20c6936ac73b 100644 --- a/src/tsserver/nodeServer.ts +++ b/src/tsserver/nodeServer.ts @@ -715,6 +715,24 @@ namespace ts.server { this.constructed = true; } + public send(msg: protocol.Message) { + if (useNodeIpc) { + if (msg.type === "event" && !this.canUseEvents) { + if (this.logger.hasLevel(LogLevel.verbose)) { + this.logger.info(`Session does not support events: ignored event: ${JSON.stringify(msg)}`); + } + return; + } + // TODO: Do we need any perf stuff here? + // const msgText = formatMessage(msg, this.logger, this.byteLength, this.host.newLine); + // perfLogger.logEvent(`Response message size: ${msgText.length}`); + process.send!(msg); + } + else { + super.send(msg); + } + } + event(body: T, eventName: string): void { Debug.assert(!!this.constructed, "Should only call `IOSession.prototype.event` on an initialized IOSession"); @@ -748,14 +766,21 @@ namespace ts.server { } listen() { - rl.on("line", (input: string) => { - const message = input.trim(); - this.onMessage(message); - }); + if (useNodeIpc) { + process.on('message', (e: any) => { + this.onMessage(JSON.stringify(e)); // TODO: should not convert to back to a string just to parse again :) + }); + } + else { + rl.on("line", (input: string) => { + const message = input.trim(); + this.onMessage(message); + }); - rl.on("close", () => { - this.exit(); - }); + rl.on("close", () => { + this.exit(); + }); + } } } @@ -765,6 +790,7 @@ namespace ts.server { const npmLocation = findArgument(Arguments.NpmLocation); const validateDefaultNpmLocation = hasArgument(Arguments.ValidateDefaultNpmLocation); const disableAutomaticTypingAcquisition = hasArgument("--disableAutomaticTypingAcquisition"); + const useNodeIpc = hasArgument('--useNodeIpc'); const telemetryEnabled = hasArgument(Arguments.EnableTelemetry); const commandLineTraceDir = findArgument("--traceDirectory"); const traceDir = commandLineTraceDir From 90d7ea8eb60e299d60356c5c5af1e28a11e1a26d Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Mon, 18 Oct 2021 15:55:05 -0700 Subject: [PATCH 2/6] Remove extra parse --- src/tsserver/nodeServer.ts | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/tsserver/nodeServer.ts b/src/tsserver/nodeServer.ts index e20c6936ac73b..c9c083d59651d 100644 --- a/src/tsserver/nodeServer.ts +++ b/src/tsserver/nodeServer.ts @@ -733,6 +733,20 @@ namespace ts.server { } } + protected parseMessage(message: any): protocol.Request { + if (useNodeIpc) { + return message as protocol.Request; + } + return super.parseMessage(message); + } + + protected toStringMessage(message: any) { + if (useNodeIpc) { + return JSON.stringify(message, undefined, 2); + } + return super.toStringMessage(message); + } + event(body: T, eventName: string): void { Debug.assert(!!this.constructed, "Should only call `IOSession.prototype.event` on an initialized IOSession"); @@ -768,7 +782,7 @@ namespace ts.server { listen() { if (useNodeIpc) { process.on('message', (e: any) => { - this.onMessage(JSON.stringify(e)); // TODO: should not convert to back to a string just to parse again :) + this.onMessage(e); }); } else { From 493d170686ea7bb51d794fd305909764fd87e016 Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Tue, 7 Dec 2021 17:08:17 -0800 Subject: [PATCH 3/6] Add extra logging when using node IPC --- src/tsserver/nodeServer.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/tsserver/nodeServer.ts b/src/tsserver/nodeServer.ts index c9c083d59651d..fcb5fc8b9643a 100644 --- a/src/tsserver/nodeServer.ts +++ b/src/tsserver/nodeServer.ts @@ -233,7 +233,7 @@ namespace ts.server { // Override sys.write because fs.writeSync is not reliable on Node 4 sys.write = (s: string) => writeMessage(sys.bufferFrom!(s, "utf8") as globalThis.Buffer); - // REVIEW: for now this implementation uses polling. + // REVIEW: for now this implementation uses polling. // The advantage of polling is that it works reliably // on all os and with network mounted files. // For 90 referenced files, the average time to detect @@ -723,9 +723,13 @@ namespace ts.server { } return; } - // TODO: Do we need any perf stuff here? - // const msgText = formatMessage(msg, this.logger, this.byteLength, this.host.newLine); - // perfLogger.logEvent(`Response message size: ${msgText.length}`); + + const verboseLogging = logger.hasLevel(LogLevel.verbose); + if (verboseLogging) { + const json = JSON.stringify(msg); + logger.info(`${msg.type}:${indent(json)}`); + } + process.send!(msg); } else { @@ -781,7 +785,7 @@ namespace ts.server { listen() { if (useNodeIpc) { - process.on('message', (e: any) => { + process.on("message", (e: any) => { this.onMessage(e); }); } @@ -804,7 +808,7 @@ namespace ts.server { const npmLocation = findArgument(Arguments.NpmLocation); const validateDefaultNpmLocation = hasArgument(Arguments.ValidateDefaultNpmLocation); const disableAutomaticTypingAcquisition = hasArgument("--disableAutomaticTypingAcquisition"); - const useNodeIpc = hasArgument('--useNodeIpc'); + const useNodeIpc = hasArgument("--useNodeIpc"); const telemetryEnabled = hasArgument(Arguments.EnableTelemetry); const commandLineTraceDir = findArgument("--traceDirectory"); const traceDir = commandLineTraceDir From d804a4d477288a47982f844fb43ca43a197ed56c Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Fri, 17 Dec 2021 20:10:35 -0800 Subject: [PATCH 4/6] Split out to subclass --- src/tsserver/nodeServer.ts | 89 +++++++++++++++++--------------------- 1 file changed, 40 insertions(+), 49 deletions(-) diff --git a/src/tsserver/nodeServer.ts b/src/tsserver/nodeServer.ts index fcb5fc8b9643a..0861086fc734f 100644 --- a/src/tsserver/nodeServer.ts +++ b/src/tsserver/nodeServer.ts @@ -715,42 +715,6 @@ namespace ts.server { this.constructed = true; } - public send(msg: protocol.Message) { - if (useNodeIpc) { - if (msg.type === "event" && !this.canUseEvents) { - if (this.logger.hasLevel(LogLevel.verbose)) { - this.logger.info(`Session does not support events: ignored event: ${JSON.stringify(msg)}`); - } - return; - } - - const verboseLogging = logger.hasLevel(LogLevel.verbose); - if (verboseLogging) { - const json = JSON.stringify(msg); - logger.info(`${msg.type}:${indent(json)}`); - } - - process.send!(msg); - } - else { - super.send(msg); - } - } - - protected parseMessage(message: any): protocol.Request { - if (useNodeIpc) { - return message as protocol.Request; - } - return super.parseMessage(message); - } - - protected toStringMessage(message: any) { - if (useNodeIpc) { - return JSON.stringify(message, undefined, 2); - } - return super.toStringMessage(message); - } - event(body: T, eventName: string): void { Debug.assert(!!this.constructed, "Should only call `IOSession.prototype.event` on an initialized IOSession"); @@ -784,21 +748,48 @@ namespace ts.server { } listen() { - if (useNodeIpc) { - process.on("message", (e: any) => { - this.onMessage(e); - }); + rl.on("line", (input: string) => { + const message = input.trim(); + this.onMessage(message); + }); + + rl.on("close", () => { + this.exit(); + }); + } + } + + class IpcIOSession extends IOSession { + + public send(msg: protocol.Message) { + if (msg.type === "event" && !this.canUseEvents) { + if (this.logger.hasLevel(LogLevel.verbose)) { + this.logger.info(`Session does not support events: ignored event: ${JSON.stringify(msg)}`); + } + return; } - else { - rl.on("line", (input: string) => { - const message = input.trim(); - this.onMessage(message); - }); - rl.on("close", () => { - this.exit(); - }); + const verboseLogging = logger.hasLevel(LogLevel.verbose); + if (verboseLogging) { + const json = JSON.stringify(msg); + logger.info(`${msg.type}:${indent(json)}`); } + + process.send!(msg); + } + + protected parseMessage(message: any): protocol.Request { + return message as protocol.Request; + } + + protected toStringMessage(message: any) { + return JSON.stringify(message, undefined, 2); + } + + listen() { + process.on("message", (e: any) => { + this.onMessage(e); + }); } } @@ -818,7 +809,7 @@ namespace ts.server { startTracing("server", traceDir); } - const ioSession = new IOSession(); + const ioSession = useNodeIpc ? new IpcIOSession() : new IOSession(); process.on("uncaughtException", err => { ioSession.logError(err, "unknown"); }); From 9c4e2bb41c780fa139935e73084154b72624b7a5 Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Fri, 17 Dec 2021 20:27:05 -0800 Subject: [PATCH 5/6] Extract common writeMessage method --- src/server/session.ts | 4 ++++ src/tsserver/nodeServer.ts | 11 ++--------- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/server/session.ts b/src/server/session.ts index a503509f07be5..a6df5491f3d54 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -940,6 +940,10 @@ namespace ts.server { } return; } + this.writeMessage(msg); + } + + protected writeMessage(msg: protocol.Message) { const msgText = formatMessage(msg, this.logger, this.byteLength, this.host.newLine); perfLogger.logEvent(`Response message size: ${msgText.length}`); this.host.write(msgText); diff --git a/src/tsserver/nodeServer.ts b/src/tsserver/nodeServer.ts index 0861086fc734f..c85943496e386 100644 --- a/src/tsserver/nodeServer.ts +++ b/src/tsserver/nodeServer.ts @@ -761,14 +761,7 @@ namespace ts.server { class IpcIOSession extends IOSession { - public send(msg: protocol.Message) { - if (msg.type === "event" && !this.canUseEvents) { - if (this.logger.hasLevel(LogLevel.verbose)) { - this.logger.info(`Session does not support events: ignored event: ${JSON.stringify(msg)}`); - } - return; - } - + protected writeMessage(msg: protocol.Message): void { const verboseLogging = logger.hasLevel(LogLevel.verbose); if (verboseLogging) { const json = JSON.stringify(msg); @@ -786,7 +779,7 @@ namespace ts.server { return JSON.stringify(message, undefined, 2); } - listen() { + public listen() { process.on("message", (e: any) => { this.onMessage(e); }); From 421e7560a8878f2cf06b9599f38468bdc1da48c5 Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Wed, 5 Jan 2022 16:21:35 -0800 Subject: [PATCH 6/6] Baseline accept --- tests/baselines/reference/api/tsserverlibrary.d.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 116be4316f918..1f270cc92e538 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -10470,6 +10470,7 @@ declare namespace ts.server { logError(err: Error, cmd: string): void; private logErrorWorker; send(msg: protocol.Message): void; + protected writeMessage(msg: protocol.Message): void; event(body: T, eventName: string): void; /** @deprecated */ output(info: any, cmdName: string, reqSeq?: number, errorMsg?: string): void;