graphql-parser icon indicating copy to clipboard operation
graphql-parser copied to clipboard

Add SchemaVisitor and QueryVisitor traits and functions

Open davidpdrsn opened this issue 5 years ago • 8 comments

Fixes https://github.com/graphql-rust/graphql-parser/issues/25

Implementation is based upon the description here https://github.com/rust-unofficial/patterns/blob/master/patterns/visitor.md

davidpdrsn avatar Jul 21 '19 21:07 davidpdrsn

It looks good to me on the whole! It's unsollicited, but if nobody else has the time, I could do a more in-depth review.

tomhoule avatar Sep 18 '19 21:09 tomhoule

@tomhoule that would be great 😊

davidpdrsn avatar Sep 20 '19 07:09 davidpdrsn

@tailhook will leave it to you to look at and land.

LegNeato avatar Mar 14 '20 16:03 LegNeato

It looks like there are multiple mistakes with visiting recursively.

Or was non-recursive visitor intentional?

tailhook avatar Apr 01 '20 10:04 tailhook

Hi y'all, I'm gonna try another experimental flavor of an implementation for this, following the graphql-js interface. Here is my reasoning for this:

  1. enter would let you manipulate a node of the AST tree before entering its subtree
  2. leave would let you manipulate a node of the AST tree after its subtree has been visited
  3. It would pave the way for schema-aware visitors using a TypeInfo struct. Here's some example usage.

The biggest use-case of a system like this in my case is doing custom validation of a query based on a schema, this API lets you do that.

vladinator1000 avatar Jul 06 '21 09:07 vladinator1000

We implemented a visitor (with type info) for both schema and queries in https://github.com/dotansimha/graphql-tools-rs , also a (almost) spec-compliant validation phase.

Thanks for this PR, it was a great inspiration!

dotansimha avatar Jan 13 '22 07:01 dotansimha

@dotansimha why did you post this in 3 different places? 😅

davidpdrsn avatar Jan 13 '22 08:01 davidpdrsn

@dotansimha why did you post this in 3 different places? 😅

I saw these PRs are duplicated anyway, so I hope it would help someone :) I'll delete if that's a problem ;) didn't plan to annoy anyone with notifications :x

dotansimha avatar Jan 13 '22 08:01 dotansimha

I'll close this for now. Its been a long time and I no longer have bandwidth to follow this through.

davidpdrsn avatar Mar 20 '23 20:03 davidpdrsn