Skip to content

Commit

Permalink
Avoid constraining read/merge parameters to StoreValue type.
Browse files Browse the repository at this point in the history
Should help with the type issues reported in #5881, by defaulting
TExisting and TIncoming to any.

The last time I tried this, I ran into problems when the user-supplied
types were primitive (already read-only) types like string, since string
values are not assignable to Readonly<any> parameters. This time, I was
able to work around that dubious error using the SafeReadonly<T> type.
  • Loading branch information
benjamn committed Jan 30, 2020
1 parent 2c5374b commit 47a667b
Showing 1 changed file with 24 additions and 15 deletions.
39 changes: 24 additions & 15 deletions src/cache/inmemory/policies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ export type TypePolicy = {

fields?: {
[fieldName: string]:
| FieldPolicy<StoreValue>
| FieldReadFunction<StoreValue>;
| FieldPolicy<any>
| FieldReadFunction<any>;
}
};

Expand All @@ -88,8 +88,17 @@ type KeyArgsFunction = (
},
) => ReturnType<IdGetter>;

// The Readonly<T> type only really works for object types, since it marks
// all of the object's properties as readonly, but there are many cases when
// a generic type parameter like TExisting might be a string or some other
// primitive type, in which case we need to avoid wrapping it with Readonly.
// SafeReadonly<string> collapses to just string, which makes string
// assignable to SafeReadonly<any>, whereas string is not assignable to
// Readonly<any>, somewhat surprisingly.
type SafeReadonly<T> = T extends object ? Readonly<T> : T;

export type FieldPolicy<
TExisting,
TExisting = any,
TIncoming = TExisting,
TReadResult = TExisting,
> = {
Expand Down Expand Up @@ -140,15 +149,15 @@ interface FieldFunctionOptions {
readField<T = StoreValue>(
nameOrField: string | FieldNode,
foreignObjOrRef?: StoreObject | Reference,
): Readonly<T>;
): SafeReadonly<T>;

// A handy place to put field-specific data that you want to survive
// across multiple read function calls. Useful for field-level caching,
// if your read function does any expensive work.
storage: StorageType;
}

export type FieldReadFunction<TExisting, TReadResult = TExisting> = (
export type FieldReadFunction<TExisting = any, TReadResult = TExisting> = (
// When reading a field, one often needs to know about any existing
// value stored for that field. If the field is read before any value
// has been written to the cache, this existing parameter will be
Expand All @@ -157,15 +166,15 @@ export type FieldReadFunction<TExisting, TReadResult = TExisting> = (
// than one of the named options) because that makes it possible for the
// developer to annotate it with a type, without also having to provide
// a whole new type for the options object.
existing: Readonly<TExisting> | undefined,
existing: SafeReadonly<TExisting> | undefined,
options: FieldFunctionOptions,
) => TReadResult;

export type FieldMergeFunction<TExisting, TIncoming = TExisting> = (
existing: Readonly<TExisting> | undefined,
export type FieldMergeFunction<TExisting = any, TIncoming = TExisting> = (
existing: SafeReadonly<TExisting> | undefined,
// The incoming parameter needs to be positional as well, for the same
// reasons discussed in FieldReadFunction above.
incoming: Readonly<TIncoming>,
incoming: SafeReadonly<TIncoming>,
options: FieldFunctionOptions,
) => TExisting;

Expand Down Expand Up @@ -193,8 +202,8 @@ export class Policies {
fields?: {
[fieldName: string]: {
keyFn?: KeyArgsFunction;
read?: FieldReadFunction<StoreValue>;
merge?: FieldMergeFunction<StoreValue>;
read?: FieldReadFunction<any>;
merge?: FieldMergeFunction<any>;
};
};
};
Expand Down Expand Up @@ -429,7 +438,7 @@ export class Policies {
return function getFieldValue<T = StoreValue>(
objectOrReference: StoreObject | Reference,
storeFieldName: string,
): Readonly<T> {
): SafeReadonly<T> {
let fieldValue: StoreValue;
if (isReference(objectOrReference)) {
const dataId = objectOrReference.__ref;
Expand All @@ -444,7 +453,7 @@ export class Policies {
fieldValue = objectOrReference && objectOrReference[storeFieldName];
}
// Enforce Readonly<T> at runtime, in development.
return maybeDeepFreeze(fieldValue) as T;
return maybeDeepFreeze(fieldValue) as SafeReadonly<T>;
};
}

Expand Down Expand Up @@ -490,7 +499,7 @@ export class Policies {
getFieldValue: FieldValueGetter,
variables?: Record<string, any>,
typename = getFieldValue<string>(objectOrReference, "__typename"),
): Readonly<V> {
): SafeReadonly<V> {
invariant(
objectOrReference,
"Must provide an object or Reference when calling Policies#readField",
Expand Down Expand Up @@ -520,7 +529,7 @@ export class Policies {
storage,
getFieldValue,
variables,
)) as Readonly<V>;
)) as SafeReadonly<V>;
}

return existing;
Expand Down

0 comments on commit 47a667b

Please sign in to comment.