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

Remove reference to injectCacheManager from RelayEnvironment code comment #1030

Closed
tlvenn opened this issue Apr 8, 2016 · 9 comments
Closed

Comments

@tlvenn
Copy link
Contributor

tlvenn commented Apr 8, 2016

The RelayEnvironment documentation states the following:

* `RelayEnvironment` is the public API for Relay core. Each instance provides
 * an isolated environment with:
 * - Methods for fetchng and updating data.
 * - An in-memory cache of fetched data.
 * - A configurable network layer for resolving queries/mutations.
 * - A configurable task scheduler to control when internal tasks are executed.
 * - A configurable cache manager for persisting data between sessions.

At the moment, the network layer and the task scheduler can be configured with injectNetworkLayer and injectTaskScheduler. Similarly the cache manager should have a injectCacheManager method.

@cdroulers
Copy link

I also can't find any clear documentation on the cache manager and how to implement it properly. Are there such resources?

@wincent
Copy link
Contributor

wincent commented Jun 22, 2016

We're using a cache manager internally in our React Native apps, but the cache is external to Relay itself and very specific to our native FB product infrastructure, so we don't have much we can say about that. There is some evidence in Relay of the hooks that we've built in to enable that integration (and the reference to injectCacheManager is one of those), but given that we haven't provided (and probably can't meaningfully provide) an example, it's not something that we've emphasized in the docs.

In fact, the mention of the "configurable cache manager" is probably unintentional in the RelayEnvironment docs, because as @tlvenn points out, no injectCacheManager is declared in the RelayEnvironment class. (But you can see one on the RelayStoreData class, which is an @internal implementation detail.)

If you look in RelayTypes, you can see what a CacheManager would need to implement:

export type CacheManager = {
  clear: () => void,
  getMutationWriter: () => CacheWriter,
  getQueryWriter: () => CacheWriter,
  readNode: (
    id: DataID,
    callback: (error: any, value: any) => void
  ) => void,
   readRootCall: (
    callName: string,
    callValue: string,
    callback: (error: any, value: any) => void
  ) => void,
};

It has fire-and-forget methods for writing to the cache, and async methods for reading from it. I'd consider all of this to be subject to change, so I am reluctant to expose any of it on RelayEnvironment itself. If anything, we should remove that reference from the doc comment so that it doesn't cause more confusion.

@cdroulers
Copy link

cdroulers commented Jun 22, 2016

So there are no plans to have caching in relay? I've already implemented a
basic cache manager + writer with localstorage, but I haven't been able to
cache a connection. I'm thinking I'm doing something wrong in my writer but
have no idea what. Is there a better place to look? Is there a future plan
for the lind of scenario requiring a cache that persists between page loads
or even sessions? I'm researching offline capabilities and the cache
manager seemed like my best bet.

@wincent
Copy link
Contributor

wincent commented Jun 22, 2016

Well, we do have it, but not inside Relay. The way we're trying to go moving forward is to make a small, modular, well-defined core upon which it is easy to build other abstractions (or compose inside existing abstractions). Offline caching is a good example of the kind of thing that we don't want in the core but we do want to be possible in "user space". Your use-case (ha, almost wrote use-cache) is very similar to the ones for which we use the disk cache (not strictly offline mode, but fulfilling queries with (potentially stale) data while we wait for fresh data to come back from the network.

I am not sure what the limitations with respect to caching connections are. At a high level the API just deals with records, fields and root calls. @yuzhi knows the most about this so perhaps she can chime in on how this interoperates (or doesn't) with connections.

@josephsavona
Copy link
Contributor

As @wincent said, our plan here is to make it easier to build things like offline caching in user space, and I agree that we'd prefer not to expose the injectCacheManager API publicly.

If you're trying to implement it anyway though - for connections you'll notice that the __range__ property is an instance of a GraphQLRange. To serialize this you can call toJSON on the range instance. When you load it back from disk, you may have to call new GraphQLRange(cachedData.__range__) - I don't remember offhand whether we do that within the cache reader or if the cache implementation has to do that.

@yuzhi
Copy link
Contributor

yuzhi commented Jun 22, 2016

To chime in on caching the connection if you insist on implementing it externally. As Joe already mentioned, connection metadata is currently stored on a GraphQLRange on the __range__ key (subject to change). You can toJSON it to serialize it. To rehydrate it, just do GraphQLRange.fromJSON(JSON.parse(blob)).

@cdroulers
Copy link

I'll look at __range__ key for now. I'm mostly in the prototyping phase, making sure all the pieces needed to use Relay are there. I'm quite interested in the disk cache and whatever else can be created in user-space. Is there a specific project for that or is it still in the works?

@wincent
Copy link
Contributor

wincent commented Jun 23, 2016

Is there a specific project for that or is it still in the works?

There is no open source project for it. It is something we currently use in our React Native apps, like I said above, and it is specific to our native infra which is not part of Relay (or even React Native).

@wincent wincent changed the title Add missing injectCacheManager in RelayEnvironment API Remove reference to injectCacheManager from RelayEnvironment code comment Jun 23, 2016
@wincent
Copy link
Contributor

wincent commented Jun 23, 2016

This specific issue is closed now, but we'll keep the desire to have offline cache functionality on our radar, specifically in the context of refactoring Relay into a smaller core (#559) with this kind of functionality possible to build in "user space".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants