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

How should we do pagination? #18

Closed
viniciussbs opened this issue Apr 27, 2017 · 9 comments
Closed

How should we do pagination? #18

viniciussbs opened this issue Apr 27, 2017 · 9 comments

Comments

@viniciussbs
Copy link
Contributor

Hi, Blake.

We have already talked about it a couple of times on Slack. I'm bringing here the topic. Are you doing pagination? How are you doing it?

I've been playing with pagination for a while. My implementation was based on Relay-style cursor pagination and written on ember-apollo-client v0.1.2. I've faced some problems with observable queries and their subscription, so I had to use this.get('apollo.client').watchQuery to handle observable queries and their subscriptions by hand. I've updated ember-apollo-client to v0.2.2 and now my implementation is broken due to some changes on Apollo Core - but I know what I need to change.

I see, now, that it's time to stop putting my hands on Apollo Core and use more ApolloService. Because the things we need to handle on Ember are not straightforward. In exemple, since Apollo Core v0.8.0, query results are deep frozen, so we need to deep copy the query result to "defrost" it because Ember asks for an extensible object as a model. And ApolloService already handle this on query and queryOnce.

I have an exemple of my old implementation. It's a little bit outdated since it's not a production code, but it shows how I play with observable queries and their subscriptions.

I would love to see how do implement pagination. Are you relying on Apollo Core too? Do you think ember-apollo-client should mirror Apollo Core functions?

@bgentry
Copy link
Member

bgentry commented May 1, 2017

For posterity, @viniciussbs and I discussed this on Slack a bit last week. I am not yet doing pagination but I won't be able to put it off much longer.

#20 might offer a good path to improving your pagination implementation. Not sure if this is the right thing to do or not, but I exposed the observable on the query result as _apolloObservable. This gives you direct access to it to be able to refetch or fetchMore, from anywhere that you pass the data down.

Take a look at let me know if that's an improvement and/or if you have a better idea :)

@viniciussbs
Copy link
Contributor Author

I was testing some implementations here. This is the boilerplate code to do the Relay-style cursor pagination on Ember:

import Ember from 'ember';
import gql from 'graphql-tag';

const {
  A,
  Route,
  inject: { service },
  copy,
  isPresent,
  get
} = Ember;

export default Route.extend({
  apollo: service(),

  query: gql`
    query UserList($cursor: String) {
      users(first: 20, after: $cursor) {
        edges {
          node {
            id
            name
            email
          }
        }
        pageInfo {
          endCursor
        }
      }
    }
  `,

  model() {
    let query = this.get('query');
    let observableQuery = this.get('apollo.client').watchQuery({ query, fetchPolicy: 'network-only' });
    let route = this;
    let querySubscription = observableQuery.subscribe({
      next(result) {
        let model = get(route, 'controller.model');
        let users = route.getUsers(result);

        // At the first run, we don't have a controller nor a model because the result was not returned yet.
        if (model && isPresent(users)) {
          model.pushObjects(users);
        }
      }
    });

    this.setProperties({ observableQuery, querySubscription });

    return observableQuery.result().then((result) => this.getUsers(result));
  },

  resetController() {
    this._super(...arguments);
    this.get('querySubscription').unsubscribe();
  },

  getUsers(result) {
    let users = result.data.users.edges.map((edge) => edge.node)
    // We need a deep copy because Apollo returns a deeply frozen result object and Ember asks for extensible objects.
    return A(copy(users, true));
  },

  actions: {
    fetchMore() {
      let observableQuery = this.get('observableQuery');
      let cursor = observableQuery.currentResult().data.users.pageInfo.endCursor;

      observableQuery.fetchMore({
        variables: { cursor },
        updateQuery(previousResult, { fetchMoreResult }) {
          let oldCursor = previousResult.users.pageInfo.endCursor;
          let newCursor = fetchMoreResult.users.pageInfo.endCursor;
          let newResult = copy(fetchMoreResult, true);

          newResult.users.pageInfo.endCursor = newCursor || oldCursor;

          return newResult;
        }
      });
    }
  }
});

We need to subscribe to an observable query waiting for more results. The fetchMore should return only the new result set - we can't aggregate the new result with the previous one like in the Apollo's exemple - to avoid Ember to render the entire list after every fetch. And need to handle the deeply frozen result objects from Apollo. It's not a straightforward code.

This shows me that the reactive aspect of fetchMore don't fits Ember out of the box. Maybe a simple query fits better. Something like this:

import Ember from 'ember';
import gql from 'graphql-tag';

const {
  A,
  Route,
  inject: { service },
  isPresent
} = Ember;

export default Route.extend({
  apollo: service(),

  query: gql`
    query UserList($cursor: String) {
      users(first: 20, after: $cursor) {
        edges {
          node {
            id
            name
            email
          }
        }
        pageInfo {
          endCursor
        }
      }
    }
  `,

  fetchData() {
    let query = this.get('query');
    let cursor = this.get('cursorToMoreResults');
    let variables = { cursor };

    return this.get('apollo').queryOnce({ query, variables, fetchPolicy: 'network-only' }, 'users').then((users) => {
      this.set('cursorToMoreResults', users.pageInfo.endCursor || cursor);

      return A(users.edges.map((edge) => edge.node));
    });
  },

  beforeModel() {
    this._super(...arguments);
    this.set('cursorToMoreResults', null);
  },

  model() {
    return this.fetchData();
  },

  actions: {
    fetchMore() {
      this.fetchData().then((newData) => {
        isPresent(newData) && this.controller.model.pushObjects(newData);
      });
    }
  }
});

This way we still have a Relay-style cursor pagination but in a more Emberish style. I could test more this simplified implementation and send a pull request with a mixin that handles the cursor and exposes this fetchData function.

#20 is still helpful because it can get rid of more then one query subscription into the same route. I have a use case for this. I'll share it there.

Let me know what do you think, @bgentry.

@bgentry
Copy link
Member

bgentry commented Sep 5, 2017

@viniciussbs I believe this was made quite a bit easier by #20? Did you get pagination working well using that PR?

Maybe something can be added to the readme or at least as a comment here to show others how it's done?

@viniciussbs
Copy link
Contributor Author

@bgentry As soon as possible I will post something here explaining how I'm doing pagination after #20 and then we update the README.

@bgentry
Copy link
Member

bgentry commented Oct 30, 2017

So I have pagination working well in my app using relay-style pagination cursors. Initially I built it to use fetchMore, but quickly abandoned that for two reasons:

  1. fetchMore has an open bug where it leaks watched queries in Apollo Client and there is no way to clean them up ( fetchMore leaks watched queries apollographql/apollo-client#2286 )
  2. It became messier to propagate pagination params to the route's query params when I used fetchMore from anywhere other than the route or controller.

What I settled on was to have my paginated table component call a closure action that was passed in. The action is just called something like nextPage and it takes the cursor as an argument. The controller is then responsible for transitioning to a route with new query params, and the route itself takes care of loading that new data with a new watchQuery. This is the approach I would recommend to folks doing traditional pagination.

I think fetchMore would be especially useful if you're doing infinite scroll, and I'm not sure of any other good way to do that. I suggest you comment / upvote the Apollo Client issue if you want it to get fixed.

Meanwhile, I'm closing this issue as I don't think there's anything that needs tracking here. If somebody wants to help with documentation and/or a demo app that shows this off, please do 🙏

Thanks for the discussion.

@bgentry bgentry closed this as completed Oct 30, 2017
@DevanB
Copy link
Contributor

DevanB commented Oct 31, 2017

This is fantastic to hear! I'm going to add it to the How to GraphQL tutorial now!

@bgentry
Copy link
Member

bgentry commented Oct 31, 2017

@DevanB awesome! I could provide some example code for that but can't do it this week, ping me in a week if you haven't figured it out already :)

@sumeetattree
Copy link

@bgentry How do you recommend going forward with this without using the query manager? According to the docs you recommend using the query manager's fetchMore:

import Route from "@ember/routing/route";
import { getObservable } from "ember-apollo-client";

export default Route.extend(RouteQueryManager, {
  model() {
    let result = this.get('apollo').watchQuery(...);
    let observable = getObservable(result);
    observable.fetchMore(...) // utilize the ObservableQuery
    ...
  }
});

However this does not work with service:apollo.

let observable = getObservable(result); // this returns undefined

Considering that ES classes are going to be de-facto going forward, I wanted to completely eliminate the use of any mixins.

Do I have to manually subscribe and unsubscribe as viniciussbs above demonstrates in his code examples?

@viniciussbs How did you finally get around to implementing pagination? I am currently trying to get it to work with relay style cursor pagination but have run into the same issues as you.

Thank you!

@viniciussbs
Copy link
Contributor Author

@viniciussbs How did you finally get around to implementing pagination? I am currently trying to get it to work with relay style cursor pagination but have run into the same issues as you.

I'm using the RouteQueryManager mixin. I haven't started removing the mixins because there are some symbiotic mixins in the app I'm working on, but it would be great a new strategy without mixins.

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

4 participants