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.

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

Allow @directive and $variable strings in field policy keyArgs: ["arg", "@dir", "$var"] arrays #8678

Merged
Next Next commit
Support field key policy as a more general synonym for keyArgs.
  • Loading branch information
benjamn committed Aug 25, 2021
commit ba347309ad1479420ccbf338152314f8a2990aac
2 changes: 1 addition & 1 deletion src/cache/inmemory/entityStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ export abstract class EntityStore implements NormalizedCache {
// share the same short fieldName, regardless of arguments.
const fieldName = fieldNameFromStoreName(storeFieldName);
if (fieldName !== storeFieldName &&
!this.policies.hasKeyArgs(merged.__typename, fieldName)) {
!this.policies.hasFieldKeyConfig(merged.__typename, fieldName)) {
fieldsToDirty[fieldName] = 1;
}

Expand Down
163 changes: 127 additions & 36 deletions src/cache/inmemory/policies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
canUseWeakMap,
isNonNullObject,
stringifyForDisplay,
isNonEmptyArray,
} from '../../utilities';
import {
IdGetter,
Expand Down Expand Up @@ -110,7 +111,7 @@ export type TypePolicy = {
}
};

export type KeyArgsFunction = (
export type FieldKeyFunction = (
benjamn marked this conversation as resolved.
Show resolved Hide resolved
args: Record<string, any> | null,
context: {
typename: string;
Expand All @@ -120,7 +121,7 @@ export type KeyArgsFunction = (
},
) => KeySpecifier | false | ReturnType<IdGetter>;

type KeyArgsResult = Exclude<ReturnType<KeyArgsFunction>, KeySpecifier>;
type FieldKeyResult = Exclude<ReturnType<FieldKeyFunction>, KeySpecifier>;

export type FieldPolicy<
// The internal representation used to store the field's data in the
Expand All @@ -136,7 +137,9 @@ export type FieldPolicy<
// data and options.args as input. Usually the same as TIncoming.
TReadResult = TIncoming,
> = {
keyArgs?: KeySpecifier | KeyArgsFunction | false;
/** @deprecated use `key` instead of `keyArgs` */
keyArgs?: KeySpecifier | FieldKeyFunction | false;
key?: KeySpecifier | FieldKeyFunction | false;
read?: FieldReadFunction<TExisting, TReadResult>;
merge?: FieldMergeFunction<TExisting, TIncoming> | boolean;
};
Expand Down Expand Up @@ -252,7 +255,7 @@ export const defaultDataIdFromObject = (
};

const nullKeyFieldsFn: KeyFieldsFunction = () => void 0;
const simpleKeyArgsFn: KeyArgsFunction = (_args, context) => context.fieldName;
const simpleFieldKeyFn: FieldKeyFunction = (_args, context) => context.fieldName;

// These merge functions can be selected by specifying merge:true or
// merge:false in a field policy.
Expand All @@ -271,7 +274,7 @@ export class Policies {
merge?: FieldMergeFunction<any>;
fields: {
[fieldName: string]: {
keyFn?: KeyArgsFunction;
keyFn?: FieldKeyFunction;
read?: FieldReadFunction<any>;
merge?: FieldMergeFunction<any>;
};
Expand Down Expand Up @@ -449,17 +452,28 @@ export class Policies {
if (typeof incoming === "function") {
existing.read = incoming;
} else {
const { keyArgs, read, merge } = incoming;
// The newer incoming.key configuration is synonymous with the older
// incoming.keyArgs one, but we prefer incoming.key since its naming
// better reflects the reality that field arguments are not the only
// factor contributing to the field's key/identity: directives,
// variables, and even aliases can influence the field's key.
const {
// We use key in the code below, but it defaults to incoming.keyArgs
// if incoming.key is undefined.
key = incoming.keyArgs,
read,
merge,
} = incoming;

existing.keyFn =
// Pass false to disable argument-based differentiation of
// field identities.
keyArgs === false ? simpleKeyArgsFn :
key === false ? simpleFieldKeyFn :
// Pass an array of strings to use named arguments to
// compute a composite identity for the field.
Array.isArray(keyArgs) ? keyArgsFnFromSpecifier(keyArgs) :
Array.isArray(key) ? fieldKeyFnFromSpecifier(key) :
// Pass a function to take full control over field identity.
typeof keyArgs === "function" ? keyArgs :
typeof key === "function" ? key :
// Leave existing.keyFn unchanged if above cases fail.
existing.keyFn;

Expand All @@ -476,7 +490,7 @@ export class Policies {
// responsibility for interpreting arguments in and out. This
// default assumption can always be overridden by specifying
// keyArgs explicitly in the FieldPolicy.
existing.keyFn = existing.keyFn || simpleKeyArgsFn;
existing.keyFn = existing.keyFn || simpleFieldKeyFn;
}
});
}
Expand Down Expand Up @@ -573,7 +587,7 @@ export class Policies {
fieldName: string,
createIfMissing: boolean,
): {
keyFn?: KeyArgsFunction;
keyFn?: FieldKeyFunction;
read?: FieldReadFunction<any>;
merge?: FieldMergeFunction<any>;
} | undefined {
Expand Down Expand Up @@ -686,19 +700,19 @@ export class Policies {
return false;
}

public hasKeyArgs(typename: string | undefined, fieldName: string) {
public hasFieldKeyConfig(typename: string | undefined, fieldName: string) {
const policy = this.getFieldPolicy(typename, fieldName, false);
return !!(policy && policy.keyFn);
}

public getStoreFieldName(fieldSpec: FieldSpecifier): string {
const { typename, fieldName } = fieldSpec;
const policy = this.getFieldPolicy(typename, fieldName, false);
let storeFieldName: KeyArgsResult;
let storeFieldName: FieldKeyResult;

let keyFn = policy && policy.keyFn;
if (keyFn && typename) {
const context: Parameters<KeyArgsFunction>[1] = {
const context: Parameters<FieldKeyFunction>[1] = {
typename,
fieldName,
field: fieldSpec.field || null,
Expand All @@ -708,7 +722,7 @@ export class Policies {
while (keyFn) {
const specifierOrString = keyFn(args, context);
if (Array.isArray(specifierOrString)) {
keyFn = keyArgsFnFromSpecifier(specifierOrString);
keyFn = fieldKeyFnFromSpecifier(specifierOrString);
} else {
// If the custom keyFn returns a falsy value, fall back to
// fieldName instead.
Expand Down Expand Up @@ -978,13 +992,26 @@ function makeMergeObjectsFunction(
};
}

function keyArgsFnFromSpecifier(
function fieldKeyFnFromSpecifier(
specifier: KeySpecifier,
): KeyArgsFunction {
): FieldKeyFunction {
return (args, context) => {
return args ? `${context.fieldName}:${
JSON.stringify(computeKeyObject(args, specifier, false))
}` : context.fieldName;
let key = context.fieldName;

const suffix = JSON.stringify(
computeFieldKeyObject(specifier, context.field, args, context.variables),
);

// If no arguments were passed to this field, and it didn't have any other
// field key contributions from directives or variables, hide the empty :{}
// suffix from the field key. However, a field passed no arguments can still
// end up with a non-empty :{...} suffix if its key configuration refers to
// directives or variables.
if (args || suffix !== "{}") {
key += ":" + suffix;
}

return key;
};
}

Expand All @@ -1008,7 +1035,7 @@ function keyFieldsFnFromSpecifier(
}

const keyObject = context.keyObject =
computeKeyObject(object, specifier, true, aliasMap);
computeKeyFieldsObject(specifier, object, aliasMap);

return `${context.typename}:${JSON.stringify(keyObject)}`;
};
Expand Down Expand Up @@ -1057,11 +1084,10 @@ function makeAliasMap(
return map;
}

function computeKeyObject(
response: Record<string, any>,
function computeKeyFieldsObject(
Comment on lines -1060 to +1072
Copy link
Member Author

Choose a reason for hiding this comment

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

We used to (ab)use computeKeyObject for both keyFields: [...] and keyArgs: [...], but this PR pushed me over the edge to separating them into two separate functions: computeKeyFieldsObject and computeFieldKeyObject.

specifier: KeySpecifier,
strict: boolean,
aliasMap?: AliasMap,
response: Record<string, any>,
aliasMap: AliasMap | undefined,
): Record<string, any> {
// The order of adding properties to keyObj affects its JSON serialization,
// so we are careful to build keyObj in the order of keys given in
Expand All @@ -1074,25 +1100,90 @@ function computeKeyObject(
let lastResponseKey: string | undefined;
let lastActualKey: string | undefined;

const aliases = aliasMap && aliasMap.aliases;
const subsets = aliasMap && aliasMap.subsets;

specifier.forEach(s => {
if (Array.isArray(s)) {
if (typeof lastActualKey === "string" &&
typeof lastResponseKey === "string") {
const subsets = aliasMap && aliasMap.subsets;
const subset = subsets && subsets[lastActualKey];
keyObj[lastActualKey] =
computeKeyObject(response[lastResponseKey], s, strict, subset);
keyObj[lastActualKey] = computeKeyFieldsObject(
s, // An array of fields names to extract from the previous response
// object (response[lastResponseKey]).
response[lastResponseKey],
subsets && subsets[lastActualKey],
);
}
} else {
const aliases = aliasMap && aliasMap.aliases;
const responseName = aliases && aliases[s] || s;
if (hasOwn.call(response, responseName)) {
keyObj[lastActualKey = s] = response[lastResponseKey = responseName];
} else {
invariant(!strict, `Missing field '${responseName}' while computing key fields`);
lastResponseKey = lastActualKey = void 0;
const responseKey = aliases && aliases[s] || s;
invariant(
hasOwn.call(response, responseKey),
`Missing field '${responseKey}' while computing key fields`,
);
keyObj[lastActualKey = s] = response[lastResponseKey = responseKey];
}
});

return keyObj;
}

function computeFieldKeyObject(
specifier: KeySpecifier,
field: FieldNode | null,
source: Record<string, any> | null,
variables: Record<string, any> | undefined,
): Record<string, any> {
// The order of adding properties to keyObj affects its JSON serialization,
// so we are careful to build keyObj in the order of keys given in
// specifier.
const keyObj = Object.create(null);

let last: undefined | {
key: string;
source: any;
};

specifier.forEach(key => {
if (Array.isArray(key)) {
if (last) {
keyObj[last.key] =
computeFieldKeyObject(key, field, last.source, variables);
}
} else {
const firstChar = key.charAt(0);

if (firstChar === "@") {
if (field && isNonEmptyArray(field.directives)) {
// TODO Cache this work somehow, a la aliasMap?
const directiveName = key.slice(1);
// If the directive appears multiple times, only the first
// occurrence's arguments will be used. TODO Allow repetition?
Comment on lines +1142 to +1145
Copy link
Member Author

Choose a reason for hiding this comment

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

Some TODOs to consider.

const d = field.directives.find(d => d.name.value === directiveName);
if (d) {
last = {
key,
// Fortunately argumentsObjectFromField works for DirectiveNode!
source: keyObj[key] = argumentsObjectFromField(d, variables),
};
return;
}
}

} else if (firstChar === "$") {
const variableName = key.slice(1);
if (variables && hasOwn.call(variables, variableName)) {
last = { key, source: keyObj[key] = variables[variableName] };
return;
}

} else if (source && hasOwn.call(source, key)) {
last = { key, source: keyObj[key] = source[key] };
return;
}

last = void 0;
}
});

return keyObj;
}