[WIP] Fetch more / Infinite Scroll

[rricard]

Adds a fetchMore method able to fetch new data and concatenate it into the store, performing, that way, an infinite scroll behavior!

TODO:

• Update CHANGELOG.md with your change
• Make sure all of the significant new logic is covered by tests
• Rebase your changes on master so that they can be merged easily
• Make sure all tests and linter rules pass
• change concatPaths: Array<string> to paginationParameters: Array<string>, that way suppressing for special paths those parameters from the store ids. (ex: instead of ROOT_QUERY.allPosts({cursor: 'x', type: 'y'}), if paginationParameters: {'ROOT_QUERY.allPosts': ['cursor']} is set, it will only be ROOT_QUERY.allPosts({type: 'y'})
• Implement correct resolution of paths
• Squash!
• paginationParameters => quietArguments
• Backwards concat possible
• Add directive for pagination !
• Add a per-directive merge option
• Add a directive selection in the fetch more to avoid fetching more of everything
• fetch more can override query-level opts
• embed into the directive common merging use cases: append/prepend/sort
• Make sure is correctly wired (fetchMore() to the public API, paginationParameters everytime there is some store access & `fetchMore everytime we're doing a query)
• Do a proper system to handle directives (right now it's clearly hacky) => REFACTOR!
• So do MORE TESTS and/or integrate with existing tests to take paginationParameters in account.
• Directive removal before sending the query to the server! OR PR on the server to support the directive there (by adding it to the schema! Maybe somewhere in graphql-tools)
• Add it to the docs!

Basic usage example:

let latest_name = null;
const subscription = client.watchQuery({
  query: gql`
    all_people(latest_name: $latest_name, type: $type) @apolloFetchMore {
       name
    }
  `,
  quietArguments: ['latest_name'],
  variables: {
    type: 'Jedi',
    latest_name: null,
  },
}).subscribe({
  next(result) {
    console.log(result);
    // First value => {all_people: [{name: 'Luke SkyWalker'}]}
    // Second value => {all_people: [{name: 'Luke SkyWalker'}, {name: 'Obi-Wan Kenobi'}]}
    const new_latest_name = result.all_people[result.all_people.length -1];
    if(new_latest_name !== latest_name) {
      latest_name = new_latest_name;
      subscription.refetchMore({latest_name});
    }
  }
});

Advanced usage example:

let latest_name = null;
const subscription = client.watchQuery({
  query: gql`
    messages(cursor: $chatCur) @apolloFetchMore(name: "messages") {
      content
      replies @apolloFetchMore(name: "replies") {
         content
       }
    }
  `,
  quietArguments: ['cursor'],
  variables: {
    chatCur: null,
  },
  mergeResults: {
    messages: (oldArr, newArr) => [].concat(newArr, oldArr),
  }
}).subscribe({
  next(result) {
    console.log(result);
    setTimeout(() => {
      subscription.refetchMore({targetedFetchMoreDirectives: ['replies']});
    }, 60000);
    setTimeout(() => {
      subscription.refetchMore({targetedFetchMoreDirectives: ['messages']});
    }, 10000);
  }
});

Advanced usage example with directives only!:

let latest_name = null;
const subscription = client.watchQuery({
  query: gql`
    messages(cursor: $chatCur) @apolloFetchMore(name: "messages", quiet: "cursor", prepend: true) {
      content
      replies @apolloFetchMore(name: "replies") {
         content
       }
    }
  `,
  variables: {
    chatCur: null,
  },
}).subscribe({
  next(result) {
    console.log(result);
    setTimeout(() => {
      subscription.refetchMore({targetedFetchMoreDirectives: ['replies']});
    }, 60000);
    setTimeout(() => {
      subscription.refetchMore({targetedFetchMoreDirectives: ['messages']});
    }, 10000);
  }
});

[rricard]

I know that merging that won't be a piece of 🍰 ... I am prepared to work with you to integrate that correctly.

[stubailo]

This looks like a really nice approach, and has huge potential! Thank you for giving it so much thought.

I just have a few questions:

1. How would this look if one were using the Relay connection spec? I imagine a lot of people have servers with that: https://facebook.github.io/relay/graphql/connections.htm
2. Is it possible to somehow make code more DRY, so that you can define a pagination behavior once, and then reuse it in multiple queries? I think it's fair to say that in a single app there will be a finite number of pagination approaches, so it would be good to make those reusable.
3. Does latest_name refer to latest_name: (the argument) or $latest_name (the variable)? In the example they are named the same so I can't tell (haven't read the code yet).
4. We should let people override where the results are added, at least give a prepend/append option. There are definitely cases like chat where you start at the bottom and scroll up.

This is definitely something we want to use in a production app. @timbotnik, can you chime in here, would this work for the Galaxy stuff?

[rricard]

1. Good point, for relay connections, this is super-straightforward: I would put the following in paginationParameters: ['first', 'last', 'after', 'before']. Those are the mandatory input parameters used by Relay.
2. I started out by defining a per-query rule but definitely (and especially with relay for instance) a global paginationParameters would be really great, I'll look into that!
3. I'm referring to the argument, maybe I should rename paginationParameters to paginationArguments, now you're asking, that makes way more sense!
4. To that end, I could make fetchMore directional maybe. What about a fetchMore(variables: {[key: string]}: any, backwardsConcat?: boolean) method signature ? So when adding in a chat, you would use something like this: fetchMore({lastMessageId}, true). OR we could do that with enum ConcatDirection = { Forward = 1, Backward = -1} and fetchMore(variables: {[key: string]}: any, concatDirection: ConcatDirection = Forward). After writing it, I prefer the second one!

[rricard]

@stubailo: I updated my PR TODO accordingly, can you tell me if that's ok for you as a PR roadmap ?

[stubailo]

1. Sounds great to me.
2. Sounds great to me - I was thinking also filtering by __typename and field would be great. So you could say "for these types, I'm using connections. for these other types, I'm using page numbers".
3. Sounds great to me.
4. I think it could be as simple as direction: 'APPEND' or PREPEND. We're using this vocabulary already in the recently merged mutation results feature that I have yet to document.

The TODO looks good! I'll try to take a look at the code soon, haven't done that yet, so I can't confirm if the implementation approach is correct.

[rricard]

@stubailo: about 2., I thought about filtering by __typename, however, in a paginated results, you may want to add other arguments than just the pagination arguments. For instance, I could want to add a filter or orderBy param (this is actually one of my use case!). And by doing that with __typename, you don't take those arguments as non-pagination arguments.

[rricard]

About 4., I'll take the existing vocabulary, that seems great!

[timbotnik]

This is pretty cool @rricard. Some thoughts:

1. I feel like trying to be smarter about global pagination params is not really an issue we should worry about (at least right now). I feel like a global config might lead to some "magic" that results in confusing behavior, and as well the caller can keep a set of pagination configs around pretty easily and use them where appropriate. We might also provide a few sample configs, such as a Relay config.
2. Is even the term pagination somewhat limiting in the effect of what this pattern could ultimately support? For example, one thing I've been pondering with respect to the idea of "fetchMore" is around the idea of maintaining a sort order in the results, such that you could also support progressive loading of data that might be interleaved, not just appended or prepended. We have some data sources that work like this, such as ElasticSearch results, but I can imagine many other use-cases. In Meteor, we got this "for free" (err, at the cost of some performance) with MiniMongo, since we can apply complex queries to our data cache at will. The current work-around we have to do here is maintain our own duplicate cache in the view layer and sort things ourselves (granted, with something like this PR, the complexity of that cache would be reduced).

So the point of all that rambling was instead of paginationParameters, I could pass something like:

moreOptions: {
  cursorFields: ['timestamp'], // same as paginationParameters, name TBD
  sortResultsBy: (d) => d.timestamp, // callback which indicates caller-specified sort order
  ... other things we haven't thought of
}

I'm sure this would be going beyond the scope of this PR, but it might inform something about the API / naming that we choose.

[rricard]

@timbotnik Interesting ... quietFields makes sense. However, I have myself sorting issues to handle:

1. I don't think that's the role of the store to handle that when concatening but either:
   • the graphql backend that orders results and us to manage how we concat stuff in the store
   • the React component (or whatever you're using) to order that before rendering.
2. Either way, sorting in the store would be an interesting matter to take on but it should be handled independently from the fetchMore system (because we may want to leverage that at any time).

For the global quietFields, I'm not sure either, but I think having it would be good. Especially over relay-compliant backends... They would be strictly identical for each query, I'll let you discuss that with @stubailo !

[timbotnik]

Good points @rricard - re: sort, wouldn't the same apply to the APPEND vs PREPEND argument? More concretely, could you have 2 instances of watchQuery with the same signature, one asking for APPEND and the other asking for PREPEND?

[rricard]

I would not associate the APPEND/PREPEND with the query but with the fetchMore. That way you control with your pagination arguments what you want next (exactly like in relay where the store concat will be different if you choose first or last)

[rricard]

But contrarily to what Relay does, Apollo works more in a configuration over convention manner when Relay seems to work more like magic (but enforces a strict schema form when Apollo does not). And for me, that was the point that made us adopt Apollo over relay.

[stubailo]

I feel like a global config might lead to some "magic" that results in confusing behavior, and as well the caller can keep a set of pagination configs around pretty easily and use them where appropriate.

I agree, let's start with local configuration, and we can add global pagination config later if it turns out to be helpful.

such that you could also support progressive loading of data that might be interleaved, not just appended or prepended

Perhaps instead of sortResults or similar, we should have addResults? Then you can do:

// Simple append
addResults: (existing, new) => {
  return existing.concat(new);
}

// Append, then sort by timestamp
addResults: (existing, new) => {
  return existing.concat(new).sort((a, b) => { return a.timestamp - b.timestamp });
}

// etc.

I agree with Tim that pagination isn't uniform. If you couldn't do this here, I suppose you could easily do it in your React component:

function MyComponent(props) {
  // Sort props before rendering
  const data = props.data.myArray.sort((a, b) => { return a.timestamp - b.timestamp });
  return <div></div>;
}

@timbotnik the above doesn't seem that bad, I wouldn't consider that to be a "duplicate cache".

However, @rricard even if your server is handling the ordering, you still need append vs. prepend, because sometimes one needs to paginate from the bottom of the list up (like in the chat usecase, but I suppose you could send the timestamp anyway).

---

Store insertion

Another thing that we should think about, which I think isn't too complex, is how the caching should behave in this case. Let's say I do the following:

1. Load a list of todo items, let's say the first 10; this will update the array in the cache to have 10 items
2. Use fetchMore to load 10 more items, in the current version this will update the same array in the cache to have 20 items
3. Leave this page, and go to a different page where I have a query to load the first 10 items, just like I did in step 1

Sounds like in this implementation, (3) will now return me 20 items instead of the 10 I ask for, because the pagination parameters will be ignored when reading from the cache?

This is leading me to think that for Relay, after and before shouldn't be ignored when reading from the cache, but they should be ignored when appending to an existing list.

[stubailo]

Oh, another concern I have when looking at the implementation - it looks like when using fetchMore, any arrays found in the result are appended to the store instead of replacing. It should probably be only the paginated array that we are interested in that is appended.

[rricard]

@stubailo need to move now, will respond to your concerns a bit later...

[stubailo]

In TypeScript, you could define a type for this to avoid duplicating this everywhere, like:

type FetchMoreMode = 'APPEND'|'PREPEND';

[rricard]

Yes, that's what I would like to do! But I'm still not sure in which file to put it ;)

[stubailo]

Thanks again for doing this! Really excited about having a pretty simple yet flexible pagination model.

[rricard]

Ok, I have a PR over at React to finish first, then back here!

[rricard]

@stubailo: So, here's what I did today first:

• APPEND/PREPEND to control how we concat after a fetch more
• while doing that, I changed the implementation to use diffFieldAgainstStore so we won't add the same items multiple times
• paginationParameters becomes quietFields, I think it's the best naming so far ...

Ok, now, followup to your comments:

• I agree, for now, we shouldn't do the global configuration. That is completely doable in a next PR 😉
• I really like the addResults approach. I would even go further. We could provide classic addResults implementations (addByAppending/addByPrepending/addBySorting(sort: Function)/...). From where the PR stands, that shouldn't be too hard to do!
• I intended at first to do the sorting at render-level in our app. Finally, having the sort going on before going into the store kinda seems more efficient (especially if I trigger multiple renderings for the same store state...)
• About the store insertion, the goal of having detached refetch and fetchMore was made to control that: I was inspired by what you said in an another issue. Because, not that any force refetch or refetch with other variables without specifying it as a fetch more will exactly act as it acted before !
• Any array will have results appended: that's a valid concern. I think it's not as bad as expected because I'm using diffFieldAgainstStore, so if the cursors don't move, I don't see issues on other arrays that could get appended. However, linking this to the type could be smart still so we would avoid really weird situations.

[rricard]

So my main concerns right now are the APIs:

• what to put on the fetchMore call ?
• what to put at the query level ?

[stubailo]

I pretty much agree with most of your conclusions. Some comments:

I would even go further. We could provide classic addResults implementations (addByAppending/addByPrepending/addBySorting(sort: Function)/...). From where the PR stands, that shouldn't be too hard to do!

We could provide some basics, but I think if it's only 3 lines of code to implement it would be more confusing than helpful. (like if it's literally return array.concat(otherArray))

Any array will have results appended: that's a valid concern. I think it's not as bad as expected because I'm using diffFieldAgainstStore, so if the cursors don't move, I don't see issues on other arrays that could get appended. However, linking this to the type could be smart still so we would avoid really weird situations.

I'm a bit confused about this. Like imagine this situation:

{
  todoList {
    todos(first: 10) {
      text
      tags # an array of strings
    }
  }
}

If I want to load 10 more todos, how do I make sure that the tags arrays aren't messed up?

[rricard]

Yes, I do agree, let's just keep it linked to the type!

[stubailo]

what to put on the fetchMore call

I'd make it a flat object of options:

fetchMore({
  variables: { ... },
  addResults: ...,
});

Perhaps there can be some way of specifying which arrays we are actually interested in appending? Maybe that's the job of addResults somehow?

Like maybe addResults just gets the complete first result, and the complete second result, and then you merge arrays as necessary?

[rricard]

Let's freeze the APIs.

I propose something type-based so we avoid any weird stuff.

query options

So we could add something with this signature into the query's option:

{
  quietArguments: string[],
  mergeResults: {[type: string]: (existing: any[], new: any[]) => any[]},
}

[rricard]

With a flat arg, we could override the query's opts from the fetchMore

fetchMore call

The fetchMore call will be able to control which types to fetch but not how to merge them nor which arguments will stay quiet.

fetchMore(opts: {
  variables: any,
  onTypes: string[],
  quietArguments?: string[],
  mergeResults?: {[type: string]: (existing: any[], new: any[]) => any[]},
})

[stubailo]

The fetchMore call will be able to control which types to fetch but not how to merge them nor which arguments will stay quiet.

It looks like with the proposed API it will actually be able to do that?

Also, does mergeResults get the arrays in the results that match onTypes?

Wait guys I have an idea - what if we just used a directive in the query to specify which fields we want to paginate on? Like this:

{
  todoList {
    todos(first: 10) @apolloFetchMore(name: "todos") {
      text
      tags # an array of strings
    }
  }
}

That way, it's totally unambiguous which array we are fetching more of.

[rricard]

That is very cool. I'll need some help there still, I know how to access directives in the backend (via the ast) but I'm not sure how to that on the client...

Also why the names argument and not a quietArgs instead?

[stubailo]

@rricard Oh, this is just to annotate that part of the query with a name - so when you call fetchMore you can tell it which array to refetch. This is instead of onTypes, which seems really hacky especially since you can't see the types in the query.

[jbaxleyiii]

@rricard if you look in the package.json you should see the size limit, I think we should be good to bump that for this great feature!

Also, when this is merged, I'll add fetchMore to the props in react-apollo 👍

I'm really looking forward / need this! Thanks so much for the great work!

[stubailo]

This is looking really good so far! I'm going to take an in-depth look today.

@rricard, I added you as a contributor to this repository. Do you mind re-submitting this as a branch from this repo so that I can also push commits to it? That way I can make small changes directly, instead of having to make another fork/PR.

Just link back to this for the conversation, and maybe copy the PR description into the new one.

[rricard]

@stubailo: yes I can do that, thanks for making me contributor!

[rricard]

Closing for a followup pr in the apollostack repo