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

Handle Fragment expansion with variables fully #107

Open
5 tasks
jlouis opened this issue Oct 30, 2017 · 1 comment
Open
5 tasks

Handle Fragment expansion with variables fully #107

jlouis opened this issue Oct 30, 2017 · 1 comment

Comments

@jlouis
Copy link
Owner

jlouis commented Oct 30, 2017

Background

Suppose we have a fragment

fragment M on Monster {
  ...
  hitPoints(above: $foo)
  ...
}

Then this fragment is valid as a spread in any query Q which defines

query Q($foo : Int) { ... }

But not in a query

query Q($foo: String) { ... }

Also, note that if we have another fragment

fragment R on Room {
    description(language: $foo)
}

Then if language : Locale you are not allowed to mix fragments M and R in the same query.

Other observations

  • This holds transitively. If M refers to a subfragment I on Iventory and that fragment refers to $bar, then M also refers to $bar.
  • These tests must happen in the type checking phase.

Implementation

The reason this has been held off for a while is that it isn't that simple to implement:

  • Introduce the notion of a type signature for fragment. This allows us to ask if a fragment "fits" when it is called by a fragment spread.
  • Inline spreads can just produce their signature and we can then check it afterwards.
  • A fragment is implemented as a function from its signature to its expansion. Thus, the function signature is verified by checking if we can "call" the fragment from the spread with a valid type.
  • When handling fragments, they can refer to other fragments. When this happens, we must run fragception and DFS the fragment world. We track already handled fragments in order to detect if we have a cycle in the fragment expansion. This runs in linear time over the fragments and is going to be reasonably fast. Also, it moves the cycle validation into the type checker where it belongs.
  • Value coercion still happens at the execution phase for variables. So this will still work as expected.
@jlouis
Copy link
Owner Author

jlouis commented Oct 31, 2017

Note: The right solution is to invert the checking:

Currently, we build a variable context and we use this to check that each variable use is valid in that context. In the new system, we "infer" a variable set and their types for each fragment, query, or mutation. Once we have a set of the variables which are referenced, the top level verification checks that the type is indeed a valid type by walking over the inferred set and checking it against the actual variable context. This allows for far simpler solution of issues #70, #71, #109 and #120 since these are now handled directly by the type checker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant