relay-hooks icon indicating copy to clipboard operation
relay-hooks copied to clipboard

usePagination bug: loadNext doesn't work when using a custom identifier field

Open Lalitha-Iyer opened this issue 2 years ago • 4 comments

Description:

We use a custom id field _id and have configured the relay compiler to use this custom field as id. useQuery hook works fine however when using the usePagination hook and loadNext to fetch more items. I see the below error in the graphql response. message: "Variable '_id' has coerced Null value for NonNull type 'ID!'",…}

Fix
I could fix this locally by changing the id field in variables to be _id. We probably want to use identifierField instead of a fixed key. Something along these lines

            paginationVariables.id = identifierValue;     // Before
            paginationVariables[identifierField] = identifierValue;    // After

https://github.com/relay-tools/relay-hooks/blob/master/src/FragmentResolver.ts#L644

Lalitha-Iyer avatar Mar 23 '22 00:03 Lalitha-Iyer

hi @Lalitha-Iyer, the logic of pagination is the same as how it is managed in relay. https://github.com/facebook/relay/blob/main/packages/react-relay/relay-hooks/useLoadMoreFunction.js#L201-L215

Can you edit the sample project (https://github.com/relay-tools/relay-hooks/tree/master/examples/relay-hook-example/pagination-nextjs-ssr) so that I can study the your case?

morrys avatar Mar 23 '22 10:03 morrys

sure thing, I will edit the project to have an example with a custom node id and try to repro the error. The customizable node id is a more recent development in relay. So it's possible the pagination in the official relay implementation also doesn't work as expected.

Lalitha-Iyer avatar Mar 23 '22 21:03 Lalitha-Iyer

I tried setting up the example, but am struggling to configure relay compiler with nextjs. I will give it a try again this week, otherwise would it be ok if I shared a code-sandbox example.

Lalitha-Iyer avatar Apr 12 '22 19:04 Lalitha-Iyer

Hi, @morrys I didn't get time to repro this in a sandbox. However there is now a relay issue that describes this with a repro

Lalitha-Iyer avatar Aug 15 '22 22:08 Lalitha-Iyer

Hi, @morrys I was looking at the solution proposed in the relay PR, looks similar to what I suggested here. See - packages/react-relay/relay-hooks/useLoadMoreFunction.js

Besides pagination variable we would also have to fix refetchable variable here https://github.com/relay-tools/relay-hooks/blob/2cda9c68ee5e69d996730f576a91603892cf771a/src/FragmentResolver.ts#L494

I am assuming the comment about @fetchable directive doesn't apply to us, hence we could just apply the simpler fix. https://github.com/facebook/relay/pull/4053#issuecomment-1231005869

What are your thoughts on fixing this issue? I am also happy to submit a PR with the fix if it helps.

Lalitha-Iyer avatar Dec 13 '22 08:12 Lalitha-Iyer

Hi @Lalitha-Iyer, thanks for the tip 👍 For now, I'd like to wait to see how the PR in Relay goes. Let's keep in touch

morrys avatar Dec 13 '22 12:12 morrys

@morrys one workaround that seems to work is passing extra variables like so. Do you see any issues with doing this.

 loadNext(3, { UNSTABLE_extraVariables: { customId } }) 

Lalitha-Iyer avatar Jan 12 '23 04:01 Lalitha-Iyer

@morrys The PR in relay has been merged, whenever you get a chance could you revisit this issue. https://github.com/facebook/relay/pull/4053#issuecomment-1572598196

Lalitha-Iyer avatar Jun 06 '23 18:06 Lalitha-Iyer

@Lalitha-Iyer As soon as relay releases PR with v16 version, I will modify relay-hooks in order to release v9 :)

morrys avatar Jun 12 '23 14:06 morrys

released with relay-hooks v9.0.0

morrys avatar Jan 17 '24 16:01 morrys