fix(oxlint,oxfmt): Redirect JS stdout to stderr for LSP

[leaysgur]

Fixes #20320

(It's fine to release this as scheduled one)

[leaysgur]

• fix(oxlint,oxfmt): Redirect JS stdout to stderr for LSP #20321 : 2 dependent PRs (#20327 , #20417 ) 👈 (View in Graphite)
• main

---

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

• 0-merge - adds this PR to the back of the merge queue
• hotfix - for urgent changes, fast-track this PR to the front of the merge queue

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

[Copilot]

Pull request overview

Adds coverage and implementation to prevent JavaScript/TypeScript config files from corrupting LSP communication by printing to stdout during config evaluation.

Changes:

• Add LSP test fixtures and snapshots for config files that write to stdout (oxlint + oxfmt).
• Wrap dynamic import() of config modules with temporary stdout suppression in both config loaders.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
apps/oxlint/test/lsp/lint/lint.test.ts	Adds a new LSP lint fixture case for stdout-polluting TS config.
apps/oxlint/test/lsp/lint/fixtures/config-ts-stdout-pollution/test.js	Fixture source to trigger a diagnostic under the loaded config.
apps/oxlint/test/lsp/lint/fixtures/config-ts-stdout-pollution/oxlint.config.ts	Fixture config that intentionally logs to stdout.
apps/oxlint/test/lsp/lint/snapshots/lint.test.ts.snap	Snapshot for the new lint fixture.
apps/oxlint/src-js/js_config.ts	Implements stdout suppression during config import().
apps/oxfmt/test/lsp/format/format.test.ts	Adds a new LSP format fixture case for stdout-polluting config.
apps/oxfmt/test/lsp/format/fixtures/config-js-stdout-pollution/test.ts	Fixture input for formatting under the loaded config.
apps/oxfmt/test/lsp/format/fixtures/config-js-stdout-pollution/oxfmt.config.ts	Fixture config that intentionally logs to stdout.
apps/oxfmt/test/lsp/format/snapshots/format.test.ts.snap	Snapshot for the new format fixture.
apps/oxfmt/src-js/cli/js_config.ts	Implements stdout suppression during config import().

---

You can also share your feedback on Copilot code review. Take the survey.

[camc314]

should we not block stdout writing for the entirity of the LS process?

else a console.log, during formatting for example will break the LSP

[Sysix]

Thank you! This is not my code-ownership, but I found maybe one little improvement :)

[Sysix]

should we not block stdout writing for the entirity of the LS process?

else a console.log, during formatting for example will break the LSP

This would probably the best, but I do not think this will break atm. The config loading happens at the start, after that there can be multiple formatting requests. And here we are only looking at the source text as &str?

Could maybe help to avoid third party outputs like: #18472

[leaysgur]

Thanks to both of you. I'll take a look at it early next week. 👍🏻

[liangmiQwQ]

I happened to meet the same problem while I forgot to check the issue and the PR, so I made a fix in my personal branch again. Silly me 😂.

Different from just disabling stdout, my approach is to redirect all stdout to stderr globally, this is actually a more common practice, make users can still able to find messages printed if they run it in terminal or VSCode's output console. And typescript language server did a similar thing as well.

// https://github.com/typescript-language-server/typescript-language-server
if (this.level >= level || options?.overrideLevel) {
// All messages logged to stderr as stdout is reserved for LSP communication.
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
console.error(`[${LogLevel.toString(level)}]`, ...this.toStrings(...args));

In this case, we can also solve the problem of Oxlint JS Plugin without worrying, because keeping the message visible to the user on stderr won't break any feature.

I don't know whether it is helpful to solve this issue, and I am willing to submit this branch as another PR if needed. Thanks.

[leaysgur]

Actually, that's very helpful!

I was thinking about that at first, but I wasn't sure if it was the right approach.

I'll finish this PR in that direction. Thank you~ 🙌🏻

[leaysgur]

UPDATED: When using --lsp, redirect all stdout to stderr for the JS main thread.

[camc314]

Merge activity

• Mar 16, 10:11 AM UTC: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
• Mar 16, 10:11 AM UTC: camc314 added this pull request to the Graphite merge queue.
• Mar 16, 10:27 AM UTC: Merged by the Graphite merge queue.

[Sysix]

Thank you!