Conversation
📝 WalkthroughWalkthroughThis PR adds a Volar-based language server and language-service surface: new packages Possibly related PRs
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
extensions/vscode/tsdown.config.ts (1)
8-9: Consider adding a brief TODO comment explaining the purpose.The commented-out copy directives lack context. Adding a short comment explaining these are for future language server bundling would improve maintainability.
copy: [ '../../res', - // { from: 'node_modules/npmx-language-server/bin/**', to: 'dist/server/bin' }, - // { from: 'node_modules/npmx-language-server/dist/**', to: 'dist/server/dist' }, + // TODO: Enable when language server integration is ready + // { from: 'node_modules/npmx-language-server/bin/**', to: 'dist/server/bin' }, + // { from: 'node_modules/npmx-language-server/dist/**', to: 'dist/server/dist' }, ],extensions/vscode/src/client.ts (1)
49-51:client.start()returns a Promise that is neither awaited nor returned.Errors during client startup will be silently swallowed. Consider returning or awaiting the promise so callers can handle failures.
♻️ Option A: Make `launch` async
-export function launch(serverPath: string) { +export async function launch(serverPath: string) { const client = new LanguageClient( ... ) - client.start() + await client.start() return client }♻️ Option B: Return the start promise separately
- client.start() - - return client + return { client, ready: client.start() } }packages/language-server/package.json (1)
34-48: TheinlinedDependenciesfield is non-standard.This field is not recognised by npm/pnpm and appears to be custom metadata documenting which dependency versions are bundled. This is fine for documentation purposes, but consider whether this information might drift out of sync with actual bundled versions over time.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1983a9f1-40a9-43c5-94f7-44ea4708cc29
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
extensions/vscode/package.jsonextensions/vscode/src/client.tsextensions/vscode/src/index.tsextensions/vscode/tsdown.config.tspackages/language-core/tsdown.config.tspackages/language-server/bin/index.jspackages/language-server/package.jsonpackages/language-server/src/index.tspackages/language-server/src/server.tspackages/language-server/tsconfig.jsonpackages/language-server/tsdown.config.tspackages/language-service/package.jsonpackages/language-service/src/index.tspackages/language-service/tsconfig.jsonpackages/language-service/tsdown.config.tsplugins/umd-to-esm.tspnpm-workspace.yamltsconfig.json
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
extensions/vscode/tsdown.config.ts (1)
8-10: Please remove commented-out copy directives from config.Keeping inactive config blocks commented in-place can drift quickly; prefer tracking this in an issue/PR note and keeping
tsdown.config.tsexecutable-only.As per coding guidelines "Write minimal comments—only when truly necessary, prefix hacks with
// HACK:".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8d92559d-485b-464d-ae6f-d25e0975ffc2
📒 Files selected for processing (3)
extensions/vscode/src/client.tsextensions/vscode/src/index.tsextensions/vscode/tsdown.config.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- extensions/vscode/src/client.ts
No description provided.