-
-
Notifications
You must be signed in to change notification settings - Fork 455
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
Run prettier instance in worker_threads
#3016
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
I've been very busy recently, and I'm not familiar with VSCode, but I'll try to find some time to take a look. |
let actual = doc.getText(); | ||
|
||
if (shouldRetry) { | ||
for (let i = 0; i < 10; i++) { | ||
if (text !== actual) { | ||
break; | ||
} | ||
await wait(150); | ||
await vscode.commands.executeCommand("editor.action.formatDocument"); | ||
actual = doc.getText(); | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some code, like PHP files, can take a bit of time, like opening a text editor and then requiring plugins and huge parsers.
If you run formatDocument without waiting for that, it won't actually format the file.
We don't want that, so we'll only retry for some tests.
It's not the best way, but I don't think it's a problem anyway.
Just doing Object.assign was enough...? |
No, just rejecting the error object passed from the worker produced the same output |
Error serialization may not be necessary here. We can add it if we need it. |
I don't quite understand, But we can use the following hack: |
Besides, the current solution does not consider yarn PnP support, |
We can't use Prettier directly since extensions don't allow dynamic imports: prettier/prettier-vscode#3016 Instead we find and read Prettier config file manually. This solution is limited -- we only support JavaScript and JSON configs, as well as configs in package.json.
This Pull Request runs instances of Prettier on
worker_threads
.Prettier v3 depends on
import()
for loading configs and plugins. However,import()
cannot be executed from the client of the VSCode extension, because the VSCode extension is run withvm.Script
. For details, please see microsoft/vscode#130367.We initially tried to solve this problem by introducing a language server, much like vscode-eslint. The language server can execute
import()
. Therefore, by running instances of Prettier on the language server, we should be able to resolve this issue.I created a prototype of this. However, I determined that it is difficult to introduce a language server while maintaining the current behavior of prettier-vscode as much as possible.
Therefore, I decided to take another approach.
According to @liuxingbaoyu's comment (#2947 (comment)), VSCode extensions can use
worker_threads
and a Worker can executeimport()
.Thus, this PR makes it so that instances of Prettier are run on
Worker
ofworker_threads
.However, the current default instance of prettier-vscode is 2.x, which does not depend on
import()
and does not need to be run onWorker
. Therefore, only those loaded fromnode_modules
will be executed onWorker
.CHANGELOG.md
with a summary of your changes