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

Cashay v2 #150

Open
mattkrick opened this issue Dec 18, 2016 · 18 comments
Open

Cashay v2 #150

mattkrick opened this issue Dec 18, 2016 · 18 comments

Comments

@mattkrick
Copy link
Owner

#149 gets us half way to a new mutations API, so let's dive in how that might change.
I like Relay 2's direction of imperatively mutating the normalized store. I also like how they scrapped intersecting fat queries with tracked queries.

  • When I write a mutation, I want to explicitly write everything I want back from the server. Cashay does something magical by writing the intersection itself, but sometimes a mutation comes before a query. Cashay handles this by using @cached on a query so it'll return nothing until the mutation comes back, but sometimes it's nice to get state from the result of cashay.mutate and not have to get it from the redux-store. Doing this will shrink cashay by roughly 40%.
  • Optimistic updates: each mutation should take a function on how to adjust the normalized state optimistically. Take the getTop5Posts example, there are 4 ways that query can change:
    • The array of documents changes (A,B,C,D,E becomes A,B,C,D,F) optimistically
    • A document within the array gets updated (A.votes = A.votes +1) optimistically
    • The array of documents changes from the server
    • A document within the array gets updated from the server
      1 and 3 should be discrete functions that are linked to the query operation (eg getTop5Posts) and passed into cashay to store.

//TODO

@mattkrick
Copy link
Owner Author

So, let's say each mutation has 3 user-defined methods:

const upvoteMutation = {
  optimistic: (store, optimisticVars) => {
    return store.entity('Post').get(optimisticVars.id).update((doc) => ({
      updatedAt: new Date(),
      votes: doc('votes') + 1
    }));
  },
  resolve: (store, response) => {
    return store.entity('Post').get(response.id).update({
      votes: response.votes
    });
  },
updateResults: (store, doc) => {
    const currentResult = store.result('getTop5Posts').get({area});
    let minIdx;
    let minVal = Infinity;
    currentResult.forEach((res, idx) => {
      if (res('votes') < minVal) {
        minVal = res('votes');
        minIdx = idx;
      }
    });
    if (minVal < doc('votes') && minIdx !== undefined) {
      currentResult.add(doc);
      currentResult.remove(minIdx);
    }
  }
};

optimistic is called immediately after the optimisticVars is sent to the server. resolve is called with the response from the server. updateResults is called after both. The first 2 do nothing but update a single entity. updateResults targets each query that it could affect. It finds out if there is a post with fewer votes than the one that just got updated, and if so, it replaces it. The dependencies are checked and if nothing else depends on that document, it is removed from the state. The order doesn't matter because changing the array invalidates the query & that causes it to be re-run the query-specific sort/filter functions.

For example, let's say client A is listening to getTop5Posts/California and client B just upvoted post126. That upvote bumped it into the top 5! The message queue is going to kick down a message to client A that looks kinda like this:

{
  resultChanges: [
    {
      channel: 'getTop5Posts/california',
      removeDoc: 'post123',
      addedDoc: 'post126'
    }
  ],
  docsToUpsert: [
    {
      id: 'post126',
      upvotes: 28
    }
  ]
}

cashay will internally call some code like:

const [result, area] = channel.split('/');
const args = {area};
const doc = store.entity('Post').upsert({
  id: 'post126',
  upvotes: 28
});
const currentResult = store.result(result).get(args);
const idxToRemove = currentResult.find((res) => res('id') === 'post123');
currentResult.add(doc);
currentResult.remove(idxToRemove);

@dustinfarris
Copy link
Contributor

These changes look really great—if I'm reading correctly, this will greatly simplify (eliminate?) mutationHandlers.

@mattkrick
Copy link
Owner Author

@dustinfarris yup. this will largely reduce the scale & complexity of the whole mutations (and eventually subscriptions) API.
What's still unclear is how to better handle pagination. I still like hiding the cursor from the front-end developer & just having cashay grab it behind the scenes, i'll need to give it more thought.

@wmertens
Copy link

Sorry, it's late on a Friday and this is a bit of a meandering post.

I'm sizing up the competition of apollo-client because I find the DX (mostly through react-apollo) rather clunky. Lokka and Cashay both have interesting ideas for making a dev's life simpler, but the apollo ecosystem is farther along in featureset, from what I can tell.

From what I can tell, everybody is shying away from the clever query intersecting because it's much easier to just ask for the whole query with a pre-arranged id instead. Am I correct in understanding that the cashay webpack loader is not necessary then?

I like that you can just call a mutation by name with variables, which 100% covers my use case and makes code much nicer. Calling two mutations in one mutation is just asking for trouble.

BTW, with react-apollo the mutation is defined and called (including any promise follow-ups) in the components, which I find grating, I'd much rather see this be done close to the Redux store actions and reducers.

In apollo, a mutation is called with extra configuration to provide an optimistic response, and there are 4 different ways to update the cache based on the mutation. You can tweak your own data, data of other active queries defined by the query name and the variables, or you can listen to all mutation responses on Redux and update your own state based on what other mutations got as results. You can also simply tell certain queries to refetch. None of those ways is always best.

Comments on your example:

  • When you optimistically add one to a vote, and accept the latest server response as correct, you will have problems when there are are multiple votes in flight. Suppose you add 5 votes to a thing, then the optimistic result will first show 5 added and then drop back to 2, 3, 4, 5 added as the results come in. Instead, you should keep track of the optimistic result for each mutation and only change the state if the server version differs from what was expected. I've been told Apollo does this.
  • In the update, you miss updating the timestamp
  • the top 5 post thing assumes per-one updates but the server can have more-than-one vote updates

Sorry for the rambling, just spouting thoughts :)

PS: Apollo does store data in denormalized form (too denormalized, I have a couple thousand keys from anonymous arrays, it slows down the devtools enormously), you might want to update your comparison table. You can also sort/filter responses via the props() function on a query, but that is per-component, the original data is what goes in the store.

@mattkrick
Copy link
Owner Author

but the apollo ecosystem is farther along in featureset

if freakin better be, $31MM of VC funding is depending on it 😂

that's very interesting about them denormalizing queries, i remembered when they were first getting started we had a little chat & that was one of the things they were adamantly against. I'm assuming they figured out that creating a whole bunch of new objects every single time a dispatch is called isn't the best use of CPU cycles...

I have a couple thousand keys from anonymous arrays, it slows down the devtools enormously

I guess me & apollo still disagree on the definition of state. To me, state is something you can't calculate (at least not cheaply). That keeps redux devtools readable & means I can persist my entire state to localStorage without too much fanfare.

Suppose you add 5 votes to a thing, then the optimistic result will first show 5 added and then drop back to 2, 3, 4, 5 added as the results come in. Instead, you should keep track of the optimistic result for each mutation and only change the state if the server version differs from what was expected. I've been told Apollo does this.

oh NOW we're getting into the fun stuff!
Premise: post.value = 1 i click "vote" twice.
So, state S0 = 1.
S1 = 2
S2 = 3

If I understand you correctly, you're saying when S1' == 2 (server response), then only invalidate if S1' !== S1 and reapply the chain of operations to S2.
Now this is sound logic. Similar to what i did in redux-optimistic-ui. Honestly though, I probably won't build this in. I'd much prefer to let the user pass in an invalidate = (source, doc) => doc.votes !== source, doc). then i don't have to store an array of cloned states, nor do i need to perform an operation on the state before i do my comparison. reasonable default for basics (cuz inflight latency is rarely an issue), but then allow the dev to make something super powerful & performant by passing in an extra option.

@wmertens
Copy link

wmertens commented Feb 12, 2017 via email

@mattkrick
Copy link
Owner Author

let's stick with the voting example, assume reddit rules where you can only upvote something once.

so, you click it twice.
Votes go from 0 to 2.
The first request gets sent via http & gets redirected to a slow server
the second request gets sent via http & goes to a fast server

the response for req2 comes back first successful, but the response for req1 comes back a failure. how do you achieve the proper state?

you need at least 1 state clone because the result of req2 may be different if req1 failed (and it is-- the state should be 1, not 2). So, you copy state at the time of the first optimistic request, then you store a history of all operations. when a failed operation comes in, you go back to the first copied state & reply the actions disregarding the fail. if it comes bak a success, you mark the optimistic ones as successes & can update the copied state to the point of the earliest inflight.

code is easier to grok than me. check out https://github.com/mattkrick/redux-optimistic-ui. it's like 100 well-commented lines.

@wmertens
Copy link

wmertens commented Feb 12, 2017 via email

@wmertens
Copy link

wmertens commented Feb 13, 2017 via email

@mattkrick
Copy link
Owner Author

mutations should not be declarative, this is the error that cashay, relay, and apollo all made. to be declarative means mutating the denormalized state. Since 1 mutation affects many queries, you'll need each query to handle that mutation. If a query doesn't handle the mutation, then the denormalized state will be stale. Sure, it's simpler for the dev, but the surface area for errors increases from S(1) to S(n).

storing the expected server response should be sufficient

mathematically, no. remember the old redux motto: state = fn(state, action). it's a fun enough proof. try the above example i outlined (2 upvotes, the second comes back a success, then the first comes back a failure) without persisting S0.

@wmertens
Copy link

wmertens commented Feb 15, 2017 via email

@mattkrick
Copy link
Owner Author

But this is a different world view. For Apollo, the server state/local
cache is not (conceptually) part of the Redux store. It requires you to
program your reducers such that for any graphql action/event A
(sendQuery/sendMutation/receiveResult) fn(state, A) === state.

Can you give an example of this? I still don't understand

@wmertens
Copy link

wmertens commented Feb 16, 2017 via email

@mattkrick
Copy link
Owner Author

mattkrick commented Feb 16, 2017 via email

@wmertens
Copy link

wmertens commented Feb 16, 2017 via email

@mattkrick
Copy link
Owner Author

Something to think about in free time:

  • When removing an item from a subscription, sometimes we want to remove that entity (say when a user gets deleted) and other times we want to leave it there (say when a user gets removed from the Top 5 list of users).

The dependency to decide whether or not to remove it is not the component/key, rather the source (query or subscription). That means we'd need to save a list of sources on each entity & track that, which is bleh.

@wmertens
Copy link

If you reason that at any one point there will only be a smallish number of cached queries (say <500), and most cacheable objects will only be involved with a small number of them, you can keep a WeakSet with each query that says if an object is used in that query.

When an object is removed, you can check all the WeakSets and only walk the caches that have the object.

@wmertens
Copy link

(that is, if you are using shared singular mutable objects as cache basis like I describe at apollographql/apollo-client#1300 (comment))

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

3 participants