Fix LSP not loading .js and .ts files correctly#4180
Fix LSP not loading .js and .ts files correctly#4180witoszekdev wants to merge 2 commits intographql:mainfrom
.js and .ts files correctly#4180Conversation
🦋 Changeset detectedLatest commit: 8052aaa The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
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 |
There was a problem hiding this comment.
Pull request overview
This PR updates graphql-config to a newer version to fix GraphQL LSP projects not correctly loading configuration from graphql.config.js / graphql.config.ts.
Changes:
- Bump
graphql-configfrom5.0.3to5.1.5across the LSP-related packages. - Refresh
yarn.lockto reflect the updatedgraphql-configdependency graph. - Add a changeset to publish patch releases for affected packages.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
packages/graphql-language-service-server/package.json |
Updates graphql-config dependency used by the LSP server. |
packages/graphql-language-service/package.json |
Updates devDependency graphql-config used in local/dev workflows. |
packages/vscode-graphql-execution/package.json |
Updates graphql-config dependency used by the VS Code execution extension. |
yarn.lock |
Lockfile refresh for the new graphql-config and transitive dependency updates. |
.changeset/green-lies-tap.md |
Declares patch releases for multiple packages related to this fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "fast-glob": "^3.2.7", | ||
| "glob": "^7.2.0", | ||
| "graphql-config": "5.0.3", | ||
| "graphql-config": "5.1.5", |
There was a problem hiding this comment.
This PR relies on graphql-config@5.1.5 to fix loading graphql.config.{js,ts} files, but the existing test suite here appears to only exercise YAML/JSON configs (and _loadConfigOrSkip tests don’t validate JS/TS config parsing). Please add a regression test that loads an actual graphql.config.js and/or graphql.config.ts fixture and asserts the config is read correctly, to prevent this bug from returning on future dependency updates.
| 'graphql-language-service-server': patch | ||
| 'graphql-language-service': patch | ||
| 'vscode-graphql-execution': patch | ||
| 'graphql-language-service-cli': patch |
There was a problem hiding this comment.
The changeset marks graphql-language-service-cli for a patch release, but this PR doesn’t change that package (and it doesn’t directly depend on graphql-config). Consider removing it from the changeset, or update the CLI’s dependency constraints in a way that makes this release meaningful (e.g., if you intend to pin/bump graphql-language-service-server there).
| 'graphql-language-service-cli': patch |
|
Not sure how to correctly bump version of graphql-language-service-cli since we will have fresh lockfile only after fixes for graphql-language-service are released |
This PR fixes issue in GraphQL LSP for projects that use
graphql.config.tsorgraphql.config.jsfiles.LSP relied on old version of
graphql-config, which usedcosmiconfigincorrectly by passing a default.jsloader tocosmiconfigSyncwhich in turn didn't await the promise and resolved the project's config as{}.graphql-config 5.1.5+ fixes this by using
createJitifor JS/TS loading instead of relying on cosmiconfig's default loaders.Tested this change in NeoVim + Mason + graphql-language-service-cli on https://github.com/saleor/saleor-dashboard