Subscriptions not triggering rerender on initial data even if cache updates

[robbert229]

Intended outcome:

I just finished upgrading a project to use the new apollo 3.0.0 packages. I expected that my subscription components would update and work like normally.

Actual outcome:

I have noticed that after upgrading that my subscription components are no longer deterministically rerendering upon initially receiving the data. They are rendering with loading true, but then not rerendering when the subscription actually receives the data. When the subscription updates again later the component correctly rerenders

How to reproduce the issue:

Specifically the data correct data is being received by the frontend, but the components themselves are not updating. I have observed this behavior with the hoc, and hooks apis.

Here the apollo devtools show that apollo has properly received the data from the backend. This is the data that I expect apollo to have, and to push to the components that are subscribed to the subscription.

Here is the chrome devtools network showing that the websockets side of things is functioning correctly. The data is requested, and returned correctly.

But the component itself doesn't update correctly. The onSubscriptionData function fires when data is received on the frontend, but the component doesn't rerender after receiving the data.

// I used this snippet to reproduce the observed behavior using hooks.
const { data, loading, error } = useSubscription(WatchSystemBannerDocument, {
    onSubscriptionData: (e) => console.log(e),
});
console.log(data, loading, error);

Keep in mind that this snippet is able to trigger the behavior, but the HOC api for apollo does so as well.

Version

npx: installed 1 in 1.259s

  System:
    OS: Linux 4.20 Manjaro Linux undefined
  Binaries:
    Node: 11.15.0 - /usr/bin/node
    Yarn: 1.17.3 - ~/.yarn/bin/yarn
    npm: 6.10.0 - /usr/bin/npm
  Browsers:
    Firefox: 67.0.4
  npmPackages:
    apollo-cache: ^1.3.2 => 1.3.2 
    apollo-cache-inmemory: ^1.6.3 => 1.6.3 
    apollo-client: ^2.6.4 => 2.6.4 
    apollo-link: ^1.2.12 => 1.2.12 
    apollo-link-debounce: ^2.1.0 => 2.1.0 
    apollo-link-error: ^1.1.11 => 1.1.11 
    apollo-link-http: ^1.5.15 => 1.5.15 
    apollo-link-mock: ^1.0.1 => 1.0.1 
    apollo-link-ws: ^1.0.18 => 1.0.18 
    apollo-utilities: ^1.3.2 => 1.3.2 
    react-apollo: ^3.0.1 => 3.0.1 

It doesn't look like envinfo returns the correct packages for the new @apollo scoped packages. So here are those.

    "@apollo/react-common": "^3.0.1",
    "@apollo/react-components": "^3.0.1",
    "@apollo/react-hoc": "^3.0.1",
    "@apollo/react-hooks": "^3.0.1",

[robbert229]

Some digging has shown me that my issues appear to be stemming from react-apollo/packages/hooks/src/data/SubscriptionData.ts

This following section being the area that causes the issue.

  private updateResult(result: SubscriptionResult) {
    if (this.isMounted) {
      this.setResult(result);
    }
  }

  private updateCurrentData(result: SubscriptionResult<TData>) {
    const { onSubscriptionData } = this.getOptions();

    this.updateResult({
      data: result.data,
      loading: false,
      error: undefined
    });

    if (onSubscriptionData) {
      onSubscriptionData({
        client: this.refreshClient().client,
        subscriptionData: result
      });
    }
  }

The onSubscriptionData function is called by updateCurrentData so I know that updateCurrentData is functioning correctly, but when we get to updateResult this.isMounted appears to be false. This is causing the component to not refresh.

Inside of useSubscription (useEffect(() => subscriptionData.afterExecute()); ) we see that afterExecute is called towards the end of the function. This is the function that sets isMounted to true in SubscriptionData.ts.

It appears that root cause is that the useSubscription gets the data, and calls updateResult to update the component before the component thinks its mounted.

[robbert229]

I have found a solution, though I am not sure that it should be the solution since I am still a bit green with hooks. So in the execute for the SubscriptionData if startSubscription() moved to afterExecute we could be sure that the we wouldn't get any data / responses before the component is mounted. This solution worked for me, but I am not sure of the ramifications of such a change.

  public execute(result: SubscriptionResult<TData>) {
    let currentResult = result;

    if (this.refreshClient().isNew) {
      currentResult = this.getLoadingResult();
    }

    let { shouldResubscribe } = this.getOptions();
    if (typeof shouldResubscribe === 'function') {
      shouldResubscribe = !!shouldResubscribe(this.getOptions());
    }

    if (
      shouldResubscribe !== false &&
      this.previousOptions &&
      Object.keys(this.previousOptions).length > 0 &&
      (this.previousOptions.subscription !== this.getOptions().subscription ||
        !isEqual(this.previousOptions.variables, this.getOptions().variables))
    ) {
      this.endSubscription();
      delete this.currentObservable.query;
      currentResult = this.getLoadingResult();
    }

    this.initialize(this.getOptions());

    this.previousOptions = this.getOptions();
    return { ...currentResult, variables: this.getOptions().variables };
  }

  public afterExecute() {
    this.isMounted = true;
    this.startSubscription();
  }

Originally I stated that I was seeing the same defect with hooks and the hoc component. Since I have tracked the defect down into hooks only code I imagine that there may be a similar defect in the hoc component.

[zenios]

I have exactly the same issue.The problem is the same as you described. The only difference is that sometimes i get data and sometimes i don't but this is because the issue is actually a timing issue.Whether we will be able to get data before or after mounted is called.

In normal network calls not localhost, most probably you will never have that issue since mounted will be faster than network.In localhost though things are different and there is a case of data to arrive before "afterExecute" effect gets called

If you look at the queryData class in execute a check is being done whether is mounted before starting the querySubscription while as in the subscriptionData class that check is not there

[robbert229]

@zenios oh, good catch. I didn't even think to look in queryData :derp

If you look at the queryData class in execute a check is being done whether is mounted before starting the querySubscription while as in the subscriptionData class that check is not there

[redmundas]

+1, have the same issue

[gopeter]

The same happens for me, but I'm using the query component and run subscribeToMore as described here: https://www.apollographql.com/docs/react/advanced/subscriptions/#subscribetomore

I get the right new data in my subscribeToMore function, but the component doesn't get rerendered.

[benkahle]

I think I'm hitting a similar issue and have found the behavior change to be between react-hooks@0.1.0-beta.10 and react-hooks@0.10-beta.11 (with the same behavior continuing into 3.0.1):
In beta.10 when I clear the cache (client.clearStore) I see a re-run of my query hook with the previous data value but loading: true.
In beta.11, I see the same re-run of the query with the previous data value but loading: false. Only after my component has re-rendered (with the out-of-date previous state), do I see a query result with loading: true and data set to {} (I think that is related to the initial data {} vs undefined note addressed in the 3.1.0 beta.

As @robbert229 and @zenios pointed out above, the beta.10 - beta.11 change set includes this change in when startQuerySubscription gets called that may be introducing a timing/race condition with regard to component mounting:

-    if (!skip) {
-      this.startQuerySubscription();
-    }
+    if (this.isMounted) this.startQuerySubscription();

(As well as the introduction of LazyQuery)
I'm not familiar enough with the hook implementation to point to a specific issue but I will see if I can simplify my repro scenario to isolate it more clearly.

Full beta.10 - beta.11 change set

Update: In reading through more open issues, what I described above may actually be closer to the loading state issue described in #3361, I'll leave this comment here in case that specific change set is still related to either or both issues.

[hwillson]

Would someone here be able to test with React Apollo 3.1.0 and let us know if this is still happening? Thanks!

[redmundas]

@hwillson it solved my problem!
previously I had a problem with subscriptions where some subscriptions were executed multiple times or they were executed as needed but the data wasn't provided to component through props. with new version I can't reproduce this anymore

[hwillson]

Great, thanks @edmundas-ramanauskas!

[robbert229]

@hwillson I can still reproduce this defect on React Apollo 3.1.0, because the concurrency defect I pointed out still exists. If data is received after startSubscription() is called in execute, but before the useEffect hook that calls subscriptionData.afterExecute takes place then then the data is lost.

The steps I initially took to fix the issue also still work on 3.1.0.

@benkahle does upgrading to 3.1.0 resolve your issue?

[benkahle]

@robbert229 Unfortunately I was only able to test very briefly on 3.1.0 which seemed to cause further issues wherein calling client.clearStore caused a re-render loop re-triggering my query until I exit. This new, very broken behavior is making me think my situation is an issue with my own implementation (that was perhaps masked until beta.11), and not actually in the library. Although if you're still seeing it too, maybe not 🤷‍♂.

I will see if I can simplify my implementation to isolate closer test case again.

[mindnektar]

@hwillson This is definitely still broken. I can reproduce this on 3.1.1 using any of my subscriptions. It was still working before I upgraded from 2.5.3. The cache updates correctly, but logging the cache contents in my components returns the old data. If I trigger a re-render, the cache contains the new data. Is there any news to this issue?

[paulsocal]

I'm also experiencing the same issue with 3.1.3

[mindnektar]

Can anybody please verify if it is possible to integrate the solution proposed by @robbert229 ? It seems plausible enough to me, but the ramifications are still unclear. This is still a huge problem, I don't see a way to use subscriptions right now without employing a stupid workaround that updates the cache a second time after a timeout (which introduces all kinds of followup issues).

[rmagatti]

Running into this too. @hwillson, any chance we can reopen this issue?

[tnguven]

Me too, I am getting the subscriptionData but is not updating my UI and also cache, after I got another update prev return undefined.