infinite render loop with data table components

[cdreier]

Hi,

in version 3.0.0-alpha-1b4ddaba.0 i am running in infinite render loops when using https://v2.grommet.io/datatable or https://react-data-table-component.netlify.app/?path=/story/getting-started-intro--page

the component renders over and over - when downgrading to 2.3.0 everything is working fine

import { DataTable, Text } from "grommet"
import { useQuery } from "../gqty"


const Start = () => {

  console.log("render loooop!")
  const q = useQuery()

  return (
    <div>
      <DataTable
        data={q.students()}
        columns={[
          {
            property: 'lastname',
            header: <Text>Name</Text>,
          },
        ]} />
    </div>
  )
}

[vicary]

@cdreier Thanks again for reporting the issue.

I don't think I am familiar with Grommet enough, it throws the following error when I try to add <DataTable /> in a freshly created next.js app.

TypeError: defaultValidatorFn is not a function

Error	Source Code

Would you mind sharing a minimum reproducing repo?

[vicary]

I can reproduce the issue now, styled-components just released v6 RC and it's breaking grommet.

I'll look into it.

[cdreier]

Grommet is worth a look, also with the hpe theme and styleguide ;)

i did not upgrade styled-components yet, so glad you found the issue! thanks again for your work and fast support!

[vicary]

<DataTable /> is reading GQty's proxy outside of a render cycle, where GQty assumes to be a user interaction, e.g. button click, triggering another render.

@cdreier I tried replacing the re-render with a soft refetch in 3.0.0-alpha-d01132d2.0, give it a try!

[cdreier]

Hi @vicary - just tested the new release in my project and it is working pretty well now :)

thanks a lot!

[vicary]

I'm closing this for now, feel free to reopen if it happens again!

[cdreier]

Hi @vicary ,

i created a repo with another few strange bugs, also fetching loops in certain conditions - i am not sure if grommet is doing strange things, but with GQty 2.3 everything was working

i created a small graphql server where you can test against, and i added descriptions on the passengers page.

short description of the issues

• every hover over a row, triggers a fetch from the server - even if the table does something with the proxy, there should be a cache?
• there are strange dependencies between different useQuery hooks - it is hard to describe, so just open the passengers page and click one passenger with your network monitor open 🚀

https://github.com/cdreier/gqty_rendering

i really like the idea and philosophy behind GQty - feel free to ping me if you have other questions or if i can help at some point

edit: and i think i am not allowed to reopen this issue?

[vicary]

Hey @cdreier, I have reopened the issue. Replying your points below:

every hover over a row, triggers a fetch from the server - even if the table does something with the proxy, there should be a cache?

The new cache defaults to a more aggressive SWR refetching, it compounded with the aggressive reading in grommet.

Try increasing the maxAge to something like 5 seconds, it would greatly reduce the fetching frequency. You may also put maxAge: Infinity to disable expiry entirely.

there are strange dependencies between different useQuery hooks - it is hard to describe, so just open the passengers page and click one passenger with your network monitor open 🚀

Passing child proxies upwards to parent would mess up query scopes a little bit, you may prop drill from the parent <Passengers data={query.passengers()} /> to prevent this for now.

I'll figure out a way to force them into one single fetch, so they don't race with each other.

[cdreier]

Hi @vicary - you are right, it seems like for my usecases, a disabled cache expiry solves lots of problems! I try to dig a bit deeper during the weekend and provide more feedback 👍

[vicary]

@cdreier Ended up hashing all variable names to properly dedupe this, but with alpha-4c58a180.0 I think the racing condition is gone now. Please feel free to give it a test, I'll wait for this one before announcing beta.

[cdreier]

no changes for me with the default settings maxAge: 0 - and even with maxAge: 5000 it happens when you hover the modal and the cache refreshes - maxAge: Infinity was the only working solution for now

Bildschirmaufzeichnung.vom.2023-05-13.23-21-04.webm

[cdreier]

and i found another issue in the latest version with the cache normalization - the rerendering does not understandable things... when disabling the normalization, it is working

Bildschirmaufzeichnung.vom.2023-05-13.23-27-11.webm

[vicary]

Yes I can reproduce it, sorry for making a hasty fix and I should test it more thoroughly. Feels like it is triggered by something else, I'll take a look.

For the second one, with updatedPassenger?.id alone, mutation and normalization shouldn't work. A selection of passenger.id and passenger.car.id should be the minimum information for a normalized cache to update the reference.

I have tested with the old versions and the car column in passenger table is still not updating after clicking. What is the expected behaviour?

[cdreier]

you are right, now i can see the same behaviour with the old version 😕

the expectation was, as shown without the cache normalization - the update runs, i can see in the network tab, that the data in the response is correct, and the view should update to the correct car name. I did not find a way to make it work with normalization, even if i fetch all the attributes from the updatedPassenger

[vicary]

I am not sure how deep this is gonna get, let's go as far as we are comfortable with.

I'll show my investigation process below, let me know if you see anything else worth trying.

Click to see investigation progress

Started a new repo and try to reproduce it bottom-up, the goal is to bisect my way to your repo to pin down the root cause.

Your API is returning empty arrays, so I am falling back to my toy API. Reduced the codebase a little bit, but keeping the fetch loop there.

https://github.com/vicary/gqty_rendering/blob/master/src/App.tsx

Screen.Recording.2023-05-15.at.14.00.30.mov

Findings

1. In gqty_rendering we don't need <DataTable /> to make it happen.
2. In gnt it doesn't happen even if grommet elements are used, e.g. Layer, DataTable, Grommet and hpe theme.
3. gqty@^3.0.0-alpha-4c58a180.0 will install older version of alpha because our canary release is using commit hashes, but pinning down to 4c58a180.0 still reproduces it.
4. Removing all grommet related things from gqty_rendering still reproduces it.
5. Using separated useQuery() for both elements can reliably trigger the issue.

Tested Scanarios

1. Rendering a data array with simple <ol>, it only triggers one more query and it is the expected behavior.

   Screen.Recording.2023-05-15.at.00.40.53.mov

2. Replacing the <ol> with <DataTable />, still works.

   Screen.Recording.2023-05-15.at.00.45.47.mov

3. Adding onClickRow, this is where it re-renders on hover. Fetches are firing from my cursor movements, but it's not looping.

   Screen.Recording.2023-05-15.at.00.46.00.mov

4. Adding hpe theme, still no avail.

   https://github.com/gqty-dev/gqty/blob/feat/scoped-query/examples/gnt/app/backflow/page.tsx

   Screen.Recording.2023-05-15.at.13.56.22.mov

I can pin down the reproduction now, working on it.

[vicary]

@cdreier Both minreps are no longer looping with alpha-524ea531.0, we can move on to the mutation issue when you confirm it working.

Note: Don't use caret selection in package.json, e.g. ^3.0.0-alpha..., some old hashes may be lexically sorted upwards and I am gonna replace this flaky release action when we go beta. Instead, use exact version like this "3.0.0-alpha-524ea531.0".

[cdreier]

@vicary i am a bit short in time, but i discovered one thing: it only loops (still) when a proxy is used in the modal!

Bildschirmaufzeichnung.vom.2023-05-15.17-22-20.webm

here, i am using {activePassenger?.name} - if i remove the name, no loops happen

{modalOpen && (<Layer position={'top'} onEsc={() => setModalOpen(false)} >
        <Box pad="medium" gap="medium" overflow="auto">
          <h1>assign {activePassenger?.name}</h1>
          <Cars onClick={c => {
            updatePassenger({
              args: {
                carID: c?.id,
              }
            })
          }} />
        </Box>
      </Layer >)}

editing: added a short clip

and thanks a lot for all your work, this is awesome!

[vicary]

With alpha-085ddbf4.0 your repo is no longer looping, moving on to mutations.

So when you're querying two lists, refetching is the only way.

There are two top-level cache key at play:

1. query.cars, and
2. query.passengers

When you update the car of a passenger, you can only query the new car, thus update its population, leaving the population of the old car dangling/stale.

With the mutation response alone, we have no way to directly update the top level cache key query.cars, post-mutation refetching is required in this scenario.

i am a bit short in time

I'll leave this open until Friday, but you may leave a comment here if you're still hitting anything.

[cdreier]

i can confirm, no more loops! 🚀