-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Storage and invalidation for custom field read functions. #5667
Storage and invalidation for custom field read functions. #5667
Conversation
TypeScript doesn't do a great job of enforcing parameter types (including `this:` types) for functions called with Function.prototype.{call,apply}, which is especially frustrating because you pretty much have to use those methods when you want to call a function with a specific `this` object. Another reason to avoid using `this` is simply that some developers prefer arrow functions, and arrow functions ignore any `this` object provided by .call or .apply. Instead, we can expose the Policies object as a property in the FieldFunctionOptions parameter, so it can be used (or ignored) without having to think about `this` at all.
We're no longer passing in a StoreObject, so the longer name is now technically inaccurate.
If a read function needs to cache expensive computations, or do any other sort of long-term bookkeeping, it's convenient to have a unique private storage object that gets passed to the read function each time it is invoked for a particular field within a particular object. Making this work for normalized entity objects that have a string ID was easy, but it was somewhat trickier to support non-normalized, nested objects. The trick is to use the object itself as a key in a WeakMap (via the KeyTrie), so the object will not be kept alive after it has been removed from the cache. We do not need or want to continue using the same storage object after such a change, because it is never safe to assume a non-normalized object has the same identity as any other (!==) object in the cache.
Custom field read functions that listen to external data sources need to inform the cache when the external data change, so any queries that previously consumed the field can be reevaluated. Invalidation can also be triggered by writing a new value for the underlying field data, but not all read functions are backed by existing data in the cache, so options.invalidate() fills the gap for those purely dynamic read functions.
At first I thought it would be risky to allow getFieldValue to call read functions, because it would open the door to infinite recursion and expensive chains of read functions. However, after attempting to write tests that made heavy use of getFieldValue, I realized that calling read functions is just too useful to forbid, and, if we crippled getFieldValue in this way, developers would probably just resort to using cache.readFragment to read data from the cache, which passes through several more layers of abstraction, and thus is almost certainly slower than readField.
export type FieldValueGetter = | ||
ReturnType<typeof makeFieldValueGetter>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this pattern of exporting just the inferred return type of a function, without exporting the function itself.
field: string, | ||
foreignRef?: Reference, | ||
) => any, | ||
typename = getFieldValue("__typename") as string, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could keep passing in the typename
(with a default expression fallback), since we always have to call getFieldValue<string>(objectOrReference, "__typename")
in executeSelectionSet
, which is the primary caller of this method. That would make reading fields that don't have custom read
functions a tiny bit faster.
invalidate() { | ||
policies.fieldDep.dirty(storage); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In order to be really useful, the options.invalidate
function should also schedule a cache.broadcastWatches()
call. I'll tackle that in a follow-up PR.
@@ -472,6 +602,473 @@ describe("type policies", function () { | |||
expect(cache.extract(true)).toEqual(expectedExtraction); | |||
}); | |||
|
|||
it("readField helper function calls custom read functions", function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a long test, but I think it's worth reading through it to get a sense for what it's like to implement highly dynamic custom read
functions in terms of other custom read
functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great @benjamn - super useful! Just a quick note while we're working on the docs: the idea of using a cache (StorageType
) while working with the cache (InMemoryCache
) might confuse people as they ramp up with the new cache API, so we'll want to make sure this is addressed clearly. Thanks!
src/cache/inmemory/policies.ts
Outdated
// consumed this field. If you use options.storage as a cache, setting a | ||
// new value in the cache and then calling options.invalidate() can be a | ||
// good way to deliver asynchronous results. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if the use invalidate
to help deliver asynchronous results mention here might confuse people, without more context. These are code comments so maybe not (as people in here are grokking the source at the same time), but since we're mentioning it we might want to add another sentence or two that expands on this.
Custom field
read
functions might need to perform expensive computations, so caching the results ofread
functions is encouraged. Unfortunately, there is no good way for theInMemoryCache
to provide that caching automatically, sinceread
function results could depend on any/all/some/none of the arguments passed to the field, and only the application developer knows which arguments are important.At this point you might be thinking, "Doesn't the developer already have an opportunity to tell the cache which arguments are important by configuring
keyArgs: [...]
in the field policy?" While that statement is true, it's only the beginning of the story. When you specifykeyArgs
, you're telling the cache how to distinguish multiple values for a given field, but theread
andmerge
functions have access to the complete set of arguments, sokeyArgs
alone is not enough to differentiate between all possibleread
function return values.In fact, a common pattern when implementing custom
read
andmerge
functions is to passkeyArgs: false
to disable the default argument-based differentiation entirely, so theread
andmerge
functions can take full responsibility for interpreting the arguments passed to the field. In this common scenario, the cache knows nothing about howread
results should be stored. In short,read
function caching is a responsibility that must be left to theread
function.To resolve this seemingly unresolvable conundrum, I've adopted a simple but flexible policy: every
read
function now has access to a privateoptions.storage
object, which is aRecord<string, any>
where theread
function can stash any information it wants to preserve across multiple invocations of theread
function. Thisoptions.storage
object is unique to the current entity object and the current field name (plus any additional information specified viakeyArgs
). In other words, you can think of thisoptions.storage
object as storing mutable metadata about the immutableexisting
field value.Here's an example of a cached
read
function for aWorkItem.expensiveResult
field:To complement the
options.storage
object, this PR also introduces anoptions.invalidate()
function that can be called to invalidate any cached query results that previously consumed the field value, which is especially useful when theread
function uses external data sources that might change over time. Specifically, callinginvalidate()
will invalidate any results that were computed using the sameoptions.storage
object:Finally, this PR renames the
options.getFieldValue
helper function tooptions.readField
, and allows it to invoke customread
functions for any fields that it reads, rather than just retrieving theexisting
field value. See the commit message and tests included in 5219e36 to understand why this change was important.