apollographql / persistgraphql

A build tool for GraphQL projects.
MIT License
425 stars 57 forks source link

Align persistgraphql output file format with proposed apollo-codegen operation ID mapping file #34

Open kompfner opened 7 years ago

kompfner commented 7 years ago

In a recent PR to apollo-codegen, a mechanism for generating unique operation IDs was introduced. In addition, it introduced a generated output file that maps between these IDs and the corresponding operation text, for use in registering operations with a server for persistence and whitelisting. The rationale for not simply using persistgraphql is contained in the PR description.

This proposed output file differs from persistgraphql's output in a few important ways:

  1. It uses operation hashes as the IDs, rather than an incrementing counter.
  2. It uses the ID as the mapping’s key, rather than its value.
  3. It maps to a JSON object that itself contains the operation source (text) and its name.

As pointed out by @martijnwalraven, there are good reasons to align the approaches taken by apollo-codegen and persistgraphql:

One area in particular where I think standardization would be helpful is in the generated mapping file. Ideally, these mapping files would be supported by different servers and tools in the same way the extracted_queries.json file that persistgraphql generates is currently supported by Scaphold for example.

So I'm not saying we should stick to what persistgraphql currently does, but I'd at least like us to synchronize the changes (like switching to hashes or changing the mapping format).

There is an existing issue on persistgraphql that addresses difference point 1. I think switching to operation hashes is the right approach.

What I’d like to propose here is that, to address difference points 2 and 3, persistgraphql adopt the approach proposed in the apollo-codegen PR. The PR lays out an argument for this approach, which I’ll recap here:

martijnwalraven commented 7 years ago

I think these changes make a lot of sense. @Poincare, what do you think?

We may want to add a root object and put the operations under an operations key. I think it would be great if we could specify some standard metadata, like app name and version, which a server or other tool can use to tag operations.

Poincare commented 7 years ago

@martijnwalraven @kompfner This makes sense. I haven't yet delved deeply into the apollo-codegen PR but the core idea of using the operation hashes seems great. Will look into this in more detail shortly.