Skip to content

Commit

Permalink
Merge pull request #9339 from julienfouilhe/readonly-keyFields
Browse files Browse the repository at this point in the history
Store `keyFields` and `keyArgs` internally as `ReadonlyArray`s, simplifying
TypeScript typing those fields elsewhere.
  • Loading branch information
benjamn authored Feb 10, 2022
2 parents 43883b3 + a772dea commit 4e147b5
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 25 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
6 changes: 4 additions & 2 deletions src/cache/inmemory/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -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)
10 changes: 5 additions & 5 deletions src/cache/inmemory/key-extractor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
isNonNullObject,
} from "../../utilities";

import { hasOwn } from "./helpers";
import { hasOwn, isArray } from "./helpers";
import {
KeySpecifier,
KeyFieldsFunction,
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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));
Expand All @@ -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(
Expand Down
3 changes: 2 additions & 1 deletion src/cache/inmemory/object-canon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
}
Expand Down
13 changes: 7 additions & 6 deletions src/cache/inmemory/policies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
selectionSetMatchesResult,
TypeOrFieldNameRegExp,
defaultDataIdFromObject,
isArray,
} from './helpers';
import { cacheSlot } from './reactiveVars';
import { InMemoryCache } from './inMemoryCache';
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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");
}

Expand Down
8 changes: 4 additions & 4 deletions src/cache/inmemory/readFromStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -67,7 +67,7 @@ type ExecSelectionSetOptions = {

type ExecSubSelectedArrayOptions = {
field: FieldNode;
array: any[];
array: readonly any[];
enclosingRef: Reference;
context: ReadContext;
};
Expand Down Expand Up @@ -374,7 +374,7 @@ export class StoreReader {
});
}

} else if (Array.isArray(fieldValue)) {
} else if (isArray(fieldValue)) {
fieldValue = handleMissing(this.executeSubSelectedArray({
field: selection,
array: fieldValue,
Expand Down Expand Up @@ -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,
Expand Down
14 changes: 7 additions & 7 deletions src/cache/inmemory/writeToStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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))
};
Expand Down Expand Up @@ -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;
});
Expand Down Expand Up @@ -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" &&
Expand Down

0 comments on commit 4e147b5

Please sign in to comment.