Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Errors from ObservableQuery.currentResult are not referentially equal #2278

Closed
dallonf opened this issue Oct 9, 2017 · 5 comments
Closed

Comments

@dallonf
Copy link

dallonf commented Oct 9, 2017

Intended outcome:

Using react-apollo (although I think the issue is in apollo-client, see below), I would like to trigger an imperative action (show an error notification) when a load error occurs. The most straightforward path to me is to compare props.data.error with nextProps.data.error, and if they are different, show the notification for the new error.

Actual outcome:

The data.error prop is actually a new ApolloError instance every time the component renders. Based on the stack trace, it appears to be created in ObservableQuery.currentResult, which in turn is called by GraphQL.render.

Because these errors are new for every render, the error notification will appear repeatedly. To get my intended behavior, I would have to compare error.networkError and error.graphQLErrors (which do seem to be referentially stable), which couples my generic error handling to implementation details of Apollo Client.

How to reproduce the issue:

Here is a sandbox:

https://codesandbox.io/s/9jon77z484

Note that whenever the graphql wrapper is re-rendered (with any new props, to bypass the componentShouldUpdate check), the error instance is new and so logs a new message, even though it is logically the same error.

Version

@jbaxleyiii jbaxleyiii added this to the 2.0 milestone Oct 10, 2017
@jbaxleyiii
Copy link
Contributor

@dallonf this should actually already be fixed with the 2.0 👍

@dallonf
Copy link
Author

dallonf commented Mar 20, 2018

@jbaxleyiii I'm still seeing this issue in 2.2.7 - either it regressed or was never fixed. Let me try to recreate the sandbox with the latest version.

@dallonf
Copy link
Author

dallonf commented Mar 20, 2018

Yup, here's a sandbox with the latest version of everything:
https://codesandbox.io/s/jplj2wp8l9

(Note the dependency on [email protected] - in the current production version of react-apollo, you'll instead observe apollographql/react-apollo#1336, which masks this issue)

@danilobuerger
Copy link
Contributor

Confirmed on

ObservableQuery.currentResult() creates a new ApolloError on each return thus not allowing referential checks.

@jbaxleyiii
Copy link
Contributor

jbaxleyiii commented Jul 9, 2019

@danilobuerger if this is still an error, can you open a new issues instead please?

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants