Test: noFetch with inline fragments

[TSMMark]

Original issue apollographql/react-apollo#336

@tmeasday @helfer I could not create a failing test here, but here is a passing one.

[apollo-cla]

@TSMMark: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

[tmeasday]

@TSMMark what if you don't include __typename in the relevant queries?

Also, would this not be simpler if the second query was the same as the first? Is that not the issue you were seeing (component unmounted then remounted)

[TSMMark]

I removed the vader portion, and now the queries should be the same.

If I remove __typename, I am told:

Error in test: Tried to log warning or error with message:
You're using fragments in your queries, but don't have the addTypename:
true option set in Apollo Client. Please turn on that option so that we can accurately
match fragments.

So I figured how to do that, and I had to change:

const queryManager = mockQueryManager(
  {
    request: { query },
    result: { data: data1 },
  },
);

to:

const queryManager = createQueryManager({
  networkInterface: mockNetworkInterface({
    request: { query },
    result: { data: data1 },
  }),
  addTypename: true,
});

Now, if I remove __typename from everywhere:

const query = gql`
  query query {
    luke: people_one(id: 1) {
      name
      ... on jedi {
        age
      }
    }
  }
`;

const data1 = {
  luke: {
    name: 'Luke Skywalker',
    age: 50,
  },
};

const queryManager = createQueryManager({
  networkInterface: mockNetworkInterface({
    request: { query },
    result: { data: data1 },
  }),
  addTypename: true,
});

Then I get this error:

Network error: No more mocked responses for the query: query query {
  luke: people_one(id: 1) {
    name
    ... on jedi {
      age
      __typename
    }
    __typename
  }
}
, variables: undefined

Which leads me to believe I need to add __typename to the cached query when priming. Now tests pass again. Still not able to reproduce the original issue.

[tmeasday]

Oh, you should try making the fragment an interface rather than a concrete type like Jedi

[helfer]

@TSMMark I think @tmeasday is right. If you try it on an interface or union, you'll probably be able to reproduce the issue!

[TSMMark]

Hey, I think I got it. result.data is undefined in the new test which uses an interface.

[TSMMark]

What happens now? Anything more I can do?

[tmeasday]

@TSMMark I think this is a bit of a tough issue to solve (se #771).

I think there is a future plan to fetch information from the server about the schema to make these decisions but without that or changes to the graphql spec (__typenames for interfaces and unions?), I'm not sure this can be solved. Interested in ideas however!

[TSMMark]

@tmeasday Shucks. I guess in our app we will have to make some changes to the way we are querying our data because we really need this query to cache properly. Do you have any recommendations on a different way to architect this query so that the query manager will recognize when the data already exists on the client? Server and/or client modifications are all feasible.

[tmeasday]

@TSMMark actually this reproduction isn't quite right as the expectation is incorrect. If you change it to

        assert.equal(result.data.allPeople.length, 2);
        assert.equal(result.data.allPeople[0].name, 'Luke Skywalker');
        assert.equal(result.data.allPeople[0].age, 50);

The test passes as is.

However if you remove the noFetch: true line, then you get a failure (due to re-fetching the query), which I believe is recreating the original problem you've seen.

So in the noFetch case things are working OK, but extra fetches are happening if you don't pass it. Could we update this PR to reflect that?

---

So to be totally clear, my understanding of why this is happening is that when AC is matching Luke Skywalker against the ... nonhuman fragment, there's no way for it to be sure that nonhuman isn't an interface which Luke's type (human) implements (and thus that Luke is incomplete as it is missing the species field).

See https://github.com/apollostack/apollo-client/blob/1054d213035cfda674198ee057195ed9d15b54aa/src/data/readFromStore.ts#L126

It makes sense that noFetch would return a "partial" result (which happens to be the whole result), but that without noFetch that partial result would trigger a re-query to the server.

I'm not sure what the best workaround would be, but one approach would be to implement your own cache checking logic by first querying with noFetch and then doing a legitimate query if no results are returned.

[helfer]

We're now keeping track of this and related issues on #1337