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

Cache FieldPolicy.merge receives stale args when using fetchMore #5951

Closed
cbergmiller opened this issue Feb 15, 2020 · 11 comments · Fixed by #6464
Closed

Cache FieldPolicy.merge receives stale args when using fetchMore #5951

cbergmiller opened this issue Feb 15, 2020 · 11 comments · Fixed by #6464
Assignees
Milestone

Comments

@cbergmiller
Copy link

I am not shure if this is a bug or if i am using fetchMore in the wrong way. Help would be greatly appreciated.

Intended outcome:
When using fetchMore for a pagination query FieldPolicy.merge should be called with FieldFunctionOptions.args that correspond to the variables given to fetchMore.

Actual outcome:
FieldFunctionOptions.args always contains the field variables of the initial query (e.g. limit: 30, offset: 0). Even if fetchMore was called with e.g. offset: 10.

Code
Client configuration

return new ApolloClient({
    cache: new InMemoryCache({
        typePolicies: {
            LogEntryPagination: {
                fields: {
                    rows: {
                        keyArgs: false,
                        merge(existing: any[], incoming: any[], params) {
                            const offset = params.variables.offset;
                            const merged = existing ? existing.slice(0) : [];
                            // Insert the incoming elements in the right places, according to args.
                            const end = offset + incoming.length;
                            for (let i = offset; i < end; ++i) {
                                merged[i] = incoming[i - offset];
                            }
                            return merged;
                        },
                    },
                },
            },
        },
    }),
    link,
    queryDeduplication: true,
    defaultOptions: {
        query: {
            errorPolicy: 'all',
        },
        mutate: {
            errorPolicy: 'all',
        },
    },
});

Query

query ListLogEntries(
    $limit: Int!
    $offset: Int!
    $order: [String]
    $site: ID
) {
    logEntries(
        order: $order
        site: $site
    ) {
        count
        rows(limit: $limit, offset: $offset) {
            ...LogEntry
        }
    }
}

Custom Query hook

export const useListLogs = (siteId: string, limit: number) => {
    const {data, loading, error, fetchMore} = useQuery(
        ListLogEntries,
        {
            variables: {
                site: siteId,
                offset: 0,
                limit,
            },
            notifyOnNetworkStatusChange: true,
        }
    );
    return {
        logs: data?.logEntries,
        fetchMoreLogs: fetchMore,
        isLoadingLogs: loading,
    };
};

Use of query hook in component

const {logs, fetchMoreLogs, isLoadingLogs} = useListLogs(siteId, batchSize);
return ( 
    <VirtualizedTable
        loadMoreRows={async (offset, limit) => {
            await fetchMoreLogs({
                variables: {
                    limit,
                    offset,
                },
            });
        }}>
);

Versions

  System:
    OS: Windows 10 10.0.18363
  Binaries:
    Node: 12.6.0
    Yarn: 1.21.1
    npm: 5.1.0
  npmPackages:
    @apollo/client: ^3.0.0-beta.34
    apollo-link-error: ^2.0.0-beta.0
@benjamn
Copy link
Member

benjamn commented Feb 16, 2020

I think you should be using params.args.offset instead of params.variables.offset, though I'm not sure if that will fix your problem.

@benjamn benjamn added this to the Release 3.0 milestone Feb 16, 2020
@cbergmiller
Copy link
Author

@benjamn you are right, i should have used params.args.offset in my example.
However, args also hold the values of the initial query when using fetchMore.
For now i am using updateQuery in the fetchMore options to merge the results.

@seandearnaley
Copy link

seandearnaley commented Feb 18, 2020

hi @benjamn thanks for all your hard work on these updates. I was following this thread as I'm about to attempt to implement fetchMore using the new merge function on the typePolicy API but I noticed you added this issue to the Release 3.0 milestone, should I hold off until it's ready or should the args be coming in updated now? thanks again, great work!

Double checked and I get the old args from before fetchMore.

@benjamn benjamn self-assigned this Feb 21, 2020
@krvajal
Copy link

krvajal commented Mar 31, 2020

I can confirm this as well. The args passed to merge after a call to fetchMore are the ones from the original query. Is this the intended behaviour?

@chrisgco
Copy link

chrisgco commented Jun 7, 2020

Chiming in here,

Looks like the issue is that ObservableQuery.fetchMore calls ObservableQuery.updateQuery, which calls QueryManager.cache.writeQuery with the variables from the original call, see here. If we could allow updateQuery to accept the updated variables, or have the mapFn return both data and updated variables, they could be passed to QueryManager.cache.writeQuery.

@benjamn What do you think is the best option to solve this?

@benjamn
Copy link
Member

benjamn commented Jun 10, 2020

Thanks @chrisgco! Your diagnosis is absolutely correct, and you made my job a lot easier by sharing your findings.

I want to apologize to everyone here for the sorry shape of fetchMore in AC3. I hope it hasn't been keeping you from using the betas/RCs, though I wouldn't blame you if it did. I believe the new field policy API (read and merge functions and keyArgs) will make pagination a lot easier and more reliable in AC3, but that doesn't matter if fetchMore is unable to cooperate with merge functions.

We should have this fixed (and more thoroughly tested) very soon.

@mschipperheyn
Copy link

mschipperheyn commented Jun 16, 2020

@benjamn I wonder if this also applies to refetch? We have been having a lot of problems with refetch and refetchQueries for a while now on 3.0.0. It's kind of hard to pin down since it doesn't seem to occur always. Is there anything to be aware of? I want to spend some time on trying to create a reproduction. But the jist of it is that when it happens,

  • the refetch query is correctly executed
  • the new results come in
  • a rerender is executed but with stale (cache) data

benjamn added a commit that referenced this issue Jun 19, 2020
This commit also deprecates the updateQuery function, while preserving its
existing behavior for backwards compatibility. If you're using a field
policy, you shouldn't need an updateQuery function.

Fixes #5951.
benjamn added a commit that referenced this issue Jun 22, 2020
This commit also deprecates the updateQuery function, while preserving its
existing behavior for backwards compatibility. If you're using a field
policy, you shouldn't need an updateQuery function.

Fixes #5951.
@krvajal
Copy link

krvajal commented Jun 22, 2020

Great 🚀

@seandearnaley
Copy link

sweet!

@rossm6
Copy link

rossm6 commented Jun 19, 2022

This is still an issue for me. I'm using version - 3.7.0-beta.5. Does anybody else have the problem? I also had the problem with version 3.6.

@vanpacheco
Copy link

This is happening to me with the read function on v3.6.9.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
8 participants