Fetching similar data with two different queries gives data undefined

[jfulse]

Hello again,

I might be misunderstanding something here, but I have issues using ErgonoMockedProvider with several queries fetching the same object. Here's a minimal reproduction adapted from the example in the Readme:

import { render } from '@testing-library/react';
import { ErgonoMockedProvider } from 'graphql-ergonomock';
import { gql, useQuery } from '@apollo/client';

const schema = gql`
  type Shape {
    id: ID!
    returnInt: Int
    returnString: String
    returnBoolean: Boolean
  }

  type Query {
    getShape: Shape
  }
`;

const QUERY = gql`
  query MyQuery {
    getShape {
      id
      returnString
      returnBoolean
    }
  }
`;

const ANOTHER_QUERY = gql`
  query AnotherQuery {
    getShape {
      id
      returnInt
      returnBoolean
    }
  }
`;

const MyComponent = () => {
  const { loading, error, data } = useQuery(QUERY);
  const { loading: anotherLoading, error: anotherError, data: anotherData } = useQuery(ANOTHER_QUERY);
  if (loading || anotherLoading) return <p>Loading</p>;
  if (error || anotherError) return <p>Error</p>;

  return (
    <div>
      MyComponent.
      <div>String: {data.getShape.returnString}</div>
      <div>Boolean: {data.getShape.returnBoolean.toString()}</div>
      <div>Int: {anotherData.getShape.returnInt}</div>
    </div>
  );
};

test('MyComponent can render the query results.', async () => {
  const mocks = {
    MyQuery: {
      getShape: { returnString: 'John Doe' },
    },
    AnotherQuery: {
      getShape: { returnInt: 5 },
    },
  };
  const { findByText } = render(
    <ErgonoMockedProvider schema={schema} mocks={mocks}>
      <MyComponent />
    </ErgonoMockedProvider>,
  );
  expect(await findByText(/String: John Doe/)).toBeVisible();
  expect(await findByText(/Boolean: (true|false)/)).toBeVisible();
  expect(await findByText(/Int: 5/)).toBeVisible();
});

The test fails because data is undefined; if I remove the second query it works. The problem is the same if I don't supply any mocks.

Thanks!

[victor-guoyu]

Thanks for raising this issue! :)

I'm able to reproduce the bug with the setup you provided. Will start working on a fix later today.

[jfulse]

Great, thanks a lot!

[jfulse]

Hello!

Let me know if would like me to test the issue on a fix branch etc. If you don't have time to work on a fix at the moment I could also try to take a look?

[joual]

I'm thinking that the same call twice will yield different data until #20 is fixed. Which isn't really hard to do, we just need to get around to it.

[victor-guoyu]

Hi, Sorry for the late reply again..! 😬

So here is what I have gathered so far. if you pass fetchPolicy: "no-cache", as part of the useQuery option. You'll able to see the mock data as expected. This is not really a work around by any mean, but I think it might give us a clue that something is going on around the apollo cache.

That being said, please feel free to raise a PR / share ideas with regard to this issue.

Thanks

[victor-guoyu]

Here is a json dump of the store from the updated test case

 {
      'Shape:2881188': [Object: null prototype] {
        id: '2881188',
        __typename: 'Shape',
        returnString: 'Pepper Church Tongue'
      },
      ROOT_QUERY: [Object: null prototype] {
        __typename: 'Query',
        returnShape: { __ref: 'Shape:4749468' }
      },
      'Shape:4749468': [Object: null prototype] {
        id: '4749468',
        __typename: 'Shape',
        returnInt: 78
      }
    }

I think the root cause for this is that we are generating random id for the same query request (same query params).

In the above example, getShape query does not take any arguments, therefore, the InMemoryCache won't able to leverage that as part of the cache id. When the second query finishes, the cache store will update the reference of the returnShape query to the latest mock data which is 4749468.

This explains why we getting undefined for the first query, since shape of the data doesn't match the query, and bypassing cache will mitigate the problem.

Just following @joual's suggestion here, I think if we can have a deterministic id returned based on the query args, we would able to take advantage of the data normalization and let the cache store handle the data merging part.

cc @jfulse

[jfulse]

Thanks for the response!

What you say makes sense, I'll think about it and see if I have anything to add.

[jfulse]

Hello!

After some digging I have some more insight into the problem in our app - and realised that it probably isn't an issue with this library after all, but rather a difficulty in using it (at least in our use case).

This is my understanding at the moment:

I think that the test above doesn't actually expose a bug but actually is expected behaviour. The issue is that the Apollo cache stores finished queries with the key queryNode(variables): .... This means that the results of QUERY and ANOTHER_ QUERY, both having no variables, are stored as getShape: .... They therefore overwrite one another. If we instead use queryShape and include an id the queries are stored as queryShape({"id": "1"}): ... etc and no longer overwrite each other.

The proposed queries in the reproduction above would be an issue in an actual application as well, and doesn't actually represent our use case - I had simplified too much to get a minimal reproduction.

---

So here is an updated understanding of our issue:

In our app we have several queries to the same node with the same variables but fetching different fields. It looks like the apollo cache then merges the data from these queries, so that if we run e.g. query { queryShape(id: '1') { id returnInt } } and query { queryShape(id: '1') { id returnString } } the cache will store something like

'Shape:1': {
  id: '1',
  returnInt: 12,
  returnString: 'Hello',
}

where I have assumed that we make sure that requesting a shape with id = '1' will result in a Shape with id: '1' as can be achieved by supplying some mocks that look something like

const mocks = {
    QueryOne: {
      queryShape: (_, { id }) => ({ id }),
    },
    QueryTwo: {
      queryShape: (_, { id }) => ({ id }),
    },
}

And this is the crux of the issue: if we don't supply these mocks the cache is in trouble, ending up with something like

'Shape:123': {
  id: '123',
  returnInt: 4,
},
ROOT_QUERY: {
  'queryShape({"id":"1"}): { __ref: 'Shape:456'}',
},
'Shape:456': {
  id: '456',
  returnString: 'Hi',
}

Where the results from the two Shape queries are not merged, and the root query only refers to the last of them. Running this in our app gives the following effect: If one component runs the first query and another component, rendering later, runs the second, the data in the first component becomes undefined after the second is finished fetching (I'm not 100% sure of the mechanism here but it seems plausible).

This undefined is the one showing up in the original issue.

---

So the solution is, clearly, to supply these kinds of mocks that makes sure the result id is the same as the one in the variables. But this is a bit awkward when there are lots of queries with different names that query the same node; we have to supply a mock for each of them. Also if we later add another query we have to remember to add a mock for it as well or it will break existing tests.

This leads me to the suggested feature: how about some sort of setting that automatically returns the same id as in the variables if found?

If you are interested in such a feature I could make a draft PR for it. Of course I understand if you view this as out of scope for the library, if for example you consider our use case unusual.

Cheers, Jørgen

EDIT: A counterpoint is that the suggested feature treats id as a special field, which I think is pretty common in graphql apps but not universal. So maybe instead a feature where you can supply a list of fields that will automatically be the same in the result as in the variables? Possibly differentiated by query node. In the example above the setting therefore could be something like

<ErgonoMockedProvider copyFieldsFromVariables={{queryShape: ['id']}} ... />

And this would then affect both QueryOne and QueryTwo if they both obtain queryShape.

[jfulse]

Hello again!

Have you had the chance to consider the proposal above? I will start working on it in a forked repo and see how involved an implementation would be.

[joual]

Hey @jfulse I am sorry for the delay in getting back to you. I fully appreciate the in-depth feedback you provided here.

Victor and I discussed and ultimately I think it might be hard to have a default feature for the behavior you mention. It really depends on the experience provided by the application. Your assumption is that a particular object would be returned more than once, which applies for a CRUD details page for example, but then on list-type pages or in more complex scenarios this assumption starts to break down.

Because of the above, I think this would be out of scope for this library. In the end, the issue you noticed makes the library slightly less magical but there are still a lot of benefits to grasp from it.

I'll document this gotcha in the main README and add a link to this issue with your findings.

[jfulse]

That is understandable. I appreciate your answer, thanks for the feedback and a great library!

[jfulse]

For future reference: It's possible to avoid the gotcha without too much inconvenience by using a combination of resolvers and mocks.