-
-
Notifications
You must be signed in to change notification settings - Fork 551
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
feat(graphql_semantic): build semantic model from AST #3378
feat(graphql_semantic): build semantic model from AST #3378
Conversation
7a69a60
to
3c82537
Compare
CodSpeed Performance ReportMerging #3378 will improve performances by 6.45%Comparing Summary
Benchmarks breakdown
|
fea9d32
to
d0b9435
Compare
Are you able to separate the changes you made to the parser from the actual model changes? Or are they connected? It doesn't seem they are by reading the description of the PR |
Ah sure |
9875121
to
1523f0a
Compare
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.
This is great!
enum BindingName { | ||
Type(TokenText), | ||
Value(TokenText), | ||
} |
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.
This distinction was created for TypeScript, is it true for GraphQL too?
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.
Yeah, consider the following example:
query Query {}
type Query
schema {
query: Query
}
Here Query
is used both as the name of the query, and the type of query in the schema
/// range of the name | ||
range: TextRange, | ||
/// If this is a variable binding, it will be defined in an operation scope | ||
scope_id: Option<usize>, |
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.
We recently refactored the JS semantic model to own u32, you should consider it
[package] | ||
authors.workspace = true | ||
categories.workspace = true | ||
description = "<DESCRIPTION>" |
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.
Update the description
1523f0a
to
dfd03cd
Compare
Summary
Depends on #3427
Build a Semantic model for GraphQL from AST.
Unlike in JS, in GraphQL, a variable can be referenced in a fragment, which in turn can be referenced by multiple operations, and then defined in those operations. This results in a variable being potentially defined in multiple locations.
In this case, the $start variable is defined in both the
HeroWithH
andHeroWithO
variable definitions, and referenced at(startWith: $start)
inHeroDetails
fragment.To address this, each operation and fragment will be associated with a scope. The scope will keep track of every referenced fragment, allowing it to track every referenced variable, either directly referenced or indirectly through a referenced fragment. Then, it will try to bind every referenced variable to its declaration inside an operation definition.
Test Plan
All the calls to retrieve
binding
andreference
should correctly return the respective bindings and references of the AST node.