Skip to content
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

avoid giving diagnostics for stuff falling out of the node-range #73

Merged
merged 4 commits into from
Jun 22, 2023

Conversation

JoviDeCroock
Copy link
Member

@JoviDeCroock JoviDeCroock commented Jun 21, 2023

Partially resolves #72

This avoids us polluting the LSP with diagnostics that go outside of the AST Node Range. An ideal solution here would be to indicate these on the FragmentDefinition itself, or TypeScript providing a way to tell us about a nested error.

This PR partially resolves the problem but introduces a new one where if folks add identifiers to the start of their document that we will convey the wrong information, however this is currently also present... After experimenting 😅

Ideally during resolve we will start returning

  • the full text
  • definition --> text
  • ranges of definition --> text

This will allow us to translate and combine diagnostics effectively

@changeset-bot
Copy link

changeset-bot bot commented Jun 21, 2023

🦋 Changeset detected

Latest commit: 69c0238

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

This PR includes changesets to release 1 package
Name Type
@0no-co/graphqlsp 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

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.

Support nested-fragment resolution errors
1 participant