From 86a2bba4c727b76da3678612fd7e5096ba01c589 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 12 Dec 2019 12:44:41 -0500 Subject: [PATCH] Warn when clobbering non-normalized data in the cache. One consequence of #5603 is that replacing non-normalized data in the cache can result in loss of useful data. In almost every case, the right solution is to make sure the data can be normalized, or (if that isn't possible) to define a custom merge function for the replaced field, within the parent TypePolicy. It turns out we can give a very detailed warning in all such situations, so that's what this commit does. Looking at the output for our test suite, every warning is legitimate and worth fixing. I will resolve the warnings and test failures that our test suite generates in subsequent commits. TODO Update the documentation URLs after #5677 is merged. --- src/cache/inmemory/entityStore.ts | 68 ++++++++++++++++++++++++++++--- 1 file changed, 63 insertions(+), 5 deletions(-) diff --git a/src/cache/inmemory/entityStore.ts b/src/cache/inmemory/entityStore.ts index 1c88766d674..475596f2f6e 100644 --- a/src/cache/inmemory/entityStore.ts +++ b/src/cache/inmemory/entityStore.ts @@ -450,11 +450,6 @@ const storeObjectReconciler: ReconcilerFunction<[EntityStore]> = function ( return incoming; } - invariant( - !isReference(existing) || isReference(incoming), - `Store error: the application attempted to write an object with no provided id but the store already contains an id of ${existing.__ref} for this object.`, - ); - // It's worth checking deep equality here (even though blindly // returning incoming would be logically correct) because preserving // the referential identity of existing data can prevent needless @@ -462,6 +457,10 @@ const storeObjectReconciler: ReconcilerFunction<[EntityStore]> = function ( if (equal(existing, incoming)) { return existing; } + + if (process.env.NODE_ENV !== 'production') { + warnAboutDataLoss(existingObject, incomingObject, property, store); + } } // In all other cases, incoming replaces existing without any effort to @@ -470,6 +469,65 @@ const storeObjectReconciler: ReconcilerFunction<[EntityStore]> = function ( return incoming; } +function warnAboutDataLoss( + existingObject: Record, + incomingObject: Record, + property: string | number, + store: EntityStore, +) { + const existing = existingObject[property]; + const incoming = incomingObject[property]; + + if (!isReference(existing) && + !isReference(incoming) && + !Object.keys(existing).every(key => hasOwn.call(incoming, key))) { + const fieldName = fieldNameFromStoreName(String(property)); + + const parentType = + getTypenameFromStoreObject(store, existingObject) || + getTypenameFromStoreObject(store, incomingObject); + + const childTypenames: string[] = []; + // Arrays always need a custom merge function, even if their elements + // are normalized entities. + if (!Array.isArray(existing) && + !Array.isArray(incoming)) { + [existing, incoming].forEach(child => { + const typename = getTypenameFromStoreObject(store, child); + if (typeof typename === "string" && + !childTypenames.includes(typename)) { + childTypenames.push(typename); + } + }); + } + + invariant.warn( +`Cache data may be lost when replacing the ${fieldName} field of a ${parentType} object. + +To address this problem (which is not a bug in Apollo Client), ${ + childTypenames.length + ? "either ensure that objects of type " + + childTypenames.join(" and ") + " have IDs, or " + : "" +}define a custom merge function for the ${ + parentType +}.${ + fieldName +} field, so the InMemoryCache can safely merge these objects: + + existing: ${JSON.stringify(existing).slice(0, 1000)} + incoming: ${JSON.stringify(incoming).slice(0, 1000)} + +For more information about these options, please refer to the documentation: + + * Ensuring entity objects have IDs: https://deploy-preview-5677--apollo-client-docs.netlify.com/docs/react/v3.0-beta/caching/cache-configuration/#generating-unique-identifiers + + * Defining custom merge functions: https://deploy-preview-5677--apollo-client-docs.netlify.com/docs/react/v3.0-beta/caching/cache-field-behavior/#merging-non-normalized-objects +` + ); + } +} + export function supportsResultCaching(store: any): store is EntityStore { // When result caching is disabled, store.depend will be null. return !!(store instanceof EntityStore && store.group.caching);