-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(graphql-language-service): fix off by one on hover #3882
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: b7e0a66 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 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 |
Seems like there is a race condition in the tests. Am unable to reproduce test failure locally. Maybe replace this graphiql/packages/graphql-language-service-server/src/MessageProcessor.ts Lines 142 to 144 in b495159
with this? try {
void mkdirSync(this._tmpDirBase);
} catch (err) {
if (err instanceof Error && 'code' in err && err.code === 'EEXIST') {
return;
}
throw err;
} |
@@ -176,14 +176,15 @@ export function getContextAtPosition( | |||
schema: GraphQLSchema, | |||
contextToken?: ContextToken, | |||
options?: { mode?: GraphQLDocumentMode; uri?: string }, | |||
offset = 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to explain this change: getContextAtPosition
is used in 2 places:
In getHoverInformation
, the offset needs to be changed to 0 to fix the off-by-one problem on hover.
const context = getContextAtPosition(queryText, cursor, schema, contextToken); |
In getAutocompleteSuggestions
, the offset needs to be kept at 1 as the current behavior is correct.
graphiql/packages/graphql-language-service/src/interface/getAutocompleteSuggestions.ts
Lines 145 to 152 in b7e0a66
const context = getContextAtPosition( | |
queryText, | |
cursor, | |
schema, | |
contextToken, | |
options, | |
1, | |
); |
Hey @acao @dimaMachina, any chance one of you could have a look? |
Hey, thanks for making this! This PR fixes a small off-by-one issue when hovering.
Details
I am using graphql-language-service-cli in Neovim. The issue is illustrated below:Hover, no information (incorrect, should show info)




Hover, shows information (correct)
Hover, shows information (incorrect, should not show info)
Hover, no information (correct)