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

Make makeVar a global function instead of a method of InMemoryCache. #6512

Merged
merged 2 commits into from
Jun 30, 2020

Conversation

benjamn
Copy link
Member

@benjamn benjamn commented Jun 30, 2020

The makeVar method was originally attached to InMemoryCache so that we could call cache.broadcastWatches() whenever the variable was updated. See #5799 and #5976 for background.

However, as a number of developers have reported, requiring access to an InMemoryCache to create a ReactiveVar can be awkward, since the code that calls makeVar may not be colocated with the code that creates the cache, and it is often desirable to create and initialize reactive variables before the cache has even been created.

As this commit shows, the ReactiveVar function can infer the current InMemoryCache from a contextual Slot<InMemoryCache>, when called without arguments (that is, when reading the variable). When the variable is updated (by passing a new value to the ReactiveVar function), any caches that previously read the variable will be notified of the update. Since this logic happens at variable access time rather than variable creation time, makeVar can be a free-floating global function, importable directly from @apollo/client.

This new system allows the variable to become associated with any number of InMemoryCache instances, whereas previously a given variable was only ever associated with one InMemoryCache. Note: when I say "any number" I very much mean to include zero, since a ReactiveVar that has not been associated with any caches yet can still be used as a container, and will not trigger any broadcasts when updated.

The Slot class that makes this all work may seem like magic, but we have been using it ever since Apollo Client 2.5 (#3394, via the optimism library), so it has been amply battle-tested. This magic works.

The makeVar method was originally attached to InMemoryCache so that we
could call cache.broadcastWatches() whenever the variable was updated.
See #5799 and #5976 for background.

However, as a number of developers have reported, requiring access to an
InMemoryCache to create a ReactiveVar can be awkward, since the code that
calls makeVar may not be colocated with the code that creates the cache,
and it is often desirable to create and initialize reactive variables
before the cache has been created.

As this commit shows, the ReactiveVar function can infer the current
InMemoryCache from a contextual Slot, when called without arguments (that
is, when reading the variable). When the variable is updated (by passing a
new value to the ReactiveVar function), any caches that previously read
the variable will be notified of the update. Since this logic happens at
variable access time rather than variable creation time, makeVar can be a
free-floating global function, importable directly from @apollo/client.

This new system allows the variable to become associated with any number
of InMemoryCache instances, whereas previously a given variable was only
ever associated with one InMemoryCache. Note: when I say "any number" I
very much mean to include zero, since a ReactiveVar that has not been
associated with any caches yet can still be used as a container, and will
not trigger any broadcasts when updated.

The Slot class that makes this all work may seem like magic, but we have
been using it ever since Apollo Client 2.5 (#3394, via the optimism
library), so it has been amply battle-tested. This magic works.
@@ -16,6 +16,7 @@ const external = [
'graphql/language/visitor',
'graphql-tag',
'fast-json-stable-stringify',
'@wry/context',
Copy link
Member Author

Choose a reason for hiding this comment

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

Super important to include any new direct dependencies in this list; otherwise the whole dependency gets pulled into the Apollo Client bundle!

package.json Show resolved Hide resolved
Copy link
Member

@hwillson hwillson left a comment

Choose a reason for hiding this comment

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

This is awesome @benjamn! 🎉

src/cache/inmemory/inMemoryCache.ts Show resolved Hide resolved
@hwillson hwillson mentioned this pull request Jun 30, 2020
47 tasks
@benjamn benjamn merged commit 363927c into master Jun 30, 2020
@benjamn benjamn deleted the free-floating-makeVar branch June 30, 2020 17:23
benjamn added a commit that referenced this pull request Jun 30, 2020
Follow-up to #6512.

Also, set cacheSlot when calling local state resolver functions, since
they are analogous to field read functions.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants