[docs] Update cache-configuration keyFields docs

[smyrick]

The docs were missing an explanation of how the keyFields can use false or a function

I would also appreciate a review from the team as I wrote this up from TS types

[changeset-bot]

⚠️ No Changeset found

Latest commit: 0c33efd

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[netlify]

✅ Deploy Preview for apollo-client-docs ready!

Name	Link
🔨 Latest commit	0c33efd
🔍 Latest deploy log	https://app.netlify.com/sites/apollo-client-docs/deploys/66305216b407320008bc124a
😎 Deploy Preview	https://deploy-preview-11778--apollo-client-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[jerelmiller]

Really appreciate the contribution here! Having the keyFields function example is super helpful and not sure why we didn't have it before.

There are a few inaccuracies though that I'd love to correct before merging. Mind making those changes? I'd be happy to get this in afterwards.

[jerelmiller]

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.

[adamesque]

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?

[jerelmiller]

@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,
	},
	],
	},
	});
	});

[adamesque]

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

[jerelmiller]

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.

[jerelmiller]

@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.

[adamesque]

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.

[jerelmiller]

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

[phryneas]

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.

[smyrick]

@jerelmiller @adamesque I made some doc updates, but feel free to add inline suggestions or more context as needed

[adamesque]

Based on this comment, I believe this should be context.readField("location") or context.storeObject.location and we should steer folks away from using the first arg altogether since it represents the raw server response which includes aliased fields and won't follow refs (aka instead of refs to other nested cache objects you'll only get the fields selected).

[adamesque]

I see you mention this in a later bullet point, but my worry is that folks will read the code example and not the following notes and be surprised.