Add support for inherit key on cacheControl.

[lunks]

This adds support for skipping using defaultMaxAge on types if inherited is
set to true. This allows to more easily cache a whole node if you do not
want or cannot set an specific maxAge.

TODO:

• Update CHANGELOG.md with your change (include reference to issue & this PR)
• Make sure all of the significant new logic is covered by tests
• Rebase your changes on master so that they can be merged easily
• Make sure all tests and linter rules pass

[apollo-cla]

@lunks: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

[evans]

@lunks Thank you for the PR! Can you elaborate on the situations when you're not able to set a specific maxAge? There is a way to set it dynamically using the cacheControl object on the info object. I think documenting it's usage in the README would help clarify when inherit would be necessary.

@martijnwalraven definitely has some thoughts on this. We'd love to hear them.

#1295 is also relevant

[zionts]

@lunks I wonder if it would provide more utility for inherit to be recursive, such that subfields will also inherit their cacheability as well from grandparents, etc. Basically, should inherit itself be inherited?

[martijnwalraven]

If I understand correctly what you're proposing, I don't think that's a good idea, because it would mark an entire tree as cacheable. And I think objects should always have to opt-in to caching (even if that's through inherit).

[martijnwalraven]

For a bit of background on why I think inheritance should be opt-in, see here:
https://github.com/apollographql/apollo-cache-control-js/issues/14#issuecomment-37315847.6

[lunks]

@lunks Thank you for the PR! Can you elaborate on the situations when you're not able to set a specific maxAge? There is a way to set it dynamically using the cacheControl object on the info object. I think documenting it's usage in the README would help clarify when inherit would be necessary.

Sure, but first, thanks for the great project! So, my issue is: I currently have is a very slow list of images I'd like to cache. Unfortunately on other parts of the system, I need other images to not be cached because they might change a lot. So I'm left with either having a short maxAge for a type Image and thus defeating the purpose of caching, or caching for a long maxAge but having issues with Images that shouldn't cache for that long. Skipping setting maxAge of 0 for images makes the engine cache the whole query like I want for a long time, while not messing with the Image type maxAge.

I'm not sure how cacheControl in info would help, but let me know if I'm missing something.

Lastly, I'll write the docs after we have agreed on the implementation if that's ok.

@lunks I wonder if it would provide more utility for inherit to be recursive, such that subfields will also inherit their cacheability as well from grandparents, etc. Basically, should inherit itself be inherited?

It sure would. Ideally, I could do something like @cacheControl(maxAge: 30, wholeQuery: true) or something similar which would just let me set it once, but I decided to take a shot with what you initially suggested. I'm open to any as they all would solve my problem. Even a config skipping defaultMaxAge being set would work for me.

[martijnwalraven]

So, my issue is: I currently have is a very slow list of images I'd like to cache. Unfortunately on other parts of the system, I need other images to not be cached because they might change a lot. So I'm left with either having a short maxAge for a type Image and thus defeating the purpose of caching, or caching for a long maxAge but having issues with Images that shouldn't cache for that long. Skipping setting maxAge of 0 for images makes the engine cache the whole query like I want for a long time, while not messing with the Image type maxAge.

You may already know this, but @cacheControl on fields overrides hints set on types. So setting slowListOfImages: [Image] @cacheControl(maxAge: 3600) would apply that max age to the list but wouldn't affect the max age set on the image type or on images elsewhere.

[martijnwalraven]

In my mind, the primary use case for something like inherit is for types that should not be independently cached, like pagination nodes or edges. It's currently a pain to be forced to specify an explicit max age there, so I think it would be great to include a way to avoid that.

[martijnwalraven]

One thing I wonder about is whether we want all cache hints to be inherited through inherit: true or make it an explicit decision to inherit specific hints through something like maxAge: "inherit".

For the former, I'm not sure what the semantics should be of combining inherit with other hints, like in @cacheControl(inherit: true, maxAge: 30). Would that only inherit the scope and override max age?

The problem with maxAge: "inherit" is that you may also want to specify scope: "inherit" to make that explicit, while in most cases you'd want to inherit all hints.

[abernix]

I think we can re-open and re-use this code in the future if this takes root, but at this point, I believe it's mostly stalled at this point. I've tagged this for cache-control so we can reference it in future considerations.

[lunks]

I haven't used Apollo in a while. I'll keep the branch in case anyone wants to take over.

[chirag04]

@abernix I share the pain point as described by lunks above: #1424 (comment). Some sort of inheritance mechanism would def. be ideal.