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

Implement apollo's direct cache access (writeQuery, readQuery) #176

Closed
lucasavila00 opened this issue Feb 21, 2019 · 12 comments
Closed

Implement apollo's direct cache access (writeQuery, readQuery) #176

lucasavila00 opened this issue Feb 21, 2019 · 12 comments
Labels
enhancement New feature or request next Feature or report is related to next 📦 cache Relates to caching functionality

Comments

@lucasavila00
Copy link
Contributor

Let's say I query a list of items and then I query just one item from this list.
I know that the query for one item has the data on the list one but I cannot tell the cache so.

Like apollo, I'd like to be able to do so, but currently the cache is keyed based on the Operation().toKey() method.

The way I thought about doing it is exposing two methods on the client (readQuery and writeQuery) that does this stuff of figuring out the key behind the scenes.

On my fork I've done it and you can check it out.
(the implementation)

My fork is not well mantained and I'm changing stuff all the time so don't assume this is how we'd implement it, I'm only showing it to discuss the surfacing API.

Anyway, having this I can write to the query to expand lists in the cache and mutate fields whenever I want.

What do you think?
Perhaps we should use names like readQueryFromCache and writeQueryToCache for clarity?

Once this is solved I can write a proper PR.

@juicycleff
Copy link

@degroote22 that's nice, I think readQueryFromCache and writeQueryToCache seem like a name for this feature. I will wait for @HofmannZ for his input, but you could send a PR so we test this out.

@juicycleff
Copy link

I just saw your PR, I will test it out

@HofmannZ
Copy link
Member

@degroote22 I think that exposing cache to the query/mutation would be great. I think we should just expose the whole cache rather than just the read/write operations. That's also how Apollo does it. This would also allow us to update the cache after a mutation.

@degroote22 @juicycleff Do you guys agree we should keep that in line with Apollo?

@HofmannZ HofmannZ added the enhancement New feature or request label Feb 24, 2019
@HofmannZ HofmannZ added the next Feature or report is related to next label Feb 24, 2019
@juicycleff
Copy link

@HofmannZ I totally agree to this. I have been looking at the Apollo cache implementation on Android it is the apollo way

@micimize micimize mentioned this issue Mar 24, 2019
3 tasks
@micimize
Copy link
Collaborator

I have some related typed state management work in my project that takes advantage of cache normalization to add custom transient state to entities (gist probably not particularly interpret-able out of context)

@micimize
Copy link
Collaborator

These methods are exposed in Apollo via the DataProxy interface, which is implemented by the cache and exposed by the client. With Operation.fromOptions and operationName as a getter in #199, the code for this will become a lot simpler.
In the cache:

void writeQuery(QueryOptions options, dynamic data) => write(
      Operation.fromOptions(options).toKey(),
      data,
);
dynamic readQuery(QueryOptions options) => read(
      Operation.fromOptions(options).toKey()
);

In the client:

void writeQuery(QueryOptions options, dynamic data) {
    cache.writeQuery(options, data);
   queryManager.rebroadcastQueries();
};
dynamic readQuery(QueryOptions options) => cache.readQuery(options);

@micimize micimize changed the title [Feature Request] Expose cache read/write Implement apollo's direct cache access (writeQuery, readQuery) Apr 18, 2019
@lucasavila00
Copy link
Contributor Author

Is there a reason not to implement this after #199 has been merged?

@lucasavila00
Copy link
Contributor Author

I implemented it on my fork on the query manager itself. I think the cache doesn't need to know about the Operation as how to generate a key.

Client:

  QueryResult readQuery(QueryOptions options) {
    return queryManager.readQuery(options);
  }

  void writeQuery(QueryOptions options, dynamic data) {
    queryManager.writeQuery(options, data);
  }

Query Manager:

  QueryResult readQuery(QueryOptions options) {
    final Operation operation = Operation.fromOptions(options);

    final QueryResult queryResult = QueryResult(
      data: cache.read(operation.toKey()),
      source: QueryResultSource.Cache,
    );

    return queryResult;
  }

  void writeQuery(QueryOptions options, dynamic data) {
    final Operation operation = Operation.fromOptions(options);

    cache.write(
      operation.toKey(),
      data,
    );
    rebroadcastQueries();
  }

@micimize
Copy link
Collaborator

@lucasavila00 yeah I was just modeling it on apollo, but I also prefer keeping the cache more generic.
Your implementation looks good to me - would be a welcome PR (on beta)

@lucasavila00
Copy link
Contributor Author

I plan to submit it with tests.
I'm in a kind of busy week but soon I'll submit it.

@klavs
Copy link
Collaborator

klavs commented Oct 7, 2019

As I mentioned in #400 (comment), I think the cache must know about the operation and it's variables to cache the data more intelligently. It is required to understand how each parameterized field was actually queried.

@micimize
Copy link
Collaborator

This has been implemented on the cache in v4. Am consolidating the rebroadcasting client methods into #728

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request next Feature or report is related to next 📦 cache Relates to caching functionality
Projects
None yet
Development

No branches or pull requests

5 participants