Closed hinok closed 7 years ago
Hey @hinok 👋
This error only should occur when you have made a query with an id
field for a portion, then made another that returns what would be the same object, but is missing the id
field.
We are planning on making this just a warning (instead of throwing) but it does help a lot to keep the id
field included in queries that share fields
Are you guys planning on converting this to a warning
rather than error as @jbaxleyiii mentioned. We're getting this too - fine to bring back id
's but a warning would be preferable.
This id
issue caused some of our production pages failure too. I expect Apollo could do more than throw an error and break the whole page. An explicit configuration that indicates using a field like id
as the key to share the resources would be good.
Should this issue be re-opened, and only closed after it's made to be just a warning?
Currently, if you make a change to one query by adding an ID field, you could be breaking other parts of your app that you haven't touched at all (because they use queries without ids and you used a query with an id)
👍 this is a blocking error for me. Please fix
We can't use it in our projects with having this error.
any update on this issue @jbaxleyiii ?
It seems like missing __typename on any level along side id field will also trigger this error
I have the same problem here, no solution so far?
Have a same problem :) Would like to see some solution...
+1
+1
+1
+1
For what it’s worth, I don’t mind the current behavior, as long as it can be caught compile-time using a tool like apollo-codegen
. The real issue here is that the scenarios that induce this error can be complex and missed by unit tests and quick human checks, but exist in real world usage.
+1
+1
+1
+1
+1
+1 on updateManySomething
You may forgot to select id
in your query.
post(id: $postID) {
id
comments(pagination: $pagination) {
edges {
node {
content
}
cursor
}
pageInfo {
endCursor
}
}
}
}```
PSA: This can also happen if you try to write an object without an ID, as happened to me when I created an unauthenticated user by mutating with {accessToken}
instead of {accessToken, id, ...rest}
I've found a strange bug related to this issue.
I have a custom element <blink-feed>
which subscribes to this query like this one when it connects to the DOM:
query BlinkFeed (...) {
orientation @client
# user :: User
user @client { id locale avatar }
blinks( ... ) @connection(key: "feed") {
pageInfo { hasNextPage hasPreviousPage }
edges {
cursor
node {
id
...
# owner :: User
owner {
id
avatar
nickname
iFollow
}
}
}
}
}
Note that the User
type is queried both client-side and server-side.
With the query as shown above, the components render without error, however, the client-side User
object is overwritten with its default values once the network query resolves.
If I omit the id from either the user
or owner
fields, the component errors on the first load, but then renders correctly on subsequent reloads (via apollo-cache-persist)
This same behaviour occurs if I alias the client-side user type, e.g., calling it Session
instead of User
Find this stack overflow can resolve my problem.
Is this fixed? I asked a similar here?=: https://stackoverflow.com/questions/53788200/fix-error-writing-to-store-for-query-queryname
same problem
query PreviewResourceSearch($cat: [String!], $zips: [String], $page:Int) {
searchResourcesPaged(categories: $cat, zipCodes: $zips, page: $page, perpage: 3) {
resources {
id
internalId
name
phone
streetAddress
city
state
}
numResults
numPages
keywords
}
}
function loadMore(data, path, page) {
console.log(page)
return data.fetchMore({
variables: {
page: page,
},
updateQuery(previousResult, { fetchMoreResult }) {
const more = fetchMoreResult[path].resources;
return {
[path]: {
resources: [ ...previousResult[path].resources, ...more],
numPages: previousResult.numPages,
numResults: previousResult.numResults,
keywords: previousResult.keywords,
__typename: previousResult.__typename
},
};
},
});
}
{…}_a: undefined_c: Array []_d: true_h: 2_n: false_s: 2_v: ErrorcolumnNumber: 36fileName: "http://localhost:3000/static/js/36.chunk.js"lineNumber: 29644message: "Error writing result to store for query:
query PreviewResourceSearch($cat: [String!], $zips: [String], $page: Int) {
searchResourcesPaged(categories: $cat, zipCodes: $zips, page: $page, perpage: 3) {
resources {
id
internalId
name
phone
streetAddress
city
state
__typename
}
numResults
numPages
keywords
__typename
}
}
Store error: the application attempted to write an object with no provided typename but the store already contains an object with typename of EsResourceSearchType for the object of id $ROOT_QUERY.searchResourcesPaged({\"categories\":[\"Bill\"],\"page\":1,\"perpage\":3,\"zipCodes\":[]}). The selectionSet that was trying to be written is:
searchResourcesPaged(categories: $cat, zipCodes: $zips, page: $page, perpage: 3) {
resources {
id
internalId
name
phone
streetAddress
city
state
__typename
}
numResults
numPages
keywords
__typename
}"stack: "./node_modules/apollo-cache-inmemory/lib/writeToStore.js/StoreWriter</StoreWriter.prototype.writeFieldToStore@http://localhost:3000/static/js/36.chunk.js:29869:17
./node_modules/apollo-cache-inmemory/lib/writeToStore.js/StoreWriter</StoreWriter.prototype.writeSelectionSetToStore/<@http://localhost:3000/static/js/36.chunk.js:29733:11
./node_modules/apollo-cache-inmemory/lib/writeToStore.js/StoreWriter</StoreWriter.prototype.writeSelectionSetToStore@http://localhost:3000/static/js/36.chunk.js:29723:5
./node_modules/apollo-cache-inmemory/lib/writeToStore.js/StoreWriter</StoreWriter.prototype.writeResultToStore@http://localhost:3000/static/js/36.chunk.js:29695:14
./node_modules/apollo-cache-inmemory/lib/inMemoryCache.js/InMemoryCache</InMemoryCache.prototype.write@http://localhost:3000/static/js/36.chunk.js:28500:5
./node_modules/apollo-client/data/store.js/DataStore</DataStore.prototype.markUpdateQueryResult@http://localhost:3000/static/js/36.chunk.js:32705:5
./node_modules/apollo-client/core/ObservableQuery.js/ObservableQuery</ObservableQuery.prototype.updateQuery@http://localhost:3000/static/js/36.chunk.js:30924:7
./node_modules/apollo-client/core/ObservableQuery.js/ObservableQuery</ObservableQuery.prototype.fetchMore/<@http://localhost:3000/static/js/36.chunk.js:30809:7
run@http://localhost:3000/static/js/36.chunk.js:43131:22
notify/<@http://localhost:3000/static/js/36.chunk.js:43152:7
flush@http://localhost:3000/static/js/36.chunk.js:40274:9
"type: "WriteError"<prototype>: Object { … }<prototype>: Object { … } Resources.js:118
except it does update the cache, but the component does not receive the new props untill a different action takes place. say a mutation. I noticed this because i have an unrelated query that polls the server, when the query runs this component updates and shows the load more results.
I have also tried to refetch from the cache directly
setTimeout(() => {
data.refetch({
options:{
fetchPolicy: 'cache-only'
}
});
}, 0);
Not sure if it does any good to comment on closed issues, but at the least I think a more helpful warning would be great here
having this error
Yeah me too, not sure why these issues keep getting closed. BTW this is a bug. If you require id
field to be present in the query and you cannot cache it if it's not there then sensible thing to do would be to show a warning. Not an error.
Same error
@capaj @Abhijeetjaiswalit just put the ID in all the Types, and that will solve the issue
@alacret what if I don't have ID on some of my object types?
I am sorry to say this, because I totally respect the work apollo team has done on apollo-client, but this issue is a shitshow. Excuse the profanity, but it seems that apollo team doesn't think this is an actual bug. yet even the most experienced people like https://twitter.com/thekitze/status/1113871705498370049 keep running into problems here.
They should either acknowledge this as a bug or they should amend documentation with a big red warning that you absolutely need to include id
or _id
field in all of the object types you query.
The docs only mention regarding this topic I found is here: https://www.apollographql.com/docs/react/advanced/caching#normalization
and there it is claimed that there is a fallback. From that I would expect that cache handles even such results correctly and without any issues whatsoever.
For all the people like me who still get this error daily on production apps where they can't magically add id
field everywhere I've wrote this terrible hack that you can add to your apps that will at least keep your react app from breaking:
window.onunhandledrejection = async (error) => {
if (error.reason && error.reason.message && error.reason.message.match(/Error writing result to store for query/)) {
/**
* THIS is nasty and hacky, but it's the best mitigation for the apollo-client problem that we've got.
* If we can't stop apollo-client to choke on these errors so often, we should just migrate every graphQL query/mutation to urql
*/
apolloClient.cache.reset(),
const rootEl = document.getElementById('appRoot') // obviously modify according to your DOM
console.log('apollo caches reset, remounting the app')
ReactDOM.unmountComponentAtNode(rootEl)
renderApp()
}
}
@capaj That's a super hack bro.
I think that just apollo-client is not so battle-tested as other libraries. I'm assuming that the decision was based on the fact that using id
or _id
is pretty much a standard nowadays, and this was more relevant for the implementation of the cache than anything else.
But it makes sense that by now this should be just a warning and not an error, or maybe allow to skip any caching check, or to allow setting a different field name as the key for the cache.
like @alacret suggests to @capaj , this might be the right direction .. still needs some tweaking maybe.
query {
object @connection(key="someobject") {
someobject {
someField
}
}
}
This error is odd. It made one of our queries fail despite the fact that the query itself was just fine. There are valid cases for making similar nested queries where one has id and the other doesn't. It's good that we are warned that Apollo is unable to update the store but it shouldn't break the app.
will there be a warning published? it's not practical for all objects to have an id attribute
This is a major issue for so many people and has been open for almost 2 years? What's going on with this? Just curious?
Is there a clear fix for this that (maybe) can simply show a warning if you try to run a graphl query without id
defined? A bunch of people on my team have independently stumbled across this issue and it always takes about a day to figure out and resolve. If apollo-client could at least throw a warning when there is no id set on the query, at least we could create an automated ticket warning the developer that their query may (will) cause a bug. I/we would be very happy to support any efforts in this direction.
For all the people still experiencing this issue-I've finally found out what is causing this issue. What helped to shed some light on it was switching from apollo-cache-inmemory
to apollo-cache-hermes
.
I experimented with Hermes hoping to mitigate this ussue, but unfortunatelly it fails to update the cache the same as apollo-cache-inmemory
. What is curious though is that hermes shows a very nice user friendly message, unlike apollo-cache-inmemory
. This lead me to a revelation that cache really hits this problem when it's trying to store an object type that is already in the cache with and ID, but the new object type is lacking it. So apollo-cache-inmemory
should work fine if you are meticolously consistent when querying your fields. If you omit id
field everywhere for a certain object type it will happily work. If you use id
field everywhere it will work correctly. Once you mix queries with and without id that's when cache blows up with this horrible error message.
This is not a bug-it's working as intended, it's just not documented anywhere-see this spec.
I've added a warning to the caching doc here: https://github.com/apollographql/apollo-client/pull/4895
Be careful include __typename
or avoid it if you're testing with MockProvider. But in some cases you have to include __typename
allowing it in the MockProvider
@wfsovereign , your answer enlighten a better picture, that is, just put id on every query or subquery that has an id field. Thanks 👍🏼
The Stackoverflow answer pointed to in this issue is actually the sensible one and actually solves this issue, which is not a bug by the way. I think this needs to be documented somewhere. In a nutshell: make sure to add id to all subselections of your queries this makes sure the cache identifies queries and selections properly... notice this has nothing to do with following a certain pattern whether you put ids somewhere else or not, it's just that simple; put ids for sub-selections and to add to that especially for ones that recurse!
I have a similar issue when an alias is used for the ID
field:
query GetService ($id: ID!) {
service(id: $id) {
image
# list of account
providers {
id: provider_id
firstName: first_name
lastName: last_name
}
}
self: account {
# if I don't use an alias here it worked even when MyProviderAccount didn't include the `provider_id`
id: provider_id
}
}
query MyProviderAccount {
account {
provider_id # using the same alias here would fix the problem
first_name
last_name
}
}
When I try to run the MyProviderAccount
query after GetService
I would get the error:
Store error: the application attempted to write an object with no provided id but the store already contains an id of ProviderType:501 for this object
I wasn't expecting that when I use an alias for a field, I have to use the same alias everywhere so that cache would work
Edit: It's related to the id
key not the alias. Using different aliases would still resolve data from cache if it's available:
query QueryA {
account {
provider_id
firstName: first_name
}
}
query QueryB {
account {
provider_id
name: first_name
}
}
Running the above queries in sequence - first A then B (or reverse) would not make a 2nd request, but resolve the 2nd query from cache
@kidroca why not use a fragment with an alias .. that way now you only need to use the fragment everywhere.
I am using a fragment where it makes sense. For example the MyProviderAccount -> account
. But there are cases where I just want a few fields like in self
-> I use the id to highlight the current user account in the providers
list
I've read more about cache and looks like this is the correct behavior, because the cache doesn't exactly know which key is the id
. It has a default logic to assume that if there is object.id
then id
is the ID
For this reasons I've decided that you should either always alias all ids in a generic way (id: provider_id
, id: service_id
) or never alias ids, which seems the easier option
When the ID key is different across your Types (provider_id, service_id), it's best to provide a dataIdFromObject(object)
function which can map the id => getIdFor(object.__typename)
like the example provided in the docs. And you can still fallback to the default dataFromObject
function in case an alias like id: provider_id
was used
Also have in mind that if you're dealing with numeric ids - they probably aren't unique across types so the returned data for dataIdFromObject
should be something like ${object.__typename}[${id}]
(e.g. "Service[10]"
) and not just the id
I wanted to chime in and report that this issue happens in the official example of the Hacker News clone: https://www.howtographql.com/basics/0-introduction/
The solution was to indeed modify the queries and add id
fields to everything that was related to subscriptions.
It does make sense that it works as intended because it's essentially replacing data in the cache by their identifier keys, but the error message is lacking IMO.
Edit: Removing the id
fields from the queries also works in the same Hacker News example, as @capaj says, the important thing is to match the query structures.
I ran into this today, I added a new subquery somewhere and it broke my whole app because it's a Next.js app which uses both SSR and CSR and depending on the cached data it would randomly crash.
For instance, refreshing a page with cmd+shift+r to erase cache would work fine (SSR) but then when navigating to another page (CSR) it would randomly crash THE WHOLE THING, depending on which page is loaded.
This definitely SHOULD NOT throw an exception, but rather a warning (as proposed in 2017). And it should be made obvious in the docs that all query must have an "id" field for Apollo to work properly.
Also, it's very hard to debug. Especially because the error message isn't always Store error: the application attempted to write an object with no provided id but the store already contains an id of
but most of the time only Network error: Error writing result to store for query
with the whole query behind... Quite complicated to understand what's the issue at hand with such cryptic error messages.
I eventually added id
everywhere, in all fragments and queries to avoid this, and it works fine now.
Totally agree with @Vadorequest! Came here to say something similar
This eventually became enough of a pain in the neck that we wrote a validation rule/linter to ensure the "id" field was present in every possible GraphQL selection set. This rule is intended to be used with the GraphQL validation APIs.
Our implementation is wrapped up in some other tooling but these are the relevant bits:
function IdOnObjectSelectionSetRule(context: ValidationContext): ASTVisitor {
/**
* Given a set of SelectionNodes from a SelectionSet, ensure "id" is included.
*/
function selectionSetIncludesId(selectionNodes: readonly SelectionNode[]) {
return selectionNodes.some(selectionNode => {
switch (selectionNode.kind) {
case 'Field':
return selectionNode.name.value === 'id'
// Fragments have their own selection sets. If they do not include an "id",
// we will catch it when we visit those selection sets.
case 'FragmentSpread':
case 'InlineFragment':
return true
}
})
}
/**
* Walk the AST and inspect every SelectionSetNode. Every SelectionSet must include an "id"
* field, if one exists. It can include it directly, or as part of a fragment.
*
* See: https://graphql.org/graphql-js/language/#visit
*/
return {
SelectionSet(node: SelectionSetNode) {
let type = context.getType()
// Unwrap NonNull types
if (isNonNullType(type)) {
type = type.ofType
}
// Ensure this is an object type (meaning it has fields)
if (!isObjectType(type)) {
return undefined
}
const possibleFieldsIncludesId = 'id' in type.getFields()
if (possibleFieldsIncludesId && !selectionSetIncludesId(node.selections)) {
context.reportError(
new GraphQLError(
`Missing \"id\" field in \"${type.name}\"`,
node,
undefined,
undefined,
// N.B. In our implementation we pass in the source location as well for more
// useful errors.
location ? [location] : undefined
)
)
}
return undefined
}
}
}
Usage is something like this:
import { loadSchema, loadDocuments } from '@graphql-toolkit/core'
function validate(schemaAst: GraphQLSchema, sources: Source[]) {
let errors: GraphQLError[] = []
sources.forEach(source => {
if (source.document) {
errors = errors.concat(
// If you want pretty errors, also make "source.location" available to the IdOnObjectSelectionSetRule
validateGraphQL(schemaAst, source.document, [IdOnObjectSelectionSetRule])
)
}
})
return errors
}
// Load the schema and documents
const [schemaAst, documents] = await Promise.all([
loadSchema(/*... your schema ...*/, {
loaders: [new UrlLoader()]
}),
loadDocuments(/*... your documents ...*/, {
loaders: [new GraphQLFileLoader()],
skipGraphQLImport: true
})
])
const errors = validate(schemaAst, documents)
Hey, Today in one of my applications I found this issue. I'm not sure if this is a bug or desired behaviour because I couldn't find any information about it in the documentation.
The fix is quite easy, just add
id
to thequery
without id.So the question is: should every query has defined
id
for a resource? It seems to work withoutid
but when there will be the next query with definedid
, this error appears.I'm asking because I have a few other cases where external API doesn't have
id
so this problem can appear again in the future.Intended outcome: Error shouldn't appear and application should render data.
Actual outcome: Error appears
How to reproduce the issue: https://codesandbox.io/s/mom8ozyy9
Check
checkbox
in provided demo.Version