Fix compilation and execution for circular dependencies in input types

[boopathi]

• fix Request variables with circular dependency #81

Previously the following schema would result in compilation error -

type Query {
  test(input: Foo): String
}
input Foo {
  foo: Foo # circular dependency
}

When input types are recursive in nature, the compilation depended on generating it inline. This PR hoists it to a function and generates it only once for input object types. This way, instead of relying on inlined object access, we have a getter and a setter defined at the top of the generated code - getPath and setPath - this moves the object access (the path - input["var"]["foo"]) to runtime - via getPath and setPath - so as to pass different paths as we move down the tree.

Extras:

Also, we compute the circular dependencies at runtime and throw them as separate user errors/ For this we now have the Set of visited inputs captured at the top-level. At the time of this writing, I've not handled the false positives where - var1.foo is the same object as var2.foo - which does not form a circular dependency, but would still result in a circular dependency error. These forms of runtime values are unintended because variables are generally passed down via request and is most likely deserialized from JSON. A JSON.parse cannot result in such cases anyway.

For the common case, we disable checking for runtime circular dependencies. To enable this, you can use the new compiler option

// default: false
variablesCircularReferenceCheck: true

[ruiaraujo]

I still need to think a bit more about the approach, I would love to have some benchmarks that stress query execution with highly nested variables (like the filter test in this PR) between master and this PR.

If this is punishing for the before case, maybe we can consider detecting the circular case in the types and bail out to such an implementation while relying on inline access for the normal case.

[jgoux]

Hello, is there anything particular blocking this PR? It would unlock a lot of people if it can be merged. 🤞

[tuan231195]

@boopathi @ruiaraujo, is there any update about this PR? It would solve lots of issues for us

[mnlbox]

Any plan to finalize and merge this PR ??

[Urigo]

@boopathi @ruiaraujo What can we do in order to help this land?
Should we open another PR with a different approach?

[BabakScript]

@boopathi Thanks for this awesome project but this issue and the PR have been open for 4 years. I'm trying to figure out what is the blocker here because we would like to help to get this issue resolved.

How we can help to proceed with the solution and fix this issue?
This issue is blocking us from using GraphQL-Mesh as it uses graphQL-jit behind the scenes. We would like to connect Hasura to GraphQL-Mesh too and as we tried in a POC, we have this circular dependency issue there as well. So we are glad to contribute and help on this issue to get it resolved.

[oporkka]

@boopathi Thanks for this awesome project but this issue and the PR have been open for 4 years. I'm trying to figure out what is the blocker here because we would like to help to get this issue resolved.

How we can help to proceed with the solution and fix this issue? This issue is blocking us from using GraphQL-Mesh as it uses graphQL-jit behind the scenes. We would like to connect Hasura to GraphQL-Mesh too and as we tried in a POC, we have this circular dependency issue there as well. So we are glad to contribute and help on this issue to get it resolved.

@BabakScript I think what is pending, is the missing benchmarks that @ruiaraujo was mentioning earlier.

I would love to have some benchmarks that stress query execution with highly nested variables (like the filter test in this PR) between master and this PR.

If this is punishing for the before case, maybe we can consider detecting the circular case in the types and bail out to such an implementation while relying on inline access for the normal case.

@BabakScript would you be able to contribute the above benchmarks?

[boopathi]

Some benchmarks are available here in these PRs - #228 and #229 and it's not performant and I was trying to optimize it. I'll get back to this when I find some time.

[BabakScript]

@BabakScript would you be able to contribute the above benchmarks?

@oporkka I guess as @boopathi mentioned the benchmarks part is somehow done. We didn't contribute to this project yet but we can try to read the code and check if we can help with the optimization part.

BTW, Thanks @boopathi for the update. I guess #228 is also a promising and good approach.