Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

loading stuck true after variable change #170

Closed
sgoll opened this issue Aug 25, 2016 · 55 comments
Closed

loading stuck true after variable change #170

sgoll opened this issue Aug 25, 2016 · 55 comments
Assignees
Labels

Comments

@sgoll
Copy link

sgoll commented Aug 25, 2016

I have the following problem: when the props of my GraphQL-connected component change, and I use one of the props in the options (as variables), loading is stuck to the value true whenever both the previous value and the current value resolve to no result.

Here is an example:

import ApolloClient from 'apollo-client';
import gql from 'graphql-tag';
import React from 'react';
import { graphql } from 'react-apollo';

// Display props.
const Demo = (props) => (
  <pre>{JSON.stringify(props, null, 2)}</pre>
);

// Connect to some GraphQL query.
const ConnectedDemo = graphql(gql`
  query getStuff($name: String!) {
    stuff(name: $name) {
      payload
    }
  }
`, {
  options: ({ name }) => ({ variables: { name } }),
})(Demo);

// Set up client.
const client = new ApolloClient(/* ... */);

const render = (props) => {
  ReactDOM.render((
    <ApolloProvider client={client}>
      <ConnectedDemo {...props} />
    </ApolloProvider>
  ), document.getElementById('main'));
};

// Initial render.
render({
  name: 'does-not-exist',
});

// Prop update: fetch.
window.setTimeout(() => {
  render({
    name: 'nothing-either',
  });
}, 1000);

Here, during the initial render, everything is fine: Demo receives { loading: true, stuff: null }. After some milliseconds, this is changed to { loading: false, stuff: null }.

However, when the prop is updated, Demo receives { loading: true, stuff: null } and it gets stuck there, even though the network request completed as did the first one.

The problem happens with react-apollo version 0.4.7 as well as 0.4.6. It does not happen whenever the first fetch did return an object (i.e. stuff was something other than null).

@jbaxleyiii
Copy link
Contributor

@sgoll I can't seem to replicate in a test? #173

@jbaxleyiii
Copy link
Contributor

Can you create a repo or a failing test?

@sgoll
Copy link
Author

sgoll commented Aug 27, 2016

@jbaxleyiii Sure. I created a failing mock demo at https://github.com/sgoll/react-apollo-170. Please let me know if that helps.

msimulcik added a commit to msimulcik/react-apollo that referenced this issue Aug 29, 2016
jbaxleyiii pushed a commit that referenced this issue Sep 5, 2016
@jbaxleyiii
Copy link
Contributor

Fixed with #191

@zol zol removed the in progress label Sep 5, 2016
@josechavezm
Copy link

I'm having this error after last update. had to downgrade.

@SachaG
Copy link

SachaG commented Apr 26, 2017

I've been running into the same issue. Could this be related to query response batching somehow? Maybe it's a coincidence, but I've noticed that when the infinite situation happens, the query response is always sent back from the server as a stand-alone result.

Whereas in scenarios where everything works as expected, it's always batched with the results of another query (I happen to have multiple separate query containers on the same page).

@helfer
Copy link
Contributor

helfer commented Apr 26, 2017

I highly doubt that this is related to batching. More likely it has something to do with fragments, where Apollo Client gets the result from the server, writes it to the store, then reads it back, but still thinks it only has partial data, and thus doesn't notify the observer.

If someone can provide a reproduction, we should be able to get this fixed pretty quickly @hiei189 @SachaG

@helfer helfer reopened this Apr 26, 2017
@SachaG
Copy link

SachaG commented Apr 26, 2017

Is there a guide on how to best provide reproductions? I seem to remember there was a sample app you could use?

@helfer
Copy link
Contributor

helfer commented Apr 26, 2017

@comus
Copy link

comus commented Apr 26, 2017

@helfer @SachaG
here is the repo to reproduce the issues we talked above
https://github.com/comus/react-apollo-issue
https://cl.ly/2b2D3J2A1e13

@helfer
Copy link
Contributor

helfer commented Apr 27, 2017

@thebigredgeek I think this is a reproduction of the same issue that we tried to debug together a while back. Can you take a quick look at the video and tell me if you think my assessment is correct?

The problem here is also due to a discrepancy between the observer.next result and the currentResult. Interestingly @comus says that it was introduced with react-apollo 0.11.0 and didn't occur before.

If you want some mystery to solve, this could be an interesting puzzle. I took a quick look but couldn't get to the bottom of it yet, and I won't have time to look again before the weekend.

@thebigredgeek
Copy link
Contributor

thebigredgeek commented Apr 27, 2017

@helfer yeah this is the EXACT issue, though I haven't encountered it in a while (ever since rebuilding the product).

@jbaxleyiii
Copy link
Contributor

jbaxleyiii commented Apr 29, 2017

geez, this is the issue that will never end 👎

jbaxleyiii pushed a commit that referenced this issue Apr 29, 2017
@jbaxleyiii
Copy link
Contributor

@helfer @thebigredgeek I setup an error repor #666

@pencilcheck
Copy link

If I have to load two queries with different variables (just different in one key) and use the { branch } from recompose to get around the loading, somehow it will get stuck in loading as well but only between loading of pages that has already been loaded (using nextjs). Which means it is loading the page in the client only.

Had to downgrade to 0.8.3 to remove the infinite loading.

@helfer
Copy link
Contributor

helfer commented May 6, 2017

Alright, small update: I've figured out that for the reproduction by @comus the issue doesn't occur without recycling observable queries. It's pretty hard to track down what the exact issue is, but it must have something to do with parameters not correctly being updated when a query is recycled.

That's all I know so far, I'll keep poking around until I find the bug. 🐞

@Morphexe
Copy link

Morphexe commented Sep 6, 2017

I am having the exact same issue here, renaming the query does not solve the issue, a full page refresh does work though.

@nabati
Copy link

nabati commented Sep 6, 2017 via email

@oliviertassinari
Copy link

I can reproduce the issue on [email protected]. It happens when unmounting the component with a variable x and mounting it again with a variable y.

@adamwade2384
Copy link

Having the same issue, has there been any progress resolving this one?

@kien8995
Copy link

same issue ! any working on this?

@oliviertassinari
Copy link

oliviertassinari commented Sep 19, 2017

I have been moving my apollo HOC one level higher in the react tree to dodge the issue. But it comes with some tradeoff. I wish I could put it as close as where the data is needed. @kien8995 The issue has been open for a year and have around 20 people involved. I'm sure the maintainers are aware of this issue. So either they miss a reproduction example and I understand it. Or there is a much larger core issue related to the design of the project.

@mattfysh
Copy link
Contributor

Also just ran into this issue... tried the naming workaround but couldn't get that to work. @oliviertassinari could you describe your workaround? not sure I followed

Something I've noticed with this issue is that the query that causes loading to become stuck is trying to fetch from the network, when the results should be drawn from the cache. If I navigate away from the screen that is stuck loading, and I go back to it then the results load from the client cache and everything is fine.

In Redux, the events are: INIT, RESULT, <nav to screen B>, INIT, RESULT, <nav back to screen A>, STOP, INIT, RESULT

What I believe they should be: INIT, RESULT, <nav to screen B>, STOP, INIT, RESULT, <nav back to screen A>, STOP, INIT, RESULT_CLIENT

The two issues you can see here:

  1. STOP is not fired when navigating to screen B
  2. navigating back to screen A causes a network call, and fires RESULT instead of RESULT_CLIENT

@mattfysh
Copy link
Contributor

Just found this solution, over on apollographql/apollo-client#1186 ... set notifyOnNetworkStatusChange to true - it works!

@cyrus-za
Copy link

thanks @mattfysh

Where do you set it? inside the constructor of ApolloClient or inside each and every query decorator?

@mattfysh
Copy link
Contributor

@cyrus-za I'm setting it on just the affected query component right now

@kien8995
Copy link

thanks @mattfysh 👍 it works !

@cyrus-za
Copy link

cyrus-za commented Sep 19, 2017 via email

@evan-scott-zocdoc
Copy link

notifyOnNetworkStatusChange should really be on by default. Does anyone know why it isn't? Found this very surprising and frustrating.

@stale
Copy link

stale bot commented Oct 13, 2017

This issue has been automatically labled because it has not had recent activity. If you have not received a response from anyone, please mention the repository maintainer (most likely @jbaxleyiii). It will be closed if no further activity occurs. Thank you for your contributions to React Apollo!

@stale
Copy link

stale bot commented Oct 27, 2017

This issue has been automatically closed because it has not had recent activity after being marked as no recent activyt. If you belive this issue is still a problem or should be reopened, please reopen it! Thank you for your contributions to React Apollo!

@stale stale bot closed this as completed Oct 27, 2017
@renatorib
Copy link

renatorib commented Nov 1, 2017

I have this same problem here. Any progress in this issue?

@mbrochh
Copy link

mbrochh commented Nov 7, 2017

I just ran into this issue as well. Setting notifyOnNetworkStatusChange: true worked for me as well.

@batjko
Copy link

batjko commented Mar 23, 2018

Still having this issue with react-apollo 2.04, and setting notifyOnNetworkStatusChange: true does nothing for me in this case.
Instead the client simply throws the network error (e.g. the 404 that one of my field resolvers returned) and loading stays at true.

@AndrewPrifer
Copy link

AndrewPrifer commented Jul 3, 2018

Just hit this bug as well. My scenario is very similar to that of @cyrus-za and @oliviertassinari. I managed to isolate the kinds of updates that result in loading being stuck to true and data not updating; Maybe the following will help someone with more intimate knowledge of Apollo internals track down the bug.

We are using react-apollo 1.4.16 and apollo-client 2.3.5.

Our scenario:
We use react router 4 and each page is wrapped in graphql(). When the page mounts, the query is executed with variables supplied to the page. We don't currently use id-based cache normalization, but the default query path based one.

I isolated 3 conditions that are necessary for it to be stuck:

  1. I navigate away from a page and then back again with a different set of variables supplied to the page.
  2. The query results are the same as those from the previous page visit. If the results are different there is no problem.
  3. There cannot be another page visit between the two, which returns a different query result. For some reason, if I navigate away, then back, and the resulting query returns a different result than the previous, then even if a subsequent page visit does lead to the same query result as the first one, it works.

Like others have noted before me, the queries execute without errors, the data is in the cache, it is just that when the data is loaded my page is not updated by the graphql() HOC. And notifyOnNetworkStatusChange: true did work for me, probably because it triggers updates to the component even if data loading doesn't.

@hinok
Copy link

hinok commented Jan 26, 2019

I can confirm that this bug occurs on

when conditions above specified by @andrewpeterprifer are met.

Setting notifyOnNetworkStatusChange: true solves the issue. The sad part of this bug is that it's so "low level" that I even haven't thought that it may be a bug in apollo thus I spend quite big amount of time debugging my own code...

Maybe it'd be worth to write about this issue in the official docs?

@xiel
Copy link

xiel commented Feb 10, 2019

Any progress on this? Ran into the issue as well and setting notifyOnNetworkStatusChange can't be the answer.

@devdudeio
Copy link

devdudeio commented Feb 13, 2019

use the new Query Component and dont do a query while exporting the component:

avoid something like this

export default withApollo(graphql(myQuery, {
    options: () => ({
        notifyOnNetworkStatusChange: true
    })
})(myComponent))

you will also face some bugs with the fetchPolicy settings that will just not work here.

e.g. you load a component and you revisit it but you get no updates.

@Maxtermax
Copy link

Base on the NetworkStatus Codes apparently the loading state come from networkStatus < 7 but that state is not reliable, a simple solution could be:
set your own loading variable, like this.

 <Query
      notifyOnNetworkStatusChange={true}
     query={/**/}
>
  {
    ({networkStatus, error, data}) => {
       let loading = !(networkStatus === 7 || networkStatus === 8);
       if(loading) return <h1>Loading:  { `${loading }` }</h1>
       return <h1>{data.name}</h1>
     }
   }
</Query>

be aware the code 7 mean that there is not query in the fligh and everything is fine (Like the meme), and the code 8 mean that there is not query in the fligh but got an error, so in both cases the request is complete, negating both cases !(networkStatus === 7 || networkStatus === 8) mean that the request is not complete and can consider the state as loading

@dnbkr
Copy link

dnbkr commented May 28, 2019

FYI: I am able to recreate this issue with "react-apollo": "2.5.5" - notifyOnNetworkChange solves the problem, but obviously not ideal.

@bardb
Copy link

bardb commented May 30, 2019

I've had the same issue.

Since it only happens when my back end sends the same response twice in a row, I make sure it never sends the same response twice in a row. I happened to have a "hello(name:S)" method which returned "Hello, S", which we had been using for early testing.

So I changed my query from
query Q($x:String) { stuff_I_want(x: $x) { name } }
to
query Q($x:String, $uniquification: String) { stuff_I_want(x: $x) { name } avoid_deduping: hello(name: $uniquification)

Then I generate a unique string — for my application, the millisecond clock is plenty unique — and pass it as $uniquification.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests