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

[Waiting for #374] Add fetchMore method to the observable query #454

Closed
wants to merge 10 commits into from

Conversation

rricard
Copy link
Contributor

@rricard rricard commented Jul 22, 2016

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

Linked to the work of #350

Usage example

let handleCount = 0;
const handle = client.watchQuery({
  query: gql`{
    allPeople @apolloFetchMore(name: "people") {
      id
      name
   }
  `
}).subscribe({
  next({data}) {
    handleCount ++;
    if(handleCount === 1) {
      // data === {allPeople: [{id: 'luke', name: 'Luke Skywalker'}]}
      handle.fetchMore(['people']);
    } else {
      // data === {allPeople: [{id: 'luke', name: 'Master Luke Skywalker'}, {id: 'jarjar', name: 'Jar Jar Binks'}]}
      // Note: if the id matches, consider the item new and discard the old one
    }
  }
});

You can't control the order in which the data is merged, that's why, for now, you should define the order in your rendering function. A next PR may change that but we have the issue of serialising functions into the store ...

Usage example after #453

let handleCount = 0;
const handle = client.watchQuery({
  query: gql`query people($cur: ID){
    allPeople(cursor: $cur) @apolloFetchMore(name: "people") {
      id
      name
   }
  `,
  paginationArguments: ['cursor'],
}).subscribe({
  next({data}) {
    handleCount ++;
    if(handleCount === 1) {
      // data === {allPeople: [{id: 'luke', name: 'Luke Skywalker'}]}
      handle.fetchMore(['people'], {cur: "afterluke"});
    } else {
      // data === {allPeople: [{id: 'luke', name: 'Luke Skywalker'}, {id: 'jarjar', name: 'Jar Jar Binks'}]}
    }
  }
});

@rricard rricard self-assigned this Jul 22, 2016
@rricard rricard changed the title Add fetchMore method to the observable query [WIP] Add fetchMore method to the observable query Jul 22, 2016
@rricard rricard added ready and removed in progress labels Jul 22, 2016
@rricard rricard changed the title [WIP] Add fetchMore method to the observable query [Waiting for #374] Add fetchMore method to the observable query Jul 22, 2016
@Slava
Copy link
Contributor

Slava commented Jul 22, 2016

Thanks for extracting your work from #350!

I just published a different proposal for a feature that might solve a similar provlem in #455. I would be curious to see if the two can co-exist or we should choose one.

@rricard
Copy link
Contributor Author

rricard commented Jul 22, 2016

I'll try to take a look on Monday. Thanks for the pointers!

This one will notify the QueryManager there are some `fetchMoreLocations` to get and concat. You can optionally carry some variables (but it is not interesting to do so until #453 is properly merged)
That way we'll be able to use it later in the write to the store
And test it!

Concatenation is enough in a basic fetchMore setting. For now, let's just advise people to re-sort at rendering-level!
And tested!

Needs to be tested on more complex stuff now!
This breaks automatically the testing suite since some are too complex to get green. Next commit is about fixing that...
By looking of ID fields we can handle complex cases such as updating at the same time two nested arrays.
One test is still failing, I need to go update an another branch for that ...
For now the test will be skipped
@stubailo stubailo closed this Aug 1, 2016
@rricard rricard deleted the basic-fetch-more branch August 2, 2016 08:11
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants