apollographql / apollo-client

:rocket:  A fully-featured, production ready caching GraphQL client for every UI framework and GraphQL server.
https://apollographql.com/client
MIT License
19.35k stars 2.66k forks source link

Inside StrictMode useSubscription subscribes twice #6037

Closed anajavi closed 2 months ago

anajavi commented 4 years ago

Intended outcome:

useSubscription should subscribe only once when run inside <StrictMode>

Actual outcome:

useSubscription subscribes twice to the same subscription. I think that this happens because <StrictMode> intentionally runs component bodies twice. I suppose that same thing could happen in concurrent mode too.

This results in double network traffic. Also, the second subscription won't get unsubscribed when the useSubscription is unmounted.

This can be inspected on chrome network tab:

usesubscription_chrome

in the picture, id 15 and 16 are the single useSubscription and receive the same data. After unmount the id 16 was closed and id 15 was left receiving data(not pictured).

I am not familiar with apollo-client codebase, but I think that this is result of the following line being run in function body instead of inside useEffect: https://github.com/apollographql/apollo-client/blob/23b17fa7283b21966d1c633c52b848f12b4a211e/src/react/hooks/useSubscription.ts#L44

How to reproduce the issue:

Any subscription should behave the same when run in StrictMode.

const SubComponent = ({vesselId}) => {
  const { data } = useSubscription(vesselStatus, {
    variables: { vesselId }
  });
  return null;
}
<StrictMode>
  <SubComponent vesselId="245" />
</StrictMode>

Workaround is to not use StrictMode, but I'm afraid that will result in code that breaks when concurrent mode lands in React.

Versions

System: OS: macOS 10.15.3 Binaries: Node: 12.16.1 - ~/.nvm/versions/node/v12.16.1/bin/node Yarn: 1.22.0 - ~/.nvm/versions/node/v12.16.1/bin/yarn npm: 6.13.4 - ~/.nvm/versions/node/v12.16.1/bin/npm Browsers: Chrome: 80.0.3987.132 Firefox: 73.0.1 Safari: 13.0.5 npmPackages: @apollo/client: ^3.0.0-beta.39 => 3.0.0-beta.39 @apollo/link-ws: ^2.0.0-beta.3 => 2.0.0-beta.3 apollo-server-express: ^2.11.0 => 2.11.0 npmGlobalPackages: apollo-codegen: 0.20.2 apollo: 2.24.0

anajavi commented 4 years ago

If there is interest in fixing this, I can try to make a PR of a failing test case? Not sure it can be done cleanly though.

flootr commented 4 years ago

I'm having the same issue. <StrictMode /> e.g. seems to be enabled for create-react-app by default and causes the useSubscription hook to subscribe twice on initial render.

Update: strict mode also causes trouble when setting a pollInterval in useQuery which then keeps polling even if the component is unmounted.

anajavi commented 4 years ago

Update: strict mode also causes trouble when setting a pollInterval in useQuery which then keeps polling even if the component is unmounted.

Both of these hooks will then fail under concurrent mode.

Tadimsky commented 4 years ago

Yep, running into this issue as well - all of my useSubscription's are subscribing twice.

estubmo commented 4 years ago

Experiencing the same issue. Any news?

esseswann commented 4 years ago

Confirmed

csteuer commented 4 years ago

We have the same problem. Any updates?

brunoreis commented 4 years ago

I have just started playing with Subscriptions. So I have a fresh CRA app here, and it is duplicating subscriptions. When I remove StrictMode, it stops.

boucherv-project commented 4 years ago

Same issue for me

MaxTibs commented 4 years ago

Notice that this is true only if you're in development mode. Try to run a production build and this effect will go away.

Fxlr8 commented 4 years ago

Same thing for me. Wasted a couple of hours. I thought that I was going crazy.

stephencorwin commented 4 years ago

Try to run a production build and this effect will go away.

That might hide the symptoms, but having to run a production build when doing active development would get tedious and annoying real fast. We need a real solution for this issue.

MaxTibs commented 4 years ago

I agree, this is annoying. In my case I listen for events and display an alert for each of them in the UI which turns out to be duplicated. However the problem does not come from Apollo-Client, it is React strict mode behavior.. I don't think we can expect a fix coming from Apollo to remove this behavior since it is external to Apollo-Client.

I can see multiple way to limit the side effect of this behavior :

anajavi commented 4 years ago

I agree, this is annoying. In my case I listen for events and display an alert for each of them in the UI which turns out to be duplicated. However the problem does not come from Apollo-Client, it is React strict mode behavior.. I don't think we can expect a fix coming from Apollo to remove this behavior since it is external to Apollo-Client.

There is a reason for strict mode rendering twice. It is to prevent bugs in the coming concurrent mode. So this should be fixed in apollo-client.

igor10k commented 4 years ago

The worst thing is that those duplicated observables leak and never get destroyed, so when you do client.resetStore() all of them get refetched and this most probably will throw errors because the auth token is not set anymore. I'm surprised that this issue is completely ignored for almost half a year already.

semoal commented 4 years ago

Screenshot 2020-09-07 at 15 14 50 I'm still getting this issue, returning twice when is triggered the subscription

davegariepy commented 4 years ago

experiencing this now, seeing duplicate subscriptions. First thing I thought of was to use useEffect, then I found this thread.

raymclee commented 3 years ago

im having this issue also

neildotvn commented 3 years ago

I have the same issue

ybatsiun commented 3 years ago

The same issue

MR-Neto commented 3 years ago

Same issue

khaphannm commented 3 years ago

same issue here, please help

kelvin2200 commented 3 years ago

https://github.com/apollographql/subscriptions-transport-ws/blob/master/src/client.ts

can't wrap my head around why this is happening but in DEV mode

this.applyMiddlewares (which returns a promise)

seems to be resolving twice for the same subscription BUT... with a different id

private executeOperation(options: OperationOptions, handler: (error: Error[], result?: any) => void): string {
    if (this.client === null) {
      this.connect();
    }

    const opId = this.generateOperationId();
    this.operations[opId] = { options: options, handler };

    this.applyMiddlewares(options)
      .then(processedOptions => {
        this.checkOperationOptions(processedOptions, handler);
        if (this.operations[opId]) {
          this.operations[opId] = { options: processedOptions, handler };
          this.sendMessage(opId, MessageTypes.GQL_START, processedOptions);
        }
      })
      .catch(error => {
        this.unsubscribe(opId);
        handler(this.formatErrors(error));
      });

    return opId;
}

the odd thing is, even though this condition exists

if (this.operations[opId]) {
      this.operations[opId] = { options: processedOptions, handler };
      this.sendMessage(opId, MessageTypes.GQL_START, processedOptions);
}

AND opId is created only once BEFORE calling

this.applyMiddlewares(...)

The second time the promise is resolved ...the same condition evaluates to true for an id that is not created and shouldn't exist in this.operations

didn't dig too much through the code, so this... could be totally unrelated to why the subscription is fired twice but maybe its a good starting point

Dezzymei commented 3 years ago

👍 same issue

mathieu-bour commented 3 years ago

I also can confirm the issue. Does someone have an idea of why this is happening? I don't really understand the relationship between <React.StrictMode/> and the subscriptions...

anajavi commented 3 years ago

I also can confirm the issue. Does someone have an idea of why this is happening? I don't really understand the relationship between <React.StrictMode/> and the subscriptions...

Because in development StrictMode causes function component bodies to double-execute: https://reactjs.org/docs/strict-mode.html#detecting-unexpected-side-effects

The useSubscription hook is breaking contract with react by starting the subscription in function body.

mathieu-bour commented 3 years ago

I also can confirm the issue. Does someone have an idea of why this is happening? I don't really understand the relationship between <React.StrictMode/> and the subscriptions...

Because in development StrictMode causes function component bodies to double-execute: https://reactjs.org/docs/strict-mode.html#detecting-unexpected-side-effects

The useSubscription hook is breaking contract with react by starting the subscription in function body.

Thank you very much for the precision. I'll try to look on the useSubscription hook and maybe submit a PR to fix this.

sgentile commented 3 years ago

I get multiple as well

joshbedo commented 3 years ago

Saw the same thing happening earlier removing StrictMode resolved it.

Xazzzi commented 3 years ago

StrictMode also affects reactive variables for me. I have cache policy field that reads rv. When changed rv's value is updated, broadcast to cache, onNewData call triggers, but deep-memo'd state does not update (or perhaps it does in first render, clearing some internal dirty flags, and skips doing so in second strict mode render).

SnekNOTSnake commented 3 years ago

Issues like these are why beginners like me often bang their head on a table.

anajavi commented 3 years ago

The #7952 is related.

tehpsalmist commented 2 years ago

At least I now know this is only a development issue, but sheesh, what a waste of time.

highri5e commented 2 years ago

still an issue

AlexanderWells-diamond commented 2 years ago

Also seeing the same issue.

bfeucht-wof commented 2 years ago

I am seeing this issue in both development and production mode, without strict mode. I'm not sure if this is relevant, but the call stack is different in

In production mode, I get

Error: Observable cancelled prematurely
    at t.removeObserver (LocalState.ts:147:27)
    at n (LocalState.ts:62:25)
    at b (TextareaAutosize.js:88:7)
    at e.unsubscribe (TextareaAutosize.js:203:7)
    at b (TextareaAutosize.js:93:21)
    at e.unsubscribe (TextareaAutosize.js:203:7)
    at resolveProps.js:118:20
    at Lu (scheduler.production.min.js:262:224)
    at t.unstable_runWithPriority (index.js:18:343)
    at Ko (scheduler.production.min.js:122:325)

In dev mode I get:

Error: Observable cancelled prematurely
    at Concast.removeObserver (Concast.ts:147:1)
    at Concast.ts:62:1
    at cleanupSubscription (module.js:88:1)
    at Subscription.unsubscribe (module.js:203:1)
    at cleanupSubscription (module.js:93:1)
    at Subscription.unsubscribe (module.js:203:1)
    at useSubscription.ts:118:1
    at HTMLUnknownElement.callCallback (react-dom.development.js:3945:1)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:3994:1)
    at invokeGuardedCallback (react-dom.development.js:4056:1)
    at flushPassiveEffectsImpl (react-dom.development.js:23543:1)
    at unstable_runWithPriority (scheduler.development.js:468:1)
    at runWithPriority$1 (react-dom.development.js:11276:1)
    at flushPassiveEffects (react-dom.development.js:23447:1)
    at react-dom.development.js:23324:1
    at workLoop (scheduler.development.js:417:1)
    at flushWork (scheduler.development.js:390:1)
    at MessagePort.performWorkUntilDeadline (scheduler.development.js:157:1)
yovanoc commented 1 year ago

same issue, but like others, only in dev

andershhh commented 8 months ago

@alessbell @anajavi do you know if this problem was ever resolved in some way? I'm experiencing this exact issue using "@apollo/client": "^3.8.10". I'd prefer to keep strict mode on for my project.

alessbell commented 8 months ago

Hi @andershhh 👋 There isn't a simple fix here since we'll have to change the mechanism by which we register and track subscriptions, so yes this is still an issue. The workaround for now is to disable strict mode.

andershoegh commented 8 months ago

Hi @andershhh 👋 There isn't a simple fix here since we'll have to change the mechanism by which we register and track subscriptions, so yes this is still an issue. The workaround for now is to disable strict mode.

Hi! Thank you for the clarification and recommendation on how to resolve the issue in the meantime. We've opted, currently, to solve it in the few places where it causes an issue, in the code instead of turning off strict mode. This is okay for now, while the use of the subscribetomore feature is relatively small for us.

Thank you for all the work all the contributors are putting into the project 😊🙏🏻

UchihaYuki commented 3 months ago

Any updates? It's been 4 years now.

phryneas commented 3 months ago

@UchihaYuki #11863 changes the subscription to useSyncExternalStore, which should fix the problem. It will be released in 3.11.

phryneas commented 2 months ago

Version 3.11 has been released with a rewrite of useSubscription.

If you find any problems with useSubscription after 3.11, please open a new issue. I'm closing this issue since the code that originally caused this issue is not part of the library anymore.

github-actions[bot] commented 2 months ago

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo Client usage and allow us to serve you better.

github-actions[bot] commented 1 month ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. For general questions, we recommend using StackOverflow or our discord server.