Support batching again

[jaydenseric]

Support for batching has been temporarily dropped in the rush to support the final Apollo Client v2 API (see #33 (comment)).

I am waiting on apollo-link-batch-http to be updated (it is still using apollo-fetch) so I can refer to it when writing our new batching terminating link.

[Eraldo]

@jaydenseric I don't get how to use apollo-fetch-upload with apollo-link-batch-http. It might be because I am using apollo-angular and I don't seem to get the import working.
Is there anything that I can do to clarify?

[jaydenseric]

@Eraldo My new advice is to wait until Apollo v2 has proper support for batching. apollo-fetch-upload uses v1 of the GraphQL multipart request spec, which is incompatible with the latest spec for apollo-upload-server.

You could downgrade everything to use the old spec, but really you don't want to since the latest versions require totally different resolver code and work so much better that the old, now unsupported versions.

[jaydenseric]

Also @Eraldo keep an eye on #44 since you are using Angular.

[Eraldo]

Great, thank you @jaydenseric for your explanation.
Yeah.. I could not resist upgrading anyway. :D

So, I guess I will remove batching and hope it will be re-supported soon. ;)

[notadamking]

Any news on batching?

[jaydenseric]

@adamjking3 yes! @evans has been busy adding batching support to apollo-link-http and deprecating apollo-link-batch-http.

After a lot of discussions it has been decided to add upload support to the official apollo-link-http package as an opt-in feature. I will prioritise a PR for that before I consider if adding batching support here is worth the effort, or if we are better off simply deprecating apollo-upload-client to support the official packages.

There are no plans to deprecate apollo-upload-server.

[glennreyes]

@jaydenseric Sounds exciting! So from your meeting notes apollo-upload-server will be moved to the official docs as well?

[jaydenseric]

It will be documented along with new features in apollo-link-http. We may or may not move the apollo-upload-server repo into the Apollo org; we'll see how things pan out.

[notadamking]

@jaydenseric Do you have any idea as to when apollo-upload-client and/or batching will be released into an upcoming version of apollo-link-http?

[jaydenseric]

Once apollographql/apollo-link#364 is merged I will create a new PR. Because that prior PR has big changes and involves API design decisions it is not being rushed; I'm not sure when it will be out. Hopefully soon. My PR might take a few days to put together? There needs to be Apollo documentation updates etc. so a release will depend on the wider Apollo team, which is super busy at the moment. Realistically it might take from a few days, to a few weeks from when apollographql/apollo-link#364 is merged. If it is easy enough I will add batching support here in the meantime.

[jaydenseric]

An update… The Apollo PR will likely not be merged. A new PR could be weeks out, and would prioritise abstracting the fetching internals for reuse between packages over batching.

One reason batching is not trivial to implement with Apollo links is that the fetch URI and options can be changed by links per operation context. So hypothetically, if there were 10 requests within a 10ms batching cycle, 5 might be headed to /graphql, and the other 5 might be headed to https://api.foo.bar/graphql with totally different credentials for fetch options. To batch properly we need to maintain a store for each batching cycle that tracks collections of batches under unique fetch URI and options signatures. Just determining the fetch options as it is now is pretty complicated.

If I were to implement something like this here, there is a risk that when there is an official API for it in a few weeks I'll have to junk what we have here and potentially ship a breaking change. The energy would probably be better spent helping Apollo work out a generic solution.

Generally speaking, interest in batching is waning as the benefits don't seem to be as great as expected. There are performance tradeoffs:

1. The bundle is a little more complex/bigger to contain the batching logic.
2. There is a 10ms (configurable) wait time for each batch that gets sent off. Instead of sending a single query off instantly, it gets delayed by 10ms waiting for more even if none come along.
3. Small, quick query responses are held hostage by the processing of larger, slower queries in the batch.

The tradeoffs are circumstantial, so I'm sure there are situations where batching might be valuable.

As a side note, I'm predicting complicated edge cases when processing batched mutations with upload streams on the server. Ideally we could get to a place where we can comfortably abort uploads in a mutation resolver if there is a validation issue on one of the arguments. But what if the multipart batched request contains other mutations with files that are fine? Aborting uploads for one mutation would abort the request for all. As the resolver is ignorant if the request is batched or not, it has to assume it is never safe to abort the request. So another tradeoff to batching might be sacrificing the aborting uploads early in resolvers if there are validation issues.

[mxstbr]

@jaydenseric that all makes sense, maybe we shouldn't be using batching after all 😊 Thank you for the update!

[PowerKiKi]

Our app performance is severely degraded when batching is not available. Especially when loading a form where there are several select boxes each trying to load their choices.

I think batching still has a lot of value when combined with a short wait time and a page doing seemingly unrelated things. So from my point of view, I wouldn't say that interest in batching is waning, on the contrary, it is a huge gain for a negligible cost.

[Zn4rK]

I've been following this thread for a while now, and I've disabled batching because I need file uploads via mutations.

Since my project is not live yet, I can get by, but I have 5 initial calls (working on reducing these) to my GraphQL backend and with batching disabled I get the overhead of the transport layer for each of these calls - instead of just once.

I don't need batching when uploading files - my users can wait for the requests to finish before continuing. That's not an issue for me.

The issue I'm having right now is that I can't combine BatchHttpLink with apollo-upload-client.

[PowerKiKi]

@Zn4rK this is very exactly our situation.

I understand it will take a bit more time to solve things properly and that's fine. I am willing to wait and help when I'll have time. But i just wanted to make my voice heard, that the need for batching should not be underestimated.

[Zn4rK]

This might be a bit off topic for the batching, but I figured that my solution might help a few others in this thread.

I'm sorry for my tone in my previous reply - reading it back a few weeks later, it feels a bit rude.
It's actually pretty easy to use apollo-upload-client and BatchHttpLink in conjunction with each other thanks to the ability to split links with ApolloLink.split.

Something like this would allow the use case I described above.

const isObject = node => typeof node === 'object' && node !== null;

// rough first draft, could probably be optimised in a loads of different ways.
const hasFiles = (node, found = []) => {
  Object.keys(node).forEach((key) => {
    if (!isObject(node[key]) || found.length > 0) {
      return;
    }

    if (
      (typeof File !== 'undefined' && node[key] instanceof File) ||
      (typeof Blob !== 'undefined' && node[key] instanceof Blob)
    ) {
      found.push(node[key]);
      return;
    }

    hasFiles(node[key], found);
  });

  return found.length > 0;
};

const httpLink = ApolloLink.split(
    ({ variables }) => hasFiles(variables),
    createUploadLink(options),
    new BatchHttpLink(options),
);

I experimented with the idea of using extract-files for the detection of files, but since it actually extracts the files from the variables object I just moved on to recursively checking the object for files instead. I'm sure my detection has some issues with FileLists etc - I just left them out right now.

This works great for me, since I only have uploads in mutations that users triggers.

Thanks @jaydenseric for your work on this superb implementation, and the spec!

[jaydenseric]

This is probably unblocked from being worked on, now that apollo-link-batch-http has been updated.

I can't spend time on it, but someone else feel free to take a crack at it! It has the potential to be a bit complicated and/or opinionated, so I can't promise a quick turnaround for a review and merge.

[PowerKiKi]

Thanks @Zn4rK for sharing your solution. I was able to re-activate batching for most queries in no time !

[sbrichardson]

I modified @Zn4rK solution for a slightly more performant version that is working for me.

I wasn't able to paste the code here, apologies

I included an example of the split at bottom. I needed to remove __typename for some mutations and not for Upload etc, among other needs.

• The File/Blob check only happens once, not each iteration
• The loop will exit immediately once it hits an Upload item, vs processing all keys regardless.
• Will exit if hit's an error
• I thought about adding an additional max count to protect from crashing a brower but it seems pretty solid. The extra try/catch is just another protection.

https://gist.github.com/sbrichardson/83ce25d91841bafc9365aba914007189

[PowerKiKi]

@sbrichardson thanks for sharing. The try/catch seems excessive to me. In what cases would you expect to have an error thrown when iterating JSON objects by their keys ?

[sbrichardson]

For sure @PowerKiKi, I think it can be cleaned up some, just overly cautious, since I hadn't tested it much when I shared, and while loops crash a browser so quickly.

Side note --

@Zn4rK I think taking advantage of context to control this process when needed is much more performant without needing to iterate over all the variables on every request. This method is especially useful since I only have a few Upload mutations, compared to many normal mutations/queries.

I've confirmed this works well. Example:

/**
 * Apollo Terminating link
 * The split will skip Batching if an
 * operation's context contains hasUpload: true.
 */
const httpLink = ApolloLink.split(
  operation => operation.getContext().hasUpload,
  createUploadLink(OPTS),
  new BatchHttpLink(OPTS)
)

/**
 * Then within a mutation component, specify the context
 * with hasUpload to activate the Upload link.
 */
const { data } = await uploadMutation({
  variables: { file },
  context: {
    hasUpload: true, // activate Upload link
  }
})

[karensg]

Heej @sbrichardson,

A much cleaner solution indeed. I borrowed your idea for the context variable and published an article how to do batching/uploading with Apollo & Rails:
https://medium.com/@karensgrigorjancs/effortless-file-uploads-with-activestorage-apollo-and-react-7a50929c40ca

Might be interesting for people in this issue.
K

[digeomel]

I'm not sure if this is relevant or off-topic, but I managed to get batched upload working with Apollo Angular and the apollo-upload-client, without the apollo-link-batch-http module, because our GraphQL server (Java-based) doesn't support batching anyway.
So what I did is I just iterate over the multiple files and build a mutation with multiple calls to upload each file, something like this:

mutation (file1: Upload!, file2: Upload!) {
    upload1: uploadOneFile(file: file1) {
        id
        fileName
    }
    upload2: uploadOneFile(file: file2) {
        id
        fileName
    }
}

This is still one mutation, and one http call. Maybe this will help someone.

[MakhBeth]

thankyou all for the split solution

Worked like a charm

I only had to update the mutation hooks

const [upload, { data, loading, error }] = useMutation(
    mutation,
    {
      context: { hasUpload: true },
    }
  )

[McPo]

Cheers @sbrichardson for the fix

Theres a potential modification which gets rid of setting the context (Although it might still be better to explicitly set the context)

/*
  Upload and batch link, are both terminating links
  Which do not support each other.
  We therefore only use the upload link when one of the variables are of type Upload
  Otherwise we use the batch link
  Alternatively we could have accomplished this by placing a boolean variable in the upload context instead
*/
const uploadAndBatchHTTPLink = opts => ApolloLink.split(
  operation => Boolean(operation.query.definitions.find(d => d.variableDefinitions.find(vd => vd.type.name?.value === 'Upload'))),
  createUploadLink(opts),
  new BatchHttpLink(opts)
);

It looks for variable definitions of type Upload, and if it finds any, it uses the upload link instead the batch link.

Appears to be working for me so far.

UPDATE

The above only works if the file is required. it fails to work if file is allowed to be nullable. Although vd.type.name?.value === 'Upload' seems to work in both cases.

[jaydenseric]

Note that extract-files (already an apollo-upload-client dependency) since the past few versions doesn't mutate the input, so you could detect files in variables like this:

const { extractFiles } = require('extract-files');

const { files } = extractFiles(variables);
const hasFiles = !!files.size

[McPo]

@jaydenseric provides a much better solution

import { extractFiles } from 'extract-files';
const uploadAndBatchHTTPLink = opts => ApolloLink.split(
  operation => extractFiles(operation).files.size > 0,
  createUploadLink(opts),
  new BatchHttpLink(opts)
);

It also has the advantage that if your setting a file to None, it can still be batched.

@jaydenseric would you consider re-exporting extractFiles, or exporting a helper function, for this purpose?

It seems like this would be a common issue and even though "extract-files" is used by "apollo-upload-client", the correct thing would be to still add it to your "package.json" as its called within the application code. Would be nicer if we didn't have to do that.

[jaydenseric]

I don’t plan to implement support for batching at this time, please refer to the earlier discussion in this issue:

• Implementing it would be a fair bit of work. Over 2 years has passed since I invited the community to submit a PR, and so far no one has stepped up to the plate.
• It would increase the maintenance burden of this project.
• Please don’t mistake closing this issue for laziness; when I believe something has technical merit I move mountains to make it happen. A long time ago I changed my mind that batching was a good idea, and nothing dramatic has caused me to change my mind back again in the years since. I’ve created and worked on numerous apps and APIs at commercial scale without it, no problems.
• If you’re a fan of batching, you can get it to conditionally work for non file upload operations with a little manual effort to compose links.