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

refactor(graphqlsp): Add getDeclarationOfIdentifier helper #351

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

kitten
Copy link
Member

@kitten kitten commented Aug 22, 2024

This mirrors the getDefinitionAtPosition method in TypeScript's language service — specifically the GoToDefinition component — to create a getDeclarationOfIdentifier helper.

Since we're only looking up definitions of identifiers, we can actually cut out a lot of special cases and end up with a relatively lean implementation of looking up declarations for identifiers. Specifically, if we were to switch over to an implementation like this, it would cut out a lot of extra wrappers, namely positional values that require is to look up file positions. It also saves us some look up work of cases that are impossible, since we're always jumping to declarations (fewer AST cases to account for)

Additionally, the returned AST node is a lot more limited in the locations it can be in, since it's guaranteed to only ever be a declaration.
To further help with this, a few modifications have been added to prevent us from jumping to type declarations, ambient declarations, and import/export/module specifiers (Although that's an artificial limitation)

The helpers also include:

  • types to clarify which nodes can be reached
  • a utility to return the actual values of declarations
  • a resolver that gets the value of a declaration and keeps resolving if it's an identifier again

Copy link

changeset-bot bot commented Aug 22, 2024

⚠️ No Changeset found

Latest commit: 5b0de48

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@kitten kitten force-pushed the refactor/pure-goto-definition branch 3 times, most recently from 85fea55 to 8841f43 Compare August 22, 2024 11:12
@kitten kitten force-pushed the refactor/pure-goto-definition branch from 8841f43 to 99e5ba5 Compare August 22, 2024 12:28
@kitten kitten force-pushed the refactor/pure-goto-definition branch from b249940 to 5b0de48 Compare August 22, 2024 13:01
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.

1 participant