Add implicit cache redirect

[joarwilk]

I wanted to open this issue as a PR because its sometimes easier to convey ideas that way.

This PR makes this issue #332 automatic.

When typing this message it felt a bit unnecessary since every field which would like to use local cache lookups will look exactly the same.

Based on my experience from working with apollo and relay, as well as the reactions from people in the chat, I'd say people assume that this functionality is present. Writing resolvers for your own cache is not expected behaviour.

This PR is not a good implementation, but it works and I figured I'd start here. I dont think everyone wants this, so it might be good to hide behind a flag. Also, not everyone uses the literal id as the id param.

TODO:

• If this PR is a new feature, reference an issue where a consensus about the design was reached (not necessary for small changes)
• 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
• Update CHANGELOG.md with your change
• Add your name and email to the AUTHORS file (optional)
• If this was a change that affects the external API, update the docs and post a link to the PR in the discussion

[helfer]

@joarwilk it's a neat idea, but is it actually useful in practice? How often do you make a query that requests exactly one item by id? And in those cases, is it unreasonable to force developers to write a bit more boilerplate?

[joarwilk]

@helfer I would guess most apps have some sort of listing -> details flow. For example: a blog post listing -> blog post, a list of videos -> video, list of emails -> email, the sidebar of a manpage -> manpage entry. All those cases have some data that can be reused, and they would probably benefit from this.

In the case of documentation / boilerplate vs built-in feature, I would compare it to this pr: #228. Similar to this functionality, that could have been something that would have been added by hand to each project as a documented network middleware.

[scf4]

I second this, I actually thought this would be the default behaviour.

[helfer]

@joarwilk I think the main difference is that __typename is built into GraphQL whereas in this case we have to use id as a convention. At the very least there will need to be a way to customize this. Maybe the best start would be to make this function one that can be provided to ApolloClient globally, but it doesn't have a default (just like dataIdFromObject).

[joarwilk]

@helfer Yeah that's a good point.

I guess dataIdFromObject would be required for this to work. Maybe that can be used somehow?

[helfer]

@joarwilk well, if you pass the function to ApolloClient at initialization, you can also use the dataIdFromObject function right there. I would opt for making it non-default at first, and when we find that everyone's needs are basically the same, we can switch it on by default.

[joarwilk]

@helfer Sounds good, I'll update the PR to make it optional

[helfer]

@joarwilk just to be clear, I think we should try an option named defaultIdLookup or something like that on Apollo Client. The argument should be optional, and when it's provided it's applied when there's no other cache redirect provided for a field.

[joarwilk]

@helfer Updated.

I realised that this requires globally unique IDs as it wont work when objects are stored in cache using the popular id + __typename convention. Bit of an inconvenience.

[helfer]

@joarwilk if you're still interested in this, can you please open a new issue or PR for discussion? Thanks! 🙂

[insidewhy]

How often do you make a query that requests exactly one item by id?

Haha I read this and was like "All the bloody time".