Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[docs] Update cache-configuration keyFields docs #11778

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

[docs] Update cache-configuration keyFields docs #11778

Changes from 2 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
4499256
[docs] Update cache-configuration keyFields docs
smyrick Apr 10, 2024
cbb325a
Apply suggestions from code review
smyrick Apr 11, 2024
9b57a7a
Update keyFields with false
smyrick Apr 13, 2024
103f8c6
Update cache-configuration.mdx
smyrick Apr 13, 2024
2e28818
Update cache-configuration.mdx
smyrick Apr 13, 2024
523c4fb
Update cache-configuration.mdx
smyrick Apr 17, 2024
24a611c
Merge branch 'main' into patch-1
smyrick Apr 22, 2024
d0516a8
Update cache-configuration.mdx
smyrick Apr 22, 2024
396a27b
Update cache-configuration.mdx
smyrick Apr 22, 2024
7483ab7
Update cache-configuration.mdx
smyrick Apr 22, 2024
0c33efd
Update cache-configuration.mdx
smyrick Apr 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions docs/source/caching/cache-configuration.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,10 +176,24 @@ const cache = new InMemoryCache({
keyFields: ["title", "author", ["name"]],
},
AllProducts: {
// Singleton types that have no identifying field can use an empty
// array for their keyFields.
keyFields: [],
// Singleton types that have no identifying field.
keyFields: false,
smyrick marked this conversation as resolved.
Show resolved Hide resolved
},
Store: {
// If a type requires logic to choose the key fields, we can specify that in a function
keyFields: (store, context) => {
if (store.location == "United States") {
smyrick marked this conversation as resolved.
Show resolved Hide resolved
// We can return the array of field specifiers
return ["state", "storeNumber"];
} else if (store.location) {
// Or we can return another function to calculate the cache key as a string
return ((obj) => "prefix:" + obj.location + obj.storeNumber);
smyrick marked this conversation as resolved.
Show resolved Hide resolved
} else {
// Or we can indicate that this type is a Singleton
return false;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a keyFields function, returning false would indicate that this particular instance of a type shouldn't be normalized and instead embedded in the parent, correct?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adamesque thats correct! Here is a test that demonstrates returning false from a keyFields function. You can see the cache is non-normalized:

apollo-client/src/cache/inmemory/__tests__/policies.ts

Lines 5543 to 5622 in 8bc7d4d

it(`allows keyFields and keyArgs functions to return false`, function () {
const cache = new InMemoryCache({
typePolicies: {
Person: {
keyFields() {
return false;
},
fields: {
height: {
keyArgs() {
return false;
},
merge(_, height, { args }) {
if (args) {
if (args.units === "feet") {
return height;
}
if (args.units === "meters") {
// Convert to feet so we don't have to remember the units.
return height * 3.28084;
}
}
throw new Error("unexpected units: " + args);
},
},
},
},
},
});
const query = gql`
query GetUser($units: string) {
people {
id
height(units: $units)
}
}
`;
cache.writeQuery({
query,
variables: {
units: "meters",
},
data: {
people: [
{
__typename: "Person",
id: 12345,
height: 1.75,
},
{
__typename: "Person",
id: 23456,
height: 2,
},
],
},
});
expect(cache.extract()).toEqual({
ROOT_QUERY: {
__typename: "Query",
// An array of non-normalized objects, not Reference objects.
people: [
{
__typename: "Person",
// No serialized units argument, just "height".
height: 5.74147,
id: 12345,
},
{
__typename: "Person",
height: 6.56168,
id: 23456,
},
],
},
});
});

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Million-dollar question — is that true for any falsy value? It seems like it based on this or this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think thats accurate, but I'd like to verify myself before giving a definitive yes. Intuitively it would make sense that if we don't get an identifier for a particular store object that we'd consider it non-normalized, but again, let me check to make sure thats accurate.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adamesque just tried this out and I can confirm it seems that any falsey value will be treated as false and won't normalize the record.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks! I don't know exactly why ReturnType<IdGetter> is important to include in the return type union but it would definitely be clearer if it was just KeySpecifier | false.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree! I'll see if we can simplify that type.

Copy link
Member

@phryneas phryneas Apr 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ReturnType<IdGetter> does add undefined (which is unfortunate because we probably want people to be more explicit with false) and string, which is important - the keyFn function can not only return an array of field names, but also directly an identifier (although I'd advise against using that in many situations and leave the field-reading to the InMemoryCache instead).

I probably wouldn't change the type around to remove undefined here, since probably a lot of people just skip the return call in some conditions and that would break a lot of code bases for no very good reason.

}
}
}
},
});
```
Expand All @@ -194,7 +208,8 @@ This example shows a variety of `typePolicies` with different `keyFields`:
```
Book:{"title":"Fahrenheit 451","author":{"name":"Ray Bradbury"}}
```
* The `AllProducts` type illustrates a special strategy for a **singleton** type. If the cache will only ever contain one `AllProducts` object and that object has _no_ identifying fields, you can provide an empty array for its `keyFields`.
* The `AllProducts` type illustrates a special strategy for a **singleton** type. If the cache will only ever contain one `AllProducts` object and that object has _no_ identifying fields, you can provide an empty array or `false` for its `keyFields`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's leave this the same as it was before. Perhaps another bullet point stating that keyFields: false disables normalization? It doesn't have anything to do with singletons though.

* The `Store` type provides an example of using custom logic from the given object and runtime context to calculate what fields should be used for the key field selection.

If an object has multiple `keyFields`, the cache ID always lists those fields in the same order to ensure uniqueness.

Expand Down
Loading