-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add Node Predicates #1459
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
Add Node Predicates #1459
Conversation
818e878
to
6b0ec5e
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.
I think you have just this one error.
I do think we want these utilities, so I'm happy if we add them. And I don't see any issues with how you've named them: it's pretty straightforward and easy to guess correctly once you know one of them exists.
import type { ASTNode } from './ast'; | ||
import { Kind } from './kinds'; | ||
|
||
export function isDefinitionNode(node: ASTNode): boolean %checks { |
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.
I am a big fan of these! We hit them a lot, and it's nice to be able to filter through definitions to just get Executable
or TypeSystem
definitions!
definition.kind !== Kind.OPERATION_DEFINITION && | ||
definition.kind !== Kind.FRAGMENT_DEFINITION | ||
) { | ||
if (!isExecutableDefinitionNode(node)) { |
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.
The flow error here is because you're asking if the Document is an ExecutableDefinitionNode rather than asking if the definition is.
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.
@mjmahone Fixed. Thanks.
6b0ec5e
to
234d8e5
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.
Looks great! Thanks for pulling these out.
For SDL validation rules I need to distinguish type definition node from type extension nodes.
+ I saw same in couple other projects, e.g.
relay
:https://github.com/facebook/relay/blob/04aac482538b83548c25e878205ecc1a581bcb25/packages/graphql-compiler/core/GraphQLSchemaUtils.js#L198-L203