fix(graphql-language-service-server): import Logger from vscode-jsonrpc#4331
Conversation
Logger is defined and exported by vscode-jsonrpc (a direct dependency) and only reached vscode-languageserver through a transitive re-export. tsgo failed to resolve that re-export on CI, breaking types:check with "Module 'vscode-languageserver' has no exported member 'Logger'". Import Logger from the package that owns the type to avoid the fragile re-export chain. Closes graphql#4330.
🦋 Changeset detectedLatest commit: f0e2a93 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
trevor-scheer
left a comment
There was a problem hiding this comment.
Fix LGTM but is the PR description correct?
My understanding is that this is a turbo cache issue, but the description also says not reproducible on Mac / platform-specific. Are both things true? Platform-specific would be surprising to me.
|
@trevor-scheer Both are true, and they answer two different questions:
To be fully transparent: I'm confident about the Turbo cache behavior and the fix. The platform mechanism is my best inference, since I only have macOS to test on and can't directly inspect the Linux Happy to tighten the PR description to separate these two points more clearly if that helps. |
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## codemirror-graphql@2.2.7 ### Patch Changes - Updated dependencies [[`a526a10`](a526a10)]: - graphql-language-service@5.5.2 ## @graphiql/react@0.37.6 ### Patch Changes - [#4332](#4332) [`61d138f`](61d138f) Thanks [@vishwakt](https://github.com/vishwakt)! - Fix image previews in the response viewer fetching from the wrong host. Monaco splits the hovered word on `:`, so a full URL like `https://example.com/img.png` reaches the preview as the protocol-relative `//example.com/img.png`. The preview stripped the leading character, turning that into the host-relative `/example.com/img.png` and fetching it from the current origin. The original host is now preserved. - Updated dependencies [[`a526a10`](a526a10)]: - graphql-language-service@5.5.2 ## graphql-language-service@5.5.2 ### Patch Changes - [#4329](#4329) [`a526a10`](a526a10) Thanks [@vishwakt](https://github.com/vishwakt)! - Make the fields block optional when parsing object, interface, input, and enum type definitions (and their extensions) in the online parser. Per the GraphQL spec these blocks are optional, so spec-valid SDL such as `extend type Foo @onType` or `type Foo @onType` (directives only, no body) no longer reports `invalidchar` during syntax highlighting. ## graphql-language-service-server@2.14.10 ### Patch Changes - [#4328](#4328) [`d6b71ce`](d6b71ce) Thanks [@vishwakt](https://github.com/vishwakt)! - Move `debounce-promise` from `devDependencies` to `dependencies`. It is imported at runtime in `MessageProcessor.ts`, so it must be a regular dependency. Previously the package only resolved it via hoisting, which fails under strict installs (e.g. `pnpm` v11), causing `graphql-lsp` to crash with `Cannot find module 'debounce-promise'`. - [#4331](#4331) [`e1077b9`](e1077b9) Thanks [@vishwakt](https://github.com/vishwakt)! - Import `Logger` from `vscode-jsonrpc` instead of `vscode-languageserver`. `Logger` is defined in `vscode-jsonrpc` (a direct dependency) and only reached `vscode-languageserver` through a transitive re-export, which `tsgo` failed to resolve on CI (`Module '"vscode-languageserver"' has no exported member 'Logger'`). Importing from the package that owns the type avoids relying on that fragile re-export chain. - Updated dependencies [[`a526a10`](a526a10)]: - graphql-language-service@5.5.2 ## vscode-graphql-execution@0.3.6 ### Patch Changes - [#4306](#4306) [`7aefde8`](7aefde8) Thanks [@dependabot](https://github.com/apps/dependabot)! - Bump `ws` to 8.20.1 to address [GHSA-58qx-3vcg-4xpx](GHSA-58qx-3vcg-4xpx) (uninitialized memory disclosure in `websocket.close()`). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary
graphql-language-service-serverimported theLoggertype fromvscode-languageserver, butLoggeris defined invscode-jsonrpcand only reachesvscode-languageserverthrough a transitive re-export (export * from 'vscode-languageserver-protocol/'). On CI this breakstypes:check:This imports
Loggerfromvscode-jsonrpcinstead, which owns the type and is already a direct dependency of the package (vscode-jsonrpc: ^8.0.1), avoiding the fragile re-export chain.Fixes #4330.
Two separate things are going on
These answer different questions and are both true:
Why it surfaced now (Turborepo cache).
graphql-language-service-server#types:checkis a cache hit for any PR that does not invalidate it, so the stale green result is reused. Only a PR that changesgraphql-language-service(a dependency) busts that cache and forces a real re-run. That is when the latent error becomes visible (it surfaced on #4329, agraphql-language-servicechange).Where the type error manifests (platform). When
types:checkactually runs, it fails on CI (Linux) but passes locally (macOS). The lockfile pins identical dependency versions in both places (vscode-languageserver@8.0.1 -> vscode-languageserver-protocol@3.17.1), so resolution is the only variable.tsgois@typescript/native-preview, which ships a separate compiled native binary per OS/arch (native-preview-linux-x64on CI vsnative-preview-darwin-arm64locally), and the pre-release binaries appear to resolve the trailing-slash re-export differently.I am confident about the cache behavior and the fix. The platform mechanism is my best inference, since I can only test on macOS and cannot directly inspect the Linux
tsgo's resolution. Either way the fix is robust because it importsLoggerfrom the package that owns it.Verification
tsgo --noEmitandoxlintpass locally on the changed package.types:checkand forces a fresh run on Linux, andTypes Check(along with all other checks) passes.