feat(lsp): add native TypeScript 7 (typescript-go) LSP support#120
Conversation
Auto-detect project-local typescript-go builds. Fall back to the classic TypeScript language server. Support pull diagnostics for native servers, with tests and documentation.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이번 풀 리퀘스트는 LSP 클라이언트 및 서버에 네이티브 TypeScript 7(typescript-go / tsgo) 지원을 추가합니다. 클라이언트가 기존의 push 방식 외에도 pull 방식(textDocument/diagnostic)으로 진단을 가져올 수 있도록 개선하였으며, 프로젝트 로컬에 설치된 네이티브 TypeScript 7 컴파일러를 자동으로 감지하여 실행하는 로직을 구현했습니다. 이에 대한 피드백으로, textDocument/diagnostic 요청 시 무한 대기를 방지하기 위한 타임아웃 적용과 Windows 환경에서 .cmd 실행 파일이 정상적으로 spawn될 수 있도록 shell: true 옵션을 추가할 것을 제안합니다.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for Native TypeScript 7 (typescript-go) by automatically selecting the native compiler (tsc or tsgo) when available in the workspace, falling back to the classic language server otherwise. To support this, the LSP client has been updated to handle the LSP pull diagnostics model (textDocument/diagnostic). The review feedback highlights potential state mismatch issues in packages/lsp/src/client.ts due to inconsistent path normalization when reading or writing to the diagnosticPulls map in both pullDiagnostics and close functions.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for Native TypeScript 7 (typescript-go) by implementing LSP pull-model diagnostics (textDocument/diagnostic) alongside the existing push-model. The TypeScript server now auto-selects native tsc or tsgo binaries when available in the workspace. Feedback on these changes highlights a potential responsiveness issue where awaiting diagnostic pulls blocks file open and change operations, and a test compatibility issue on Windows where binary extensions (.cmd) are not properly handled in test helpers.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for the native TypeScript 7 compiler (typescript-go) by automatically upgrading the TypeScript language server to use tsc --lsp or tsgo when a native build is detected in the workspace. To support this, the LSP client has been updated to handle the pull-based diagnostics model (textDocument/diagnostic) alongside the classic push-based model, complete with new unit tests and fake server fixtures. The review feedback highlights a correctness issue in packages/lsp/src/client.ts where waitForDiagnostics only resolves the first listener when multiple concurrent listeners wait for the same file path, and suggests using filter instead of find to resolve all matching listeners.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for native TypeScript 7 (typescript-go) by auto-selecting tsc or tsgo when available, and falling back to the classic typescript-language-server otherwise. It also implements the LSP pull diagnostics model (textDocument/diagnostic) in the client to support these native servers. The reviewer pointed out a medium-severity issue where pullDiagnostics failures or non-full responses (such as 'unchanged') cause pending waitForDiagnostics listeners to hang for the full 3-second timeout. They suggested immediately resolving these listeners with the current or empty diagnostics in case of errors or unchanged reports.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for Native TypeScript 7 (typescript-go/tsgo) by enabling the LSP client to handle the pull-diagnostics model (textDocument/diagnostic) alongside the traditional push model. The TypeScript server definition was updated to auto-detect and spawn local native compiler binaries (tsc or tsgo) when present in the workspace, falling back to the classic language server otherwise. Corresponding unit tests and fake LSP servers were added to verify these integration paths. A review comment suggests improving the native package detection logic in typescript.ts to immediately return the version check result if a valid major version is parsed, preventing unnecessary and potentially incorrect file-based marker checks for older TypeScript versions.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 13 files
Architecture diagram
sequenceDiagram
participant UI as Editor UI
participant Client as LSP Client
participant Resolver as Native TS Resolver
participant FS as File System
participant TS7 as Native TS7 (tsc/tsgo)
participant TSCL as TS Language Server (fallback)
participant Diag as Diagnostics Store
Note over UI,Diag: NEW: Native TypeScript 7 LSP Flow
UI->>Client: open file (.ts/.tsx/.js/.mjs/.cjs/.mts/.cts)
Client->>Resolver: resolveNativeTypeScriptServer(root)
Resolver->>FS: find node_modules/typescript/package.json
FS-->>Resolver: package metadata
alt TS7 Native Detected
Resolver->>Resolver: Check version ≥ 7 OR getExePath.js OR missing tsserver.js
Resolver->>FS: Check node_modules/.bin/tsc (or tsgo)
FS-->>Resolver: binary path
Resolver-->>Client: { command: tsc, label: 'native tsc' }
Client->>Client: Initialize with diagnostic capability
Client->>TS7: spawn tsc --lsp --stdio
TS7-->>Client: initialize result with diagnosticProvider
Client->>Client: supportsPullDiagnostics = true
else @typescript/native-preview Detected
Resolver->>FS: Walk up for node_modules/.bin/tsgo
FS-->>Resolver: tsgo binary path
Resolver-->>Client: { command: tsgo, label: 'native tsgo' }
Client->>Client: Initialize with diagnostic capability
Client->>TS7: spawn tsgo --lsp --stdio
TS7-->>Client: initialize result with diagnosticProvider
Client->>Client: supportsPullDiagnostics = true
else Classic Fallback
Resolver-->>Client: undefined
Client->>TSCL: spawn typescript-language-server --stdio
TSCL-->>Client: initialize result without diagnosticProvider
Client->>Client: supportsPullDiagnostics = false
end
Note over Client,Diag: File Open with Pull Diagnostics
Client->>Client: open file, generate pullID
Client->>TS7: textDocument/didOpen
Client->>TS7: textDocument/diagnostic (with pullID)
alt Successful Pull
TS7-->>Client: { kind: 'full', items: [...] }
Client->>Client: Check pullID matches current
alt Match
Client->>Diag: applyDiagnostics(file, items)
Diag-->>Client: resolve waiters
Client->>UI: onDiagnostics callback
else Superseded
Client->>Client: Discard stale response
end
else Unchanged Pull
TS7-->>Client: { kind: 'unchanged' }
Client->>Client: Keep existing diagnostics map
Client->>Diag: applyDiagnostics(file, existing)
else Failed Pull
TS7-->>Client: Error response
Client->>Client: Settle waiters with current diagnostics
Diag-->>Client: resolve waiters (empty/fresh)
end
Note over Client,Diag: File Change
Client->>Client: didChange, increment pullID
Client->>TS7: textDocument/didChange
Client->>TS7: textDocument/diagnostic (new pullID)
TS7-->>Client: Pull response
Client->>Client: Validate pullID before applying
Note over Client,Diag: File Close
Client->>Client: close file, increment pullID to invalidate pending pulls
Client->>TS7: textDocument/didClose
Client->>Diag: Delete entry
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Greptile SummaryThis PR adds native TypeScript 7 (
Confidence Score: 4/5Safe to merge for the common case; calling waitForDiagnostics after open() on a pull-model server silently times out after 3 s. The implementation is well-tested and prior review feedback was fully applied. One remaining gap: waitForDiagnostics registered after open() resolves on a pull-model server misses the notification that pullDiagnostics already fired during open(), causing a silent 3-second timeout. All existing tests avoid this by registering the waiter first, but the reverse order is not protected. packages/lsp/src/client.ts — the ordering contract between open() and waitForDiagnostics() on pull-model servers warrants a guard or documentation. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[TypescriptServer.spawn root] --> B{resolveNativeTypeScriptServer}
B --> C[findTypeScriptPackageRoot]
C --> D{typescript package found?}
D -- No --> E[Walk up for @typescript/native-preview]
D -- Yes --> F{isNativeTypeScriptPackage?}
F -- No --> E
F -- Yes --> G[resolvePackageBin tsc]
G --> H{tsc shim valid?}
H -- Yes --> I[return native tsc]
H -- No --> E
E --> J{tsgo shim found?}
J -- Yes --> K[return native tsgo]
J -- No --> L[Walk parent dir]
L --> M{reached root?}
M -- No --> E
M -- Yes --> N[return undefined]
I --> O[spawn tsc --lsp --stdio]
K --> O
N --> P{Bun.which typescript-language-server}
P -- Found --> Q[spawn tsserver --stdio]
P -- Not found --> R[spawn bunx tsserver --stdio]
O --> S[createLSPClient]
Q --> S
R --> S
S --> T{diagnosticProvider in capabilities?}
T -- Yes --> U[pull model: textDocument/diagnostic]
T -- No --> V[push model: publishDiagnostics]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[TypescriptServer.spawn root] --> B{resolveNativeTypeScriptServer}
B --> C[findTypeScriptPackageRoot]
C --> D{typescript package found?}
D -- No --> E[Walk up for @typescript/native-preview]
D -- Yes --> F{isNativeTypeScriptPackage?}
F -- No --> E
F -- Yes --> G[resolvePackageBin tsc]
G --> H{tsc shim valid?}
H -- Yes --> I[return native tsc]
H -- No --> E
E --> J{tsgo shim found?}
J -- Yes --> K[return native tsgo]
J -- No --> L[Walk parent dir]
L --> M{reached root?}
M -- No --> E
M -- Yes --> N[return undefined]
I --> O[spawn tsc --lsp --stdio]
K --> O
N --> P{Bun.which typescript-language-server}
P -- Found --> Q[spawn tsserver --stdio]
P -- Not found --> R[spawn bunx tsserver --stdio]
O --> S[createLSPClient]
Q --> S
R --> S
S --> T{diagnosticProvider in capabilities?}
T -- Yes --> U[pull model: textDocument/diagnostic]
T -- No --> V[push model: publishDiagnostics]
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
/gemini review |
There was a problem hiding this comment.
Code Review
본 풀 리퀘스트는 프로젝트 내에 Native TypeScript 7(typescript-go/tsgo)이 존재할 경우 이를 자동으로 감지하여 활성화하고, LSP pull-diagnostics 모델(textDocument/diagnostic)을 통해 진단을 수행하는 기능을 추가합니다. 리뷰 결과, resolvePackageBin에서 fs.realpathSync를 통한 엄격한 경로 비교가 래퍼 셸 스크립트 환경에서 오탐을 유발할 수 있으므로 fs.existsSync로 단순화할 것이 권장됩니다. 또한, pullDiagnostics에서 타임아웃이나 에러 발생 시 서버 측 리소스 낭비를 방지하기 위해 cancellation.cancel()을 호출하여 명시적으로 취소 요청을 전송하도록 개선이 필요합니다.
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
/gemini review |
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request implements support for project-local native TypeScript 7 (typescript-go) in the LSP package. It introduces automatic detection and spawning of native tsc or tsgo (from @typescript/native-preview) when available in the workspace, falling back to the classic typescript-language-server otherwise. To support this, the LSP client has been enhanced to handle the LSP pull-diagnostics model (textDocument/diagnostic) with proper request cancellation and timeout handling. Comprehensive unit tests and fake servers have been added to verify the pull-diagnostics client logic and the native server resolution paths. No review comments were provided, so there is no additional feedback to address.
There was a problem hiding this comment.
Code Review
This pull request introduces support for native TypeScript 7 (typescript-go) in the LSP package, enabling the client to automatically detect and spawn local tsc or tsgo servers and handle diagnostics via the LSP pull model (textDocument/diagnostic). The changes include updates to the LSP client, server resolution logic, documentation, and comprehensive unit tests. The review feedback highlights two important issues: a potential unhandled promise rejection on timeout within the pullDiagnostics function, and a case-sensitive path comparison in resolvePackageBin that could cause detection failures on Windows.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for native TypeScript 7 (typescript-go) by automatically detecting and spawning tsc or tsgo in LSP mode when a project-local native build is present, falling back to the classic typescript-language-server otherwise. It also implements the LSP pull-diagnostics model (textDocument/diagnostic) in the client to handle diagnostics from these native servers. The review feedback highlights a critical issue where native server detection fails in pnpm environments due to symlink path mismatches, suggesting the use of fs.realpathSync to resolve physical paths, along with a corresponding regression test to verify this behavior.
|
/gemini review |
|
There was a problem hiding this comment.
Code Review
본 풀 리퀘스트는 프로젝트 로컬의 네이티브 TypeScript 7(typescript-go) 언어 서버(tsc 또는 tsgo)를 자동 감지하여 기존 typescript-language-server에서 자동으로 업그레이드하는 기능을 추가합니다. 이를 위해 LSP 클라이언트에 요청 취소, 타임아웃 및 에러 처리가 포함된 풀 기반 진단 모델(textDocument/diagnostic) 지원을 구현했습니다. 또한, 테스트 픽스처를 리팩터링하여 JSON-RPC 전송 로직을 분리하고, 풀 진단 모의 서버 및 네이티브 서버 감지 로직에 대한 포괄적인 단위 테스트를 추가했습니다. 관련 문서들도 이에 맞춰 업데이트되었습니다. 제출된 리뷰 코멘트가 없으므로, 변경 사항에 대한 추가 피드백은 없습니다.
1 participant
Summary
Adds native TypeScript 7 (
typescript-go) LSP support without introducing a second configured server. The existingtypescriptserver now selects the appropriate project-local binary at spawn time, preventing duplicate diagnostics and requiring no configuration changes.Detection in
packages/lsp/src/server/typescript.tsselects:node_modules/.bin/tsc --lsp --stdiowhen localtypescriptis major version 7 or newer,lib/getExePath.jsexists, orlib/tsserver.jsis absentnode_modules/.bin/tsgo --lsp --stdiowhen@typescript/native-previewprovidestsgotypescript-language-server --stdiootherwiseThe LSP client now supports pull diagnostics through
textDocument/diagnosticwhen the server advertises adiagnosticProvider. Existing push-diagnostics handling remains unchanged.Tests cover eight native TypeScript detection scenarios, pull-diagnostics client behavior, and a pull-model fake server fixture. Documentation has been updated across the root and LSP package guidance, architecture documentation, and docs-site configuration and supported-language pages.
Verification completed:
lspandcodepackages typecheckRelated issue
None.
Checklist
bun run lint)BREAKING CHANGE:note is includedSummary by cubic
Adds native TypeScript 7 LSP by auto-selecting project-local
tsc --lsportsgo, with fallback totypescript-language-server. Pull diagnostics are non-blocking; unchanged or failed pulls settle waiters and never hang.New Features
typescriptserver auto-detects native TS7 per project (tscortsgo) using version/file markers and hoisted installs; resolves pnpm symlinked packages, validates.binshims (symlink/wrapper), normalizes Windows.cmd, rejects unrelated shims, and supports.mts/.cts. Falls back totypescript-language-serverwhen no native build is found.textDocument.diagnostic, pulls on open/change, cancels on close, ignores stale replies, uses timeouts, debounces notifications, preserves diags onunchanged, and falls back topublishDiagnostics.Migration
@typescript/native-preview.Written for commit cb76f69. Summary will update on new commits.