gqty-dev / gqty

The No-GraphQL Client for TypeScript
https://gqty.dev
MIT License
935 stars 26 forks source link

Bug: Infinite looping GraphQL requests, reported from faustjs #558

Open PabloSzx opened 2 years ago

PabloSzx commented 2 years ago

Copying issue reported in https://github.com/wpengine/faustjs/issues/723#issuecomment-1015833199 thank you @mediashane

Hi @PabloSzx ! We've set up a Github repo and Vercel deploy that reproduces the bug. I'll also attach a video that shows steps to repro and the bug itself in action.

Github repo: https://github.com/mediashane/graphql-bug After cloning, create a .env file in the base directory of the project and copy + save the contents of .env.local.sample into it. This will configure your local development environment to point towards the live WordPress backend. Then just npm install, npm run build and npm run start.

Here is a Vercel deploy of the repo linked above, where you can jump straight to reproducing the bug without cloning the repo: https://graphql-bug.vercel.app

Steps to reproduce it in Vercel (staging):

  1. Navigate to https://graphql-bug.vercel.app,
  2. Open Network Dev Tools tab
  3. Click “Our Story”
  4. Click “Rugs”
  5. Click “From the Farm”

If you don't see the looping GraphQL requests the first time, return to the home page, do a cache clearing reload and follow the sequence again. There are other occasions that this looping starts, but this is a sequence that we've found reproduces the bug consistently.

We're using the plugin WP GraphQL to expose our WordPress data, and Faust + GQty to do our queries and generate our schema locally. We think this issue is closely related to the issue you encountered and solved here: gqty-dev/gqty#520

Here is the video: https://user-images.githubusercontent.com/14046192/150017319-3c419d6a-5484-4129-b8bf-49802477b033.mp4

Thank you in advance for all of your work, and let us know if there's anything else we can do to help!


I could confirm this bug, and doesn't seem to be a bug on server side, the weirdest thing is that this bug doesn't happen on development mode, only after building Next.js and starting on production mode, this makes debugging way slower since making changes on node_modules takes a whole build+start

theodesp commented 2 years ago

Looking at the repo containing the bug It seems that the code triggers the loop originates from this component: https://github.com/mediashane/graphql-bug/blob/2b1a20dab7214377d56e7e163708d4a88e43a01c/src/koa-framework/AcfModules/AcfModules.tsx#L13-L23

Screenshot 2022-01-24 at 12 59 21

It then triggers something and goes back to the same susbscibe-resolve section here:

https://github.com/gqty-dev/gqty/blob/237fb26cabeab75ddb4475b671b6490faa5de1d5/packages/react/src/common.ts#L417-L447

The stack trace loops again and again with this component.

I suspect it's something to do the way it renders the React Component again and again.

theodesp commented 2 years ago

I also suspect that because it uses queries from the previous page, it triggers an effect like this:

  1. A page is loaded. GQty triggers a schedule to request a query. Query is pending.
  2. Before the query finishes the user clicks on another page. React recycles all relevant components and loads the new Page.
  3. GQty somehow gets stuck in a loop trying to resolve the pending query from the previous page. Until that happens it does not see the new subscriptions from the new pages.

You can clearly see that stale promise by inspecting the state of the QueryFetcher.

Screenshot 2022-01-24 at 13 56 20

It loops here requesting parameters from a different page.

theodesp commented 2 years ago

The code also seems to loop around this block:

https://github.com/gqty-dev/gqty/blob/237fb26cabeab75ddb4475b671b6490faa5de1d5/packages/react/src/common.ts#L106-L142

Screenshot 2022-01-24 at 14 21 56
theodesp commented 2 years ago

Looking at the code here it looks like it falls into the troublesome case of data-selections-conditionals: https://gqty.dev/docs/react/troubleshooting#data-selections--conditionals

The AcfModules component is basically the map part of the example. I've rewriten the component to bring all related queries together:

import React from 'react';
import { client, RugIdType } from '../../client';

export interface Props {
  modules?: Array<any>;
  isLoading: boolean;
}
export default function AcfModules({ isLoading }: Props) {
  const rugDetails = client.useQuery().rug({
    id: `/${pageUri.join('/')}`,
    idType: RugIdType.URI,
  });
  const mods = client.usePage().pageBuilder.modules;
  const modss = rugDetails?.rug?.modules;

  // if query contains custom post type 'rug', use the Rug ACF modules
  // otherwise use the Page Builder ACF modules
  const modules = pageUri.includes('rug')
    ? Object.values(modss)
      .filter((e) => typeof e !== 'string')
      .sort((a, b) => {
        return a.order - b.order;
      })
    : mods; // **<--- HERE: Reacting data to be mapped**
  return (
    <>
      {modules.map((module, index) => { **<--- HERE: Mapping starts here**
        const componentName = module?.__typename?.split('_')?.pop() ?? module?.[0]?.__typename?.split('_')?.pop();
        if (!componentName) {
          console.error('[KOA]', 'AcfModules', 'Component name is empty');
          return;
        }
        const moduleData = module.$on ? module?.$on[module?.__typename] : module;
        return renderComponent(componentName, moduleData, index, isLoading);  // **<--- HERE: Data selection behind switch statements**
      })}
    </>
  );
}

The modules constant is the reactive data that change based on the page the user visits. For example it may be:

{"__typename":"Page_Pagebuilder_Modules_ContactBanner"}{"__typename":"Page_Pagebuilder_Modules_CenteredText"}{"__typename":"Page_Pagebuilder_Modules_ContactList"}{"__typename":"Page_Pagebuilder_Modules_ContactList"}

For the Contact-us page and:

{"__typename":"Page_Pagebuilder_Modules_HeroCenterTop"}{"__typename":"Page_Pagebuilder_Modules_FarmPostsList"}

For the From-the-farm page

The renderComponent is basically the acfRenderComponent function which is a big switch statement. The problem here is that they all refer to different components types as they depend on the componentName parameter which is essentially the module?.__typename string.

@PabloSzx is there a recommended way to deal with switch statements of that kind? What do you think?

theodesp commented 2 years ago

I also managed to replicated it more consistently. It looks like in the switch statement here: https://github.com/mediashane/graphql-bug/blob/main/src/helpers/renderComponent.tsx

Any component that shares the same properties with a different component in a switch statement picks up their properties and tries to request them under a different Query Fragment. For example:

The components HeroLeftJustified with HeroRightJustified with ContactBanner share the same image property. Now for some reason when you click on the pages it will pick up the properties from a previous component. For example in the our-story page it will request the following modules:

{"query":"query($id1:ID!$idType2:PageIdType){page0:page(id:$id1 idType:$idType2){__typename id pageBuilder{__typename modules{__typename ...on Page_Pagebuilder_Modules_HeroRightJustified{textHeadline textColor textSubline textMediaLabel image{__typename id mediaItemUrl}mediaIcon{__typename id mediaItemUrl}}...on Page_Pagebuilder_Modules_TwoColumnContent{includeParagraph textHeadline textParagraph image{__typename id mediaItemUrl}}...on Page_Pagebuilder_Modules_OneColumnContent{includeParagraph textHeadline textParagraph textColor image{__typename id mediaItemUrl}}}}}}","variables":{"id1":"our-story","idType2":"URI"}}

Now before the page finishes loading you click on the Contact-us page. It will try to use some of the properties of the Page_Pagebuilder_Modules_HeroRightJustified module:

{"query":"query($id1:ID!$idType2:PageIdType){page0:page(id:$id1 idType:$idType2){__typename pageBuilder{__typename modules{__typename ...on Page_Pagebuilder_Modules_HeroRightJustified{image{__typename id mediaItemUrl}}...on Page_Pagebuilder_Modules_CenteredText{textColor}}}id}}","variables":{"id1":"contact-us","idType2":"URI"}}

However the HeroRightJustified is not within the list of available modules for that page. The correct module is the Page_Pagebuilder_Modules_ContactBanner. If you refresh the Contact-us page you get the correct query:

{"query":"query($first1:Int$where2:RootQueryToMenuItemConnectionWhereArgs$id3:ID!$idType4:PageIdType){menuItems0:menuItems(first:$first1 where:$where2){__typename nodes{__typename id label}}page0:page(id:$id3 idType:$idType4){__typename pageBuilder{__typename modules{__typename ...on Page_Pagebuilder_Modules_ContactBanner{image{__typename id mediaItemUrl}}...on Page_Pagebuilder_Modules_CenteredText{textColor}}}id}}","variables":{"first1":100,"where2":{"location":"PRIMARY"},"id3":"contact-us","idType4":"URI"}}

Notice that both Page_Pagebuilder_Modules_ContactBanner and Page_Pagebuilder_Modules_HeroRightJustified have the same query payload:

image{__typename id mediaItemUrl}

This means that somehow GQty uses the wrong Query Fragment ( it picks up Page_Pagebuilder_Modules_HeroRightJustified because it shares the same query payload and it was recently used).

On the other hand you can also check the CardList Component as it does not share any other properties with the other Component in the switch. If you remove all elements in the switch and render only the CardList component and any other component (for exampleHeroRightJustified) then it won't trigger a loop because there is no common property within the CardListcomponent that can be used in a Query Fragment of any another Component.

I hope that helps.

CesarBenavides777 commented 2 years ago

@theodesp Having the same issue here: https://github.com/NoisyTrumpet/TexasYesProject/blob/main/src/features/BlockReturner.tsx So you're saying is that maybe if we change the generic image or title fields to something more specific like heroTitle or something like that might help?

jordanmaslyn commented 2 years ago

@PabloSzx is there a way to globally disable cache for GQty to bypass this issue?