apollo-client: dataIdFromObject can result in missing data
This is a more general case of #818 in that if the result of a query made later in time changes a linked to object, the original query will fail with “Perhaps you want to use the returnPartialData option?”. In this test case after the first query has finished we issue a slightly different second query that changes the id of the linked object.
import * as chai from 'chai';
const { assert } = chai;
import gql from 'graphql-tag';
import { QueryManager } from '../src/core/QueryManager';
import mockNetworkInterface from './mocks/mockNetworkInterface';
import { createApolloStore, ApolloReducerConfig } from '../src/store';
import subscribeAndCount from './util/subscribeAndCount';
const defaultReduxRootSelector = (state: any) => state.apollo;
const friendAgeQuery = gql`
query($id: ID!) {
person(id: $id) {
id
friend {
id
age
}
}
}
`;
const friendAgeData1 = {
person: {
id: 1,
friend: {
id: 2,
age: 12,
},
},
};
const friendAgeData2 = {
person: {
id: 1,
friend: {
id: 3,
age: 13,
},
},
};
const friendQuery = gql`
query($id: ID!) {
person(id: $id) {
id
friend {
id
name
}
}
}
`;
const friendData = {
person: {
id: 1,
friend: {
id: 3,
name: 'Chris',
},
},
};
describe('dataIdFromObject', () => {
it.only('handles missing shared object fields', (done) => {
const mockedResponses = [{
request: { query: friendAgeQuery, variables: { id: 1 } },
result: { data: friendAgeData1 },
}, {
request: { query: friendQuery, variables: { id: 1 } },
result: { data: friendData },
}, {
request: { query: friendAgeQuery, variables: { id: 1 } },
result: { data: friendAgeData2 },
}];
const reducerConfig: ApolloReducerConfig = {
mutationBehaviorReducers: {},
dataIdFromObject: (result: any) => {
if (result.id) {
return result.id;
}
return null;
},
};
const store = createApolloStore({ config: reducerConfig });
const queryManager = new QueryManager({
networkInterface: mockNetworkInterface(...mockedResponses),
store,
reduxRootSelector: defaultReduxRootSelector,
addTypename: false,
reducerConfig,
});
const friendAge = queryManager.watchQuery({
query: mockedResponses[0].request.query,
variables: mockedResponses[0].request.variables,
});
const friend = queryManager.watchQuery({
query: mockedResponses[1].request.query,
variables: mockedResponses[1].request.variables,
});
subscribeAndCount(done, friendAge, (count, result) => {
if (count === 1) {
subscribeAndCount(done, friend, (innerCount, innerResult) => {
if (innerCount === 1) {
assert.deepEqual(innerResult.data, friendData);
}
});
}
if (count === 2) {
assert.isTrue(result.loading);
}
if (count === 3) {
assert.deepEqual(result.data, friendAgeData2);
done();
}
});
});
});
About this issue
- Original URL
- State: closed
- Created 8 years ago
- Comments: 25 (18 by maintainers)
That is unfortunate since that requires tight coupling between different parts of the application. One query shouldn’t need to know what fields other parts of the application are requesting. We have a highly connected graph of data which would make this workaround very difficult to manage.
I agree. This isn’t REST. I am not flushing the entire object downstream with every mutation, so I don’t care if some fields happen to be outdated. If a newer query fetches part of an existing object, the properties that weren’t fetched could be marked as “stale” so that if a different view requested those fields apollo would automatically run the query rather than only returning the cache to “freshen” the data. I feel like there are a lot of work arounds here. From a systems design standpoint, forcing coupling between queries between various UI components to avoid errors or missing data seems like a non-negotiable. Stale data is an easier problem to deal with and solve 😃
I agree that tight coupling is bad, but you guys see the problem, right? If we have a normalized cache, then we expect an update to one query to affect another query if they have overlapping data. We can choose several ways of dealing with these overlapping fetches:
Of all the options above, I think the first one is the best as an initial step. If you use polling, you still get eventual consistency. And if you need better consistency guarantees, then you’ll just have to couple your queries for now. It’s not like this affects every query in your app, just the overlapping ones.
I think this is resolved now - our chosen approach was to mark queries as stale if they end up in a situation where they no longer have all of the data they need, you can read about it here: https://github.com/apollographql/apollo-client/pull/1306
I will reiterate that a solution that requires having to manually modify other queries in the system is unworkable.
@thebigredgeek actually, merging isn’t quite as easy as I thought. If you look carefully at @NeoPhi 's description you can see that the problem isn’t that an existing object is changed in the cache (friend with id 2), it’s that the reference is updated to a new object (friend with id 3), but that new object wasn’t requested with all the fields for the previous object.
Merging doesn’t make sense in this case, so in my opinion the best option would be to invalidate the original query and refetch it. It’s very tempting to want to use dynamically composed queries in this case, but I think it would be the wrong course of action, unless we could compute those overlapping queries at compile-time (and thus effectively make them static).
@helfer I wonder if instead of trying to solve the general case, a good first approach would be to better surface what is going on to the consumer. If I specify
returnPartialDataand Apollo thinks that the query is cached and would not fetch it again, perhaps a newNetworkStatusofreadyPartialDatacould be added. The receiving component could then make the decision to manuallyrefetchif desired. The general idea being to tell the consumer, I’ve given you all the data I have and I’m not planning to load anymore. I could see this status being helpful when using thenoFetchoption as well.Much like @NeoPhi I fear that this is another piece of the design which requires a growing app to tightly couple its query usage across what often may be a large, de-centralized UI. It doesn’t seem ideal for queries to be responsible for knowing anything whatsoever about the needs of any other queries in the application (or even their names, another problem I’m having when it comes to the design of updateQueries & refetchQueries). It seems that much of the benefit of GraphQL (your data is your API; UI components can cleanly notate their isolated data needs) fades as these patterns begin to rely on queries being so globally orchestrated.