Merge pull request #9339 from julienfouilhe/readonly-keyFields

Store `keyFields` and `keyArgs` internally as `ReadonlyArray`s, simplifying
TypeScript typing those fields elsewhere.

CHANGELOG.md

@@ -1,3 +1,8 @@
+## Apollo Client 3.5.9 (unreleased)
+
+- Interpret `keyFields: [...]` and `keyArgs: [...]` configurations in `InMemoryCache` type/field policies as `ReadonlyArray`s, since they are never mutated internally. <br/>
+  [@julienfouilhe](https://github.com/julienfouilhe) in [#9339](https://github.com/apollographql/apollo-client/pull/9339)
+
 ## Apollo Client 3.5.8 (2022-01-24)
 
 ### Bug Fixes

src/cache/inmemory/helpers.ts

@@ -88,7 +88,7 @@ export function selectionSetMatchesResult(
   variables?: Record<string, any>,
 ): boolean {
   if (isNonNullObject(result)) {
-    return Array.isArray(result)
+    return isArray(result)
       ? result.every(item => selectionSetMatchesResult(selectionSet, item, variables))
       : selectionSet.selections.every(field => {
         if (isField(field) && shouldInclude(field, variables)) {
@@ -113,9 +113,11 @@ export function storeValueIsStoreObject(
 ): value is StoreObject {
   return isNonNullObject(value) &&
     !isReference(value) &&
-    !Array.isArray(value);
+    !isArray(value);
 }
 
 export function makeProcessedFieldsMerger() {
   return new DeepMerger;
 }
+
+export const isArray = (a: any): a is any[] | readonly any[] => Array.isArray(a)

src/cache/inmemory/key-extractor.ts

@@ -7,7 +7,7 @@ import {
   isNonNullObject,
 } from "../../utilities";
 
-import { hasOwn } from "./helpers";
+import { hasOwn, isArray } from "./helpers";
 import {
   KeySpecifier,
   KeyFieldsFunction,
@@ -197,12 +197,12 @@ export function getSpecifierPaths(spec: KeySpecifier): string[][] {
     const currentPath: string[] = [];
 
     spec.forEach((s, i) => {
-      if (Array.isArray(s)) {
+      if (isArray(s)) {
         getSpecifierPaths(s).forEach(p => paths.push(currentPath.concat(p)));
         currentPath.length = 0;
       } else {
         currentPath.push(s);
-        if (!Array.isArray(spec[i + 1])) {
+        if (!isArray(spec[i + 1])) {
           paths.push(currentPath.slice(0));
           currentPath.length = 0;
         }
@@ -238,7 +238,7 @@ export function extractKeyPath(
   // possibly unknown) is the honest answer.
   extract = extract || extractKey;
   return normalize(path.reduce(function reducer(obj, key): any {
-    return Array.isArray(obj)
+    return isArray(obj)
       ? obj.map(child => reducer(child, key))
       : obj && extract!(obj, key);
   }, object));
@@ -249,7 +249,7 @@ function normalize<T>(value: T): T {
   // key fields are scalar, but just in case we get an object or an array, we
   // need to do some normalization of the order of (nested) keys.
   if (isNonNullObject(value)) {
-    if (Array.isArray(value)) {
+    if (isArray(value)) {
       return value.map(normalize) as any;
     }
     return collectSpecifierPaths(

src/cache/inmemory/object-canon.ts

@@ -6,10 +6,11 @@ import {
   canUseWeakSet,
   isNonNullObject as isObjectOrArray,
 } from "../../utilities";
+import { isArray } from "./helpers";
 
 function shallowCopy<T>(value: T): T {
   if (isObjectOrArray(value)) {
-    return Array.isArray(value)
+    return isArray(value)
       ? value.slice(0) as any as T
       : { __proto__: Object.getPrototypeOf(value), ...value };
   }

src/cache/inmemory/policies.ts

@@ -32,6 +32,7 @@ import {
   selectionSetMatchesResult,
   TypeOrFieldNameRegExp,
   defaultDataIdFromObject,
+  isArray,
 } from './helpers';
 import { cacheSlot } from './reactiveVars';
 import { InMemoryCache } from './inMemoryCache';
@@ -59,7 +60,7 @@ export type TypePolicies = {
 
 // TypeScript 3.7 will allow recursive type aliases, so this should work:
 // type KeySpecifier = (string | KeySpecifier)[]
-export type KeySpecifier = (string | KeySpecifier)[];
+export type KeySpecifier = ReadonlyArray<string | KeySpecifier>;
 
 export type KeyFieldsContext = {
   // The __typename of the incoming object, even if the __typename field was
@@ -382,7 +383,7 @@ export class Policies {
     let keyFn = policy && policy.keyFn || this.config.dataIdFromObject;
     while (keyFn) {
       const specifierOrId = keyFn(object, context);
-      if (Array.isArray(specifierOrId)) {
+      if (isArray(specifierOrId)) {
         keyFn = keyFieldsFnFromSpecifier(specifierOrId);
       } else {
         id = specifierOrId;
@@ -457,7 +458,7 @@ export class Policies {
       keyFields === false ? nullKeyFieldsFn :
       // Pass an array of strings to use those fields to compute a
       // composite ID for objects of this typename.
-      Array.isArray(keyFields) ? keyFieldsFnFromSpecifier(keyFields) :
+      isArray(keyFields) ? keyFieldsFnFromSpecifier(keyFields) :
       // Pass a function to take full control over identification.
       typeof keyFields === "function" ? keyFields :
       // Leave existing.keyFn unchanged if above cases fail.
@@ -479,7 +480,7 @@ export class Policies {
             keyArgs === false ? simpleKeyArgsFn :
             // Pass an array of strings to use named arguments to
             // compute a composite identity for the field.
-            Array.isArray(keyArgs) ? keyArgsFnFromSpecifier(keyArgs) :
+            isArray(keyArgs) ? keyArgsFnFromSpecifier(keyArgs) :
             // Pass a function to take full control over field identity.
             typeof keyArgs === "function" ? keyArgs :
             // Leave existing.keyFn unchanged if above cases fail.
@@ -729,7 +730,7 @@ export class Policies {
       const args = argsFromFieldSpecifier(fieldSpec);
       while (keyFn) {
         const specifierOrString = keyFn(args, context);
-        if (Array.isArray(specifierOrString)) {
+        if (isArray(specifierOrString)) {
           keyFn = keyArgsFnFromSpecifier(specifierOrString);
         } else {
           // If the custom keyFn returns a falsy value, fall back to
@@ -966,7 +967,7 @@ function makeMergeObjectsFunction(
   store: NormalizedCache,
 ): MergeObjectsFunction {
   return function mergeObjects(existing, incoming) {
-    if (Array.isArray(existing) || Array.isArray(incoming)) {
+    if (isArray(existing) || isArray(incoming)) {
       throw new InvariantError("Cannot automatically merge arrays");
     }
 

src/cache/inmemory/readFromStore.ts

@@ -36,7 +36,7 @@ import {
   ReadMergeModifyContext,
 } from './types';
 import { maybeDependOnExistenceOfEntity, supportsResultCaching } from './entityStore';
-import { getTypenameFromStoreObject, shouldCanonizeResults } from './helpers';
+import { getTypenameFromStoreObject, isArray, shouldCanonizeResults } from './helpers';
 import { Policies } from './policies';
 import { InMemoryCache } from './inMemoryCache';
 import { MissingFieldError, MissingTree } from '../core/types/common';
@@ -67,7 +67,7 @@ type ExecSelectionSetOptions = {
 
 type ExecSubSelectedArrayOptions = {
   field: FieldNode;
-  array: any[];
+  array: readonly any[];
   enclosingRef: Reference;
   context: ReadContext;
 };
@@ -374,7 +374,7 @@ export class StoreReader {
             });
           }
 
-        } else if (Array.isArray(fieldValue)) {
+        } else if (isArray(fieldValue)) {
           fieldValue = handleMissing(this.executeSubSelectedArray({
             field: selection,
             array: fieldValue,
@@ -462,7 +462,7 @@ export class StoreReader {
       }
 
       // This is a nested array, recurse
-      if (Array.isArray(item)) {
+      if (isArray(item)) {
         return handleMissing(this.executeSubSelectedArray({
           field,
           array: item,

src/cache/inmemory/writeToStore.ts

@@ -29,7 +29,7 @@ import {
 } from '../../utilities';
 
 import { NormalizedCache, ReadMergeModifyContext, MergeTree } from './types';
-import { makeProcessedFieldsMerger, fieldNameFromStoreName, storeValueIsStoreObject } from './helpers';
+import { makeProcessedFieldsMerger, fieldNameFromStoreName, storeValueIsStoreObject, isArray } from './helpers';
 import { StoreReader } from './readFromStore';
 import { InMemoryCache } from './inMemoryCache';
 import { EntityStore } from './entityStore';
@@ -450,7 +450,7 @@ export class StoreWriter {
       return __DEV__ ? cloneDeep(value) : value;
     }
 
-    if (Array.isArray(value)) {
+    if (isArray(value)) {
       return value.map((item, i) => {
         const value = this.processFieldValue(
           item, field, context, getChildMergeTree(mergeTree, i));
@@ -590,7 +590,7 @@ export class StoreWriter {
         // Items in the same position in different arrays are not
         // necessarily related to each other, so when incoming is an array
         // we process its elements as if there was no existing data.
-        !Array.isArray(incoming) &&
+        !isArray(incoming) &&
         // Likewise, existing must be either a Reference or a StoreObject
         // in order for its fields to be safe to merge with the fields of
         // the incoming object.
@@ -621,7 +621,7 @@ export class StoreWriter {
         from: typeof e | typeof i,
         name: string | number,
       ): StoreValue => {
-        return Array.isArray(from)
+        return isArray(from)
           ? (typeof name === "number" ? from[name] : void 0)
           : context.store.getFieldValue(from, String(name))
       };
@@ -652,7 +652,7 @@ export class StoreWriter {
 
       if (changedFields) {
         // Shallow clone i so we can add changed fields to it.
-        incoming = (Array.isArray(i) ? i.slice(0) : { ...i }) as T;
+        incoming = (isArray(i) ? i.slice(0) : { ...i }) as T;
         changedFields.forEach((value, name) => {
           (incoming as any)[name] = value;
         });
@@ -792,8 +792,8 @@ function warnAboutDataLoss(
   const childTypenames: string[] = [];
   // Arrays do not have __typename fields, and always need a custom merge
   // function, even if their elements are normalized entities.
-  if (!Array.isArray(existing) &&
-      !Array.isArray(incoming)) {
+  if (!isArray(existing) &&
+      !isArray(incoming)) {
     [existing, incoming].forEach(child => {
       const typename = store.getFieldValue(child, "__typename");
       if (typeof typename === "string" &&