From ccc9e6a60c5ac4f8cda6691767c096fcf0602d0c Mon Sep 17 00:00:00 2001 From: Morgan Zolob Date: Fri, 26 Jul 2024 19:38:19 -0700 Subject: [PATCH 1/2] Fix worker call IDs with multiple worker instances Previously, 'currentCallMethodId' was a field on each instance of PrettierWorkerInstance, however, there was only a single instance of 'worker'. So when there were multiple instances of PrettierWorkerInstance, they would each have their own 'currentCallMethodId' which could overlap. This meant that the instances could not reliably get results back from the worker, as there were multiple results with the same ID. This results in many issues, such as overwriting files with the contents of other files while formatting. This change moves 'currentCallMethodId' outside PrettierWorkerInstance, so that all instances will use the same value and cannot have overlapping IDs. --- CHANGELOG.md | 2 ++ src/PrettierWorkerInstance.ts | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 10c891a22..d34e563ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ All notable changes to the "prettier-vscode" extension will be documented in thi ## [Unreleased] +- Fix issue where formatting multiple files in a workspace with multiple instances of Prettier could result in files being overwritten with the contents of other files (#3423, #3040) + ## [10.5.0] - Extend list of Prettier config files by ECMAScript module extentions diff --git a/src/PrettierWorkerInstance.ts b/src/PrettierWorkerInstance.ts index 5ca413f28..696334155 100644 --- a/src/PrettierWorkerInstance.ts +++ b/src/PrettierWorkerInstance.ts @@ -14,6 +14,8 @@ import { PrettierSupportLanguage, } from "./types"; +let currentCallMethodId = 0; + const worker = new Worker( url.pathToFileURL(path.join(__dirname, "/worker/prettier-instance-worker.js")) ); @@ -34,8 +36,6 @@ export const PrettierWorkerInstance: PrettierInstanceConstructor = class Prettie } > = new Map(); - private currentCallMethodId = 0; - public version: string | null = null; constructor(private modulePath: string) { @@ -124,7 +124,7 @@ export const PrettierWorkerInstance: PrettierInstanceConstructor = class Prettie // eslint-disable-next-line @typescript-eslint/no-explicit-any private callMethod(methodName: string, methodArgs: unknown[]): Promise { - const callMethodId = this.currentCallMethodId++; + const callMethodId = currentCallMethodId++; const promise = new Promise((resolve, reject) => { this.callMethodResolvers.set(callMethodId, { resolve, reject }); }); From bc3cf90cd6ae8e724b8b5bf630b032f6b04192c2 Mon Sep 17 00:00:00 2001 From: Morgan Zolob Date: Fri, 26 Jul 2024 21:08:40 -0700 Subject: [PATCH 2/2] Use ID in all messages to worker The same issue that existed for the callMethod messages also existed for the import messages, where multiple PrettierWorkerInstances could get messages meant for a different instance, so the 'version' field could end up with the wrong value. This changes the code to include an ID in all messages sent to the worker, so that the PrettierWorkerInstances can properly determine whether a message is meant for them or not. --- src/PrettierWorkerInstance.ts | 47 ++++++++++++-------------- src/worker/prettier-instance-worker.js | 26 ++++++++------ 2 files changed, 37 insertions(+), 36 deletions(-) diff --git a/src/PrettierWorkerInstance.ts b/src/PrettierWorkerInstance.ts index 696334155..664dd70ab 100644 --- a/src/PrettierWorkerInstance.ts +++ b/src/PrettierWorkerInstance.ts @@ -14,7 +14,7 @@ import { PrettierSupportLanguage, } from "./types"; -let currentCallMethodId = 0; +let currentCallId = 0; const worker = new Worker( url.pathToFileURL(path.join(__dirname, "/worker/prettier-instance-worker.js")) @@ -23,12 +23,7 @@ const worker = new Worker( export const PrettierWorkerInstance: PrettierInstanceConstructor = class PrettierWorkerInstance implements PrettierInstance { - private importResolver: { - resolve: (version: string) => void; - reject: (version: string) => void; - } | null = null; - - private callMethodResolvers: Map< + private messageResolvers: Map< number, { resolve: (value: unknown) => void; @@ -39,38 +34,40 @@ export const PrettierWorkerInstance: PrettierInstanceConstructor = class Prettie public version: string | null = null; constructor(private modulePath: string) { - worker.on("message", ({ type, payload }) => { - switch (type) { - case "import": { - this.importResolver?.resolve(payload.version); - this.version = payload.version; - break; - } - case "callMethod": { - const resolver = this.callMethodResolvers.get(payload.id); - this.callMethodResolvers.delete(payload.id); - if (resolver) { + worker.on("message", ({ type, id, payload }) => { + const resolver = this.messageResolvers.get(id); + if (resolver) { + this.messageResolvers.delete(id); + switch (type) { + case "import": { + resolver.resolve(payload.version); + this.version = payload.version; + break; + } + case "callMethod": { if (payload.isError) { resolver.reject(payload.result); } else { resolver.resolve(payload.result); } + break; } - break; } } }); } public async import(): Promise { - const promise = new Promise((resolve, reject) => { - this.importResolver = { resolve, reject }; + const callId = currentCallId++; + const promise = new Promise((resolve, reject) => { + this.messageResolvers.set(callId, { resolve, reject }); }); worker.postMessage({ type: "import", + id: callId, payload: { modulePath: this.modulePath }, }); - return promise; + return promise as Promise; } public async format( @@ -124,14 +121,14 @@ export const PrettierWorkerInstance: PrettierInstanceConstructor = class Prettie // eslint-disable-next-line @typescript-eslint/no-explicit-any private callMethod(methodName: string, methodArgs: unknown[]): Promise { - const callMethodId = currentCallMethodId++; + const callId = currentCallId++; const promise = new Promise((resolve, reject) => { - this.callMethodResolvers.set(callMethodId, { resolve, reject }); + this.messageResolvers.set(callId, { resolve, reject }); }); worker.postMessage({ type: "callMethod", + id: callId, payload: { - id: callMethodId, modulePath: this.modulePath, methodName, methodArgs, diff --git a/src/worker/prettier-instance-worker.js b/src/worker/prettier-instance-worker.js index 1b7ce9b6c..56c42c738 100644 --- a/src/worker/prettier-instance-worker.js +++ b/src/worker/prettier-instance-worker.js @@ -14,7 +14,7 @@ function requireInstance(modulePath) { return prettierInstance; } -parentPort.on("message", ({ type, payload }) => { +parentPort.on("message", ({ type, id, payload }) => { switch (type) { case "import": { const { modulePath } = payload; @@ -22,22 +22,32 @@ parentPort.on("message", ({ type, payload }) => { const prettierInstance = requireInstance(modulePath); parentPort.postMessage({ type, + id, payload: { version: prettierInstance.version }, }); } catch { parentPort.postMessage({ type, + id, payload: { version: null }, }); } break; } case "callMethod": { - const { modulePath, methodName, methodArgs, id } = payload; + const { modulePath, methodName, methodArgs } = payload; const postError = (error) => { parentPort.postMessage({ type, - payload: { result: error, id, isError: true }, + id, + payload: { result: error, isError: true }, + }); + }; + const postResult = (result) => { + parentPort.postMessage({ + type, + id, + payload: { result, isError: false }, }); }; let prettierInstance = path2ModuleCache.get(modulePath); @@ -62,10 +72,7 @@ parentPort.on("message", ({ type, payload }) => { if (methodName === "getSupportInfo") { value = { languages: value.languages }; } - parentPort.postMessage({ - type, - payload: { result: value, id, isError: false }, - }); + postResult(value); } catch (error) { postError(error); } @@ -81,10 +88,7 @@ parentPort.on("message", ({ type, payload }) => { if (methodName === "getSupportInfo") { result = { languages: result.languages }; } - parentPort.postMessage({ - type, - payload: { result, id, isError: false }, - }); + postResult(result); } catch (error) { postError(error); }