Relay returning too many connection edges from QueryRenderer

[ekosz]

Hi there. I'm trying to build a windowed pagination component and I've discovered a regression(?) when upgrading from compat to modern.

Because Relay doesn't support windowed pagination natively, I implemented it using QueryRenderers manually keeping track of the endCursor. This worked with a compat QueryRenderer, but once I upgraded to modern this stopped working as expected.

In this first screenshot you can see what the QueryRenderer is returning in terms of props. This is the first "page" after requesting the first 2 edges in the connection.

In this next screenshot you can see that we are requesting the next "page" which is the next 2 edges in the connection after the endCursor of the prev "page".

What happened in compat

We would receive props which only contained only the two new edges. This is exactly what we requested and what we want.

What's happening in modern

We are actually receiving 4 edges. The 2 from the prev page and the 2 from the next page of results. This makes is so that instead of windowed pagination we are instead making it a infinitely growing list.

This doesn't seem like the correct behavior. I would expect that we would only get props related to the variables I gave the QueryRenderer not additional edges that wasn't part of the request / response from the server.

[sibelius]

can you share code that was working and now is not working anymore?

[ekosz]

@sibelius @josephsavona For sure! Here you go: https://github.com/ekosz/relay-pagination-bug

Currently, this repo still has a bug where the compat doesn't render its results? I don't think its related at all, but at least the modern version is showing the bug absolutely.

I'm going to continue trying to get compat to work.

[ekosz]

(whoops, sorry about the accidental close/missclick)

So I traced the relay code and I found out why compat isn't rendering. This line is erroring out https://github.com/facebook/relay/blob/master/packages/react-relay/classic/store/RelayStoreData.js#L775 because graphql-js responses objects have their prototypes striped (relevant issue).

That means a TypeError: response.hasOwnProperty is not a function error is thrown at that location. Something then swallows that error as nothing is logged.

A fix on the relay side would be to change that line of code to

if (!Object.prototype.hasOwnProperty.call(response, serializationKey)) {

But I'll see if there's anything I can do for my demo in the meantime

[devknoll]

@ekosz Is your test case passing results directly from graphql-js into Relay? If so, I would recommend resolving the issue in your network layer (e.g. by cloning the response). Relay Classic is likely assuming the result is coming from a network request, therefore being a regular Object.

[josephsavona]

I think the issue here is that for Modern you’re using @connection, which implements cursor-based pagination for you and merges the edges from subsequent fetches into the store after the previous edges. So you can either keep @connection and simplify the implementation of withPagination a bunch, or if you want to do everything manually just drop the @connection.

[ekosz]

@josephsavona Do you know of a "blessed" implementation of a windowed paginator using @connection? We would really like to continue using @connection for the mutation helpers relay gives.

I guess what we could do it track both the start and end cursor that we're looking for. Then just iterate the list until we find the start custor and continuing until the end cursor is found. Does that sound correct?

[josephsavona]

@ekosz I'm not aware of one. Note that @connection effectively creates a client/local field in the graphql schema to store the merged results of the initial fetch plus pagination queries to load more edges - on the assumption that you're doing infinite scroll style. A few options:

• Use a custom handler (@connection(handler: "...") and configure that custom handler in your handlerProvider when creating an Environment). It's complicated but doable.
• Use @connection and slice out the window of edges that you care about in render or similar.
• Don't use @connection, do pagination manually (your code seems to handle most cases already?).

[sibelius]

you should start from here https://github.com/facebook/relay/blob/master/packages/relay-runtime/handlers/connection/RelayConnectionHandler.js#L46

to create a new handler

[sibelius]

you can slice edges in your component until you figure it out how to implement a custom RelayConnectionHandler

you could have a hook like this

const { edges } = useRelayPagination({ connection: myConnection })

[babangsund]

you can slice edges in your component until you figure it out how to implement a custom RelayConnectionHandler

you could have a hook like this

const { edges } = useRelayPagination({ connection: myConnection })

I guess you were ahead of time on this one 😂

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[mercurio-developing]

I get work cursor based pagination like windowed pagination getting the total count of items with that I get the number of pages, and the page you click * number of items per page, and calls the query with variable first = page number * number of items per page and pass also last with the number of items per page. Example 100 items 20 per page 5 pages click page 3 it will 3 * 20 = 60 this value it will be first and last 20 you get the items from 40 to 60.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.