Skip to content

Comments

refactor(language_server): Add formatter and linter features#15767

Merged
graphite-app[bot] merged 1 commit intomainfrom
11-17-refactor_language_server_add_formatter_and_linter_features
Nov 17, 2025
Merged

refactor(language_server): Add formatter and linter features#15767
graphite-app[bot] merged 1 commit intomainfrom
11-17-refactor_language_server_add_formatter_and_linter_features

Conversation

@leaysgur
Copy link
Member

@leaysgur leaysgur commented Nov 17, 2025

Currently, there is 2 ways to start LS for oxc apps.

  • oxc_language_server
  • oxlint --lsp
  • (oxfmt --lsp will be added soon)

Having duplicates are fine for now, but ServerFormatter (and oxc_formatter) should not bundled in oxlint --lsp case.

This PR adds linter and formatter features to exclude.
For oxc_language_server, both features are enabled by default, keep it as-is.

Copilot AI review requested due to automatic review settings November 17, 2025 07:07
@github-actions github-actions bot added A-linter Area - Linter A-cli Area - CLI A-editor Area - Editor and Language Server C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior labels Nov 17, 2025
Copy link
Member Author

leaysgur commented Nov 17, 2025


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 hot fixes, skip the queue and merge this PR next

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.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the language server to make the formatter and linter features optional and independently configurable, allowing for smaller binary sizes and more flexible compilation options.

  • Added Cargo feature flags for formatter and linter with both enabled by default
  • Made oxc_formatter and oxc_linter optional dependencies
  • Added conditional compilation gates throughout the codebase for feature-specific code

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
crates/oxc_language_server/src/main.rs Updated to conditionally push tools based on enabled features
crates/oxc_language_server/src/lib.rs Added feature gates for formatter and linter module declarations and exports
crates/oxc_language_server/Cargo.toml Made formatter and linter dependencies optional and defined corresponding features with both in default
apps/oxlint/Cargo.toml Explicitly enabled the linter feature for oxc_language_server dependency
Cargo.toml Set default-features = false for workspace-level oxc_language_server dependency to allow consumers to opt-in to features

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@leaysgur leaysgur force-pushed the 11-17-refactor_language_server_add_formatter_and_linter_features branch 2 times, most recently from 39ff395 to 1b35906 Compare November 17, 2025 07:46
@Boshen
Copy link
Member

Boshen commented Nov 17, 2025

We have #15738 down the road.

Copy link
Member

@Sysix Sysix left a comment

Choose a reason for hiding this comment

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

oh here are the features flags :)

@graphite-app graphite-app bot added the 0-merge Merge with Graphite Merge Queue label Nov 17, 2025
@graphite-app
Copy link
Contributor

graphite-app bot commented Nov 17, 2025

Merge activity

graphite-app bot pushed a commit that referenced this pull request Nov 17, 2025
Currently, there is 2 ways to start LS for oxc apps.

- `oxc_language_server`
- `oxlint --lsp`
- (`oxfmt --lsp` will be added soon)

Having duplicates are fine for now, but `ServerFormatter` (and `oxc_formatter`) should not bundled in `oxlint --lsp` case.

This PR adds `linter` and `formatter` features to exclude.
For `oxc_language_server`, both features are enabled by default, keep it as-is.
@graphite-app graphite-app bot force-pushed the 11-17-refactor_language_server_add_formatter_and_linter_features branch from 1b35906 to 03e8f28 Compare November 17, 2025 13:32
graphite-app bot pushed a commit that referenced this pull request Nov 17, 2025
Support `--lsp`, fixes #15739

- Implementation still lives in `oxc_language_server`
  - The same as `oxlint --lsp`
  - But now we can remove `oxc_language_server`'s `bin`
- For Rust CLI, `.await` with `tokio`
- For JS CLI, `napi-rs` automatically wraps with `tokio`

---

Now I'm seeing 2 problems:

- For JS CLI, `node ./dist/cli.js --lsp` does not work
  - Instead of waiting LSP message, just panics with error
  - `{"jsonrpc":"2.0","error":{"code":-32700,"message":"Parse error"},"id":null}`
  - `import process2 from "process";` in bundled file makes this happen, but I don't know why
  - 👉🏻 Fixed by import `prettier` lazily...
- `oxc_linter` is also bundled with `oxfmt`
  - via `oxc_language_server`
  - 👉🏻 #15767
Currently, there is 2 ways to start LS for oxc apps.

- `oxc_language_server`
- `oxlint --lsp`
- (`oxfmt --lsp` will be added soon)

Having duplicates are fine for now, but `ServerFormatter` (and `oxc_formatter`) should not bundled in `oxlint --lsp` case.

This PR adds `linter` and `formatter` features to exclude.
For `oxc_language_server`, both features are enabled by default, keep it as-is.
@graphite-app graphite-app bot force-pushed the 11-17-refactor_language_server_add_formatter_and_linter_features branch from 03e8f28 to f1c62b9 Compare November 17, 2025 13:35
graphite-app bot pushed a commit that referenced this pull request Nov 17, 2025
Support `--lsp`, fixes #15739

- Implementation still lives in `oxc_language_server`
  - The same as `oxlint --lsp`
  - But now we can remove `oxc_language_server`'s `bin`
- For Rust CLI, `.await` with `tokio`
- For JS CLI, `napi-rs` automatically wraps with `tokio`

---

Now I'm seeing 2 problems:

- For JS CLI, `node ./dist/cli.js --lsp` does not work
  - Instead of waiting LSP message, just panics with error
  - `{"jsonrpc":"2.0","error":{"code":-32700,"message":"Parse error"},"id":null}`
  - `import process2 from "process";` in bundled file makes this happen, but I don't know why
  - 👉🏻 Fixed by import `prettier` lazily...
- `oxc_linter` is also bundled with `oxfmt`
  - via `oxc_language_server`
  - 👉🏻 #15767
@graphite-app graphite-app bot merged commit f1c62b9 into main Nov 17, 2025
21 checks passed
@graphite-app graphite-app bot deleted the 11-17-refactor_language_server_add_formatter_and_linter_features branch November 17, 2025 13:41
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Nov 17, 2025
taearls pushed a commit to taearls/oxc that referenced this pull request Dec 11, 2025
…ject#15767)

Currently, there is 2 ways to start LS for oxc apps.

- `oxc_language_server`
- `oxlint --lsp`
- (`oxfmt --lsp` will be added soon)

Having duplicates are fine for now, but `ServerFormatter` (and `oxc_formatter`) should not bundled in `oxlint --lsp` case.

This PR adds `linter` and `formatter` features to exclude.
For `oxc_language_server`, both features are enabled by default, keep it as-is.
taearls pushed a commit to taearls/oxc that referenced this pull request Dec 11, 2025
Support `--lsp`, fixes oxc-project#15739

- Implementation still lives in `oxc_language_server`
  - The same as `oxlint --lsp`
  - But now we can remove `oxc_language_server`'s `bin`
- For Rust CLI, `.await` with `tokio`
- For JS CLI, `napi-rs` automatically wraps with `tokio`

---

Now I'm seeing 2 problems:

- For JS CLI, `node ./dist/cli.js --lsp` does not work
  - Instead of waiting LSP message, just panics with error
  - `{"jsonrpc":"2.0","error":{"code":-32700,"message":"Parse error"},"id":null}`
  - `import process2 from "process";` in bundled file makes this happen, but I don't know why
  - 👉🏻 Fixed by import `prettier` lazily...
- `oxc_linter` is also bundled with `oxfmt`
  - via `oxc_language_server`
  - 👉🏻 oxc-project#15767
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-cli Area - CLI A-editor Area - Editor and Language Server A-linter Area - Linter C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants