Skip to content

Fix error encountered running in helix#4123

Open
forivall wants to merge 4 commits intographql:mainfrom
forivall:patch-1
Open

Fix error encountered running in helix#4123
forivall wants to merge 4 commits intographql:mainfrom
forivall:patch-1

Conversation

@forivall
Copy link
Copy Markdown

I manually applied this fix and the lsp works now. Single character fix to add a missing ?.

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Oct 29, 2025

🦋 Changeset detected

Latest commit: 2230bf8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
graphql-language-service-server Patch

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

Copy link
Copy Markdown
Contributor

@trevor-scheer trevor-scheer left a comment

Choose a reason for hiding this comment

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

@forivall change seems reasonable to me, but can you share a reproduction that demonstrates the issue?

@forivall
Copy link
Copy Markdown
Author

forivall commented Apr 17, 2026

Here's my reproduction repository: https://github.com/forivall/gql-config-repro-workspace/tree/89204bd11c78655db950237db8264e205f624d90

setup: npm install -g graphql-language-service-cli and helix editor. helix has graphql-lsp support out of the box.

to reproduce, i first watch the helix logs via tail -f ~/.cache/helix/*.log | rg --max-columns 10000 graphql-language-service

and then run hx -vvv test-files/1.ts

Among the log lines, i will see

2026-04-17T09:52:55.949 helix_lsp::transport [INFO] graphql-language-service <- {"jsonrpc":"2.0","method":"window/logMessage","params":{"type":1,"message":"TypeError: Cannot read properties of undefined (reading 'length')"}}

and the error will not appear.

The expected behaviour, which is how it works when i apply my fix, is to show this error:

ScreenShot 2026-04-17 at 9 55 18 AM

(the files for the above repo were adapted from the tests for graphql-codegen)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could you improve type for settings?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done.

this._settings = { ...settings, ...vscodeSettings };
const rootDir = this._settings?.load?.rootDir.length
const rootDir = this._settings?.load?.rootDir?.length
? this._settings?.load?.rootDir
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
? this._settings?.load?.rootDir
? this._settings.load.rootDir

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants