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

Apollo Angular 4.0 QueryRef.fetchMore parameter typings remove updateQuery property #1810

Closed
KeithGillette opened this issue Aug 25, 2022 · 6 comments

Comments

@KeithGillette
Copy link
Contributor

KeithGillette commented Aug 25, 2022

Describe the bug
Upgrading from [email protected] to [email protected] produces TS2345 error on code which passes an updateQuery property to the fetchMoreOptions parameter of a QueryRef.fetchMorecall.

To Reproduce
Previously, the QueryRef.fetchMore fetchMoreOptions parameter was typed with FetchMoreQueryOptions & FetchMoreOptions but Apollo Angular 4 removed FetchMoreOptions and with it the updateQuery property on the parameter.

Expected behavior
The method signature should not change, since the underlying Apollo Client ObservableQuery.fetchMore method still has updateQuery in its signature.

Environment:

├── @angular/[email protected] 
├── @angular/[email protected] 
├── @apollo/[email protected] 
├── [email protected] 
├── [email protected]
├── [email protected] 
└── [email protected] 

Additional context
That pretty much sums it up.

@KeithGillette
Copy link
Contributor Author

Finding that this typing issue still exists in release 4.1.0, I investigated further and found that while updateQuery is still supported Apollo Client 3, it is deprecated and will be removed in the next major version of Apollo Client, so while its removal in Apollo Angular 4 may be somewhat premature, it is inevitable. I refactored our use of updateQuery (merging paginated results) to use the Apollo Client recommended solution of adding a field policy with a merge function.

@alfaproject
Copy link

@KeithGillette I have a dynamic helper that creates a dynamic updateQuery to avoid repetition, something like this:

      updateQuery: (prev, { fetchMoreResult }) => {
        if (!fetchMoreResult) {
          return prev;
        }

        const newEdges = fetchMoreResult[this.connectionFieldName].edges;
        if (newEdges.length === 0) {
          return prev;
        }

        return {
          [this.connectionFieldName]: {
            ...fetchMoreResult[this.connectionFieldName],
            edges:
              this.direction === Direction.ASCENDING
                ? [...prev[this.connectionFieldName].edges, ...newEdges]
                : [...newEdges, ...prev[this.connectionFieldName].edges],
          },
        };
      },

Are you telling me that we now need to go to our cache adaptor and add type policy for each individual connection. We have hundreds of them and each one is 'owned' by a particular service which is agnostic to the cache o.o

@KeithGillette
Copy link
Contributor Author

Hi, @alfaproject — Well, so far as I can see, updateQuery is still available on ObservableQuery in Apollo Client 3.x and not marked in the code as deprecated, so I think you could continue using it with Apollo Angular 4 with suitable @ts-ignore comments, but since I haven't seen anything to contradict the statements from the Apollo team that I linked to in my last message indicating that updateQuery will be removed in Apollo Client 4 and the fetchMore documentation has been updated to remove reference to updateQuery in favor of field policies, I do think that you'll eventually have to update your code to use merge functions on field policies, one for each field that needs your dynamic updateQuery logic. Note that field policy merge functions can be generic with respect to the data type, just as your updateQuery function example is.

@jwrascoe
Copy link

@KeithGillette I also do the same as you with respect to a dynamic helper.... I cant seem to figure out get a simple policy merge working let alone a dynamic one have you?

For now I put in the a @ts-ignore and im working fine on the latest but somehow need to figure out this policy merge.

My existing super simple merge that works fine.

    updateQuery: (prev, { fetchMoreResult }) => {
      if (!fetchMoreResult) {
        return prev;
      }
      fetchMoreResult.allProducts.edges = [
        ...prev.allProducts.edges,
        ...fetchMoreResult.allProducts.edges
      ];
      return fetchMoreResult;
    }
  }

I would think something like this would work.. but incoming is always null and not iterable

cache: new InMemoryCache({
  typePolicies: {
    Query: {
      fields: {
        allProducts: {
          keyArgs: false,
          merge(existing = [], incoming) {
            console.log('I need to figure out how to merge');
            return [...existing, ...incoming];
          }
        }
      }
    }
  }
})

@KeithGillette
Copy link
Contributor Author

@jwrascoe — I'm not sure why the incoming argument would always be null in your case. The field policy merges we're using are either integrating skip/limit or page-based pagination results, not relay-style cursors. We're using basically a copy/paste of the sample merge functions on the Apollo documentation.

@jwrascoe
Copy link

jwrascoe commented Nov 1, 2022

@KeithGillette -- yes its strange, I have been using Apollo since its early inception against a postgres via PostGraphile and have never had a situation like this before, perhaps its related to how edges are returned or that I am calling only with a limit and offset from a cursor

So strange...

--Forgive me on the formatting.. not sure why its getting messed up in git

fetchDataMore() {
this.productQuery.fetchMore({
variables: {
limit: 8,
offset: this.searchResults.edges.length
}
});
}

query allProductsByClientIdForSearchMore(
$offset: Int
$limit: Int
) {
allProducts(
orderBy: NAME_ASC
offset: $offset
first: $limit

) {
totalCount
pageInfo {
hasNextPage
endCursor
}
edges {
cursor
node {
id
name
price
}
}
}
}

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