fixes issue #7549

[EandrewJones]

Description

In the typescript-react-query plugin, there are useInfinite${operationName}Query hooks. These require a pageParamKey to be specified in order for the hook to fetch the next page in paginated queries. Previously, this param was defined as a private param with a preceding '_' and was left unused in the call to the fetcher.

I dropped the private underscore prefix pageParamKey and updated the fetcher call so that the metaData.pageParam is passed to the pageParamKey variable. This allows pagination function properly.

Related dotansimha/graphql-code-generator-community#174

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)

Screenshots/Sandbox (if appropriate/relevant):

N/A

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

• Updated the typescript-react-query plugin test suite (packages/plugins/typescript/react-query/tests/react-query.spec.ts) to match the new outputs

Can be tested with:

$ yarn build
$ yarn test

Test Environment:

• OS: Ubuntu 18.04
• NodeJS: v14.20.1

Checklist:

• I have followed the CONTRIBUTING doc and the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules

Further comments

N/A

[changeset-bot]

🦋 Changeset detected

Latest commit: 858902a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@graphql-codegen/typescript-react-query	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[saihaj]

@EandrewJones can you fix tests?

[EandrewJones]

I will take a look later today. All tests passed locally when I ran yarn test, so I need to see which ones are failing.

…

On Tue, Oct 18, 2022, 8:25 AM Saihajpreet Singh ***@***.***> wrote: @EandrewJones <https://github.com/EandrewJones> can you fix tests? — Reply to this email directly, view it on GitHub <#8497 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AJ2T6APKOPZVRSBKIWYVM23WD2JLVANCNFSM6AAAAAARHOGKGU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[EandrewJones]

@dotansimha I ran:

yarn run generate:examples:esm
yarn run generate:examples:cjs

I believe this updated the types for the dev-test flow and should've fixed the issue.

[marcmll]

@EandrewJones, sadly it seems this change has broken all of my generated useInfiniteQuery hook calls.
cc @charlypoly

Context

One example of a paginated query I am calling is EmployeeQuery. The GraphQL schema has some non-nullable variables, including the one that I have set as the pageParamKey (paginationInput):

export type EmployeesQueryVariables = Exact<{
  paginationInput: WorkPaginationInput;
  filterInput?: InputMaybe<WorkEmployeesFilterInput>;
  sortInput: WorkEmployeeSortInput;
}>;

I call it in the following way:

const {
  data,
  fetchNextPage,
  hasNextPage,
  isFetching,
  isFetchingNextPage,
  isLoading,
  isStale,
} = useInfiniteEmployeesQuery(
  'paginationInput',
  getClient(),
  {
    filterInput,
    paginationInput: { first: PAGE_SIZE },
    sortInput,
  },
  {
    getNextPageParam,
    onError: () => {
      showToast('general', 'error');
    },
  }
);

As you can see, paginationInput is part of the variables.

Issue

When the query is called for the first time, metaData.pageParam is undefined. Previously, { ...variables, ...(metaData.pageParam ?? {}) } would result in all variables being passed during the first call. Now, the new implementation means the paginationInput variable is overwritten with undefined. By overwriting the [pageParamKey] with undefined during the first call, GraphQL returns an error as a required variable isn't defined.

Solution

Instead of assuming that metaData.pageParam is always defined, only overwrite the pageParamKey variable when metaData.pageParam has a value.

For example, in packages/plugins/typescript/react-query/src/fetcher-graphql-request.ts:

(metaData) => fetcher<${operationResultType}, ${operationVariablesTypes}>(client, ${documentVariableName}, {...variables, ...(metaData.pageParam ? { [pageParamKey]: metaData.pageParam } : {}}, headers)(),

Conclusion

Overall... I'm unsure about two things:

1. How come you assumed this was a "non-breaking change which fixes an issue" when this change clearly has breaking potential? Something not being used → something being used = something can potentially break
2. You state that "This allows pagination function properly." Did it not function properly before? I have been using the typescript-react-query plugin with react-query since react-query v3 all the way up until the newest version and have never had an issue with the pagination function not working properly. With this change in place... I will no longer be able to update the typescript-react-query plugin as this change results in it no longer working for my use case.

[EandrewJones]

@marcmll

Thanks for your comment and I apologize for the breaking change. I will promptly add the ternary operator.

Did it not function properly before?

In my case, no. The page variable is indeed part of the variables and the page param was being updated but not correctly passed back into the variables because I couldn't define the key.

I'm curious as to why the types generated for your infiniteQuery hooks correctly allowed for defining the pageParamKey but mine did not...

Answer

In fixing the PR, I've found the answer to my question. I am using a custom fetcher and the mapper (packages/plugins/typescript/react-query/src/fetcher-custom-mapper.ts) was missing the pageParamKey variable in the generated type.

Whereas it wasn't missing in packages/plugins/typescript/react-query/src/fetcher-graphql-request.ts.

I didn't notice the inconsistency so erroneously assumed infinite queries were broken in the same way for all users and didn't consider the breaking change possibility. That would explain why more people weren't complaining about infinite queries not properly working.