Sorting of fragments in getQueryDocumentKey incorrect

[apollographql]

Sorting of fragments in getQueryDocumentKey incorrect

[fubhy]

You can see that the query sent by apollo-client has the fragments sorted in order of their occurrence while the one generated for the query map has them randomly sorted.

[Poincare]

Internally, this tool uses a graphql-js function that extracts the fragments, etc. for a particular query. It seems that it doesn't order the fragments in the same way.

I guess there are two solutions: keep the ordering consistent or make the serialization function (which is currently just graphql-js's print) robust to re-ordering of the fragments. I'm inclined to just doing the first since the second one seems like a can of worms and might require solving the first one. Thoughts?

[fubhy]

I agree.

[fubhy]

Instead of crawling all files sequentially for any gql tags we needs to just pick up queries and mutations and then resolve those with their fragments, replacing occurrences of ${Foo.fragments.barFragment}, properly. This would then be equivalent to what happens at runtime. Obviously we would then also need to eliminate duplicate fragments.

[fubhy]

I guess this is what we are looking for: apollographql/graphql-tag#31. Extracting the queries and converting them to an AST at build time and then using that AST both on the server and the client (if both are JS) or printing it for the queries.json for non-JS servers would be the 100% robust solution. That means that this project would drop all of the template literal extraction and only contain the networkInterface alterations.

[Poincare]

Hm. I think there are two orthogonal issues here, one slightly more important the other.

The first issue is that stuff like ${Foo.fragments.barFragment} is not replaced so the template literal support breaks when you use fragments since they are not ordered correctly, etc.

The second issue that the way that the fragments are currently ordered by this tool is different from what happens at runtime. I think this is a more immediate, pressing concern since it breaks both template literal extraction and external file queries in certain cases (as you've outlined above). This problem is also relatively easy to solve since it just involves not using graphql-js's utility function.

If I understand correctly, you are suggesting that this tool perform resolution of stuff like ${Foo.fragments.barFragment} at build-time. How would this work in the general case? You could, potentially, have arbitrary JS code that constructs the query.

[wyattjoh]

Also experiencing the same issue.

We have a query https://github.com/coralproject/talk/blob/2dda81f2574cc6662ca3a528324246b814558e17/client/coral-framework/graphql/queries/streamQuery.graphql that is not being sorted properly resulting in the code generated by the graphql-tag/loader (https://github.com/coralproject/talk/blob/2dda81f2574cc6662ca3a528324246b814558e17/webpack.config.js#L78) and the persistgraphql binary mismatching causing the Could not find query inside query map. error to be thrown (and unfortunately swallowed somewhere by the apolllo-client).

[Poincare]

@wyattjoh I think a good solution to this problem is to simply sort the query definitions in a well-defined way before inserting them into the query map and before comparing into the query map. The problem @fubhy outlined is somewhat different as I outlined above but this particular solution is definitely needed. Would you interested in working on a PR for this?

[fubhy]

I opened #22 which does the trick for me in my current project. This only solves the fragment definition sorting problem, not the more fundamental issue outlined above.

[Poincare]

Fixed thanks to @fubhy :)